> When we called the isOwner function, it returned a Promise even though our function signature did not specify it as an async function. Our synchronous-looking function was invisibly converted into an async function. This meant it no longer returned a boolean, it returned a Promise.
I'll be down voted by JavaScript lobby for saying this but I'll still say this.
Never use JavaScript on the server side. The amount of bugs that can happen is insane. JavaScript is just a specification with varying implementations across various versions.
A proper language like Java, Go, or Rust ensures that your code will have fewer logical bugs of this type.
this issue was caused by a framework that's trying to do too much, relying on "magic" interfaces to supposedly reduce developer burden. the function is very unambiguously written and the language did nothing wrong
I also support using whatever language you and your team prefer when you can. that's the glory of backend: no restrictions on what you can run. but sometimes you need to write client software, and those are strictly easier to manage in the platform's native tongue: Swift, Kotlin, JS, and so on
This reminds me of functions that rely on properties, but the properties could, themselves, be functions (in Swift, we call them "computed properties," and they are indistinguishable, on the outside, from stored properties).
C++ can also have a lot of tripwires, from overloaded operators (same with Swift, but people don't overload operators very often, in Swift. They compute properties, all the time, though).
I almost never write single function unit tests. There's usually some subset of the codebase that's self contained that makes sense to be the "unit" you're testing, but it's almost always a handful of functions with a clear entry point.
My general rule is to never mock or remove anything that has no side effects from the execution path of a test, even if it's some utility function that's already tested in isolation in its own test suite - trying to isolate every little bit of behaviour to be tested separately is just a bunch of extra work for questionable benefit.
I still call these tests "unit tests", and I think a lot of people do also. But there are the dogmatic people to whom only a test covering a single function with all dependencies mocked out is a true unit test.
I think you're looking for `import 'server-only'` and not "use server";. Use server exposes functions as endpoints to the client. I.e. they are simply obfuscated api endpoints without boilerplate. Their main use is for mutations such as a form submission from the client.
Since pages are, by default, SSR, you don't need to have the server call out to itself to run an endpoint to check permissions. Instead, the server should just run the function.
I'm pretty sure Next does some behind the scenes black magic optimizations and doesn't actually make an API request over the wire, but it's still running through some layer of abstractions (causing it to be async) to run the function when instead it could simply be a synchronous function if implemented properly.
These abstractions make sense if you know how to use them properly, but I honestly blame Nextjs for the whole server action confusion. I remember when they first came out and seeing how almost every single question on /r/nextjs was about being confused about server actions. All these footguns and confusion to avoid some boilerplate... I'm not sure if they've improved it since, but I've moved to Svelte and haven't looked back.
> It is written as if this only applies to functions you mark with use server and not the whole file.
The demonstration code illustrating the problem uses a file-level "use server" directive (and doesn't have other functions in the same file with the problem isOwner function); if they were using function level "use server" directives and it impacted a different function in the same file, I would say this is clearly surprising and unexpected (and even buggy) behavior, but this seems to be using a clearly documented feature and getting exactly what is advertised.
I don't know next.js, can someone explain how the client side can call a server function inside of an IF on the client side for security? It seems like there would be a trivial bypass of the security from the client side.
It opaquely makes a network call. You call it from the client-side and it abstracts away the network round-trip, but inside the function context you're running code on the server.
Under the hood it opens up an endpoint and the function calls it via a HTTP request.
always nice to read about these things. i like the note on 'all tests were green'. it sounds like the test of this function only test for the good case right? it should also test a false case? or am i missing something here?
We tested for both, the "happy" and the negative path. But the Javascript unit tests are run without the framework in between. So our function was returning expected results when run in isolation. Only when running it with Next.js the function became async which led to this dilemma.
> The snippet above was called as a server function. This is React's new way of calling server side code from the client side.
Tangential to the post, but mixing client-side and server-side code sounds like a recipe for disaster. There are already one too many services that perform authorization client-side, and I have a feeling making it harder to tell what runs where only makes the situation worse.
The problem was the framework's bundler was transforming those functions to return promises, a linter would need to understand next.js's specific transforms to catch this.
IMO, the idea of trying to blur the line between client and server is a big mistake. I worked on WebSocket frameworks in the Node.js space so I was also tempted but I completely abandoned this approach years ago. Though with Node.js, I do often reuse utility functions between client and server, I reject any framework which tries to hide the separation. I demand to have complete understanding of where the code is executing and how. I need to know what is being executed, where and how.
I also avoid technologies where the code I write is different from the code being executed. This is why I've been avoiding TypeScript. It performs code transformations which obfuscate my understanding of the logic which will be executed. With TS, the code I write is not the code which gets executed. This is scary to me. My code gets compiled into some messy junk-looking code and then that messy junk gets executed in ways I don't fully comprehend.
> I also avoid technologies where the code I write is different from the code being executed.
Not to be snarky, but as opposed to writing assembly? Where do you draw the line if you don't allow TypeScript (which is, with limited exceptions, only type erasure unless you specifically request polyfill), but allow other forms of compilation? Would you define JVM bytecode or IR as messy-looking junk code?
It's hard to see how a principled line could be drawn here.
You can use (ts 5.8 and above) a compiler flag `erasableSyntaxOnly` which enforces only TS features that can be dropped without modifying the underlying JavaScript. Unfortunately one of my favourite TS features, parameter properties, isn't erasable although it doesn't change runtime semantics.
My first thought was is JavaScript === case insensitive for string comps, because while I do use minimal JavaScript to enhance some web pages functionality it’s all basic vanilla JS.
> When we called the isOwner function, it returned a Promise even though our function signature did not specify it as an async function. Our synchronous-looking function was invisibly converted into an async function. This meant it no longer returned a boolean, it returned a Promise.
Oh God.
I'll be down voted by JavaScript lobby for saying this but I'll still say this.
Never use JavaScript on the server side. The amount of bugs that can happen is insane. JavaScript is just a specification with varying implementations across various versions.
A proper language like Java, Go, or Rust ensures that your code will have fewer logical bugs of this type.
https://ashishb.net/tech/javascript/
this issue was caused by a framework that's trying to do too much, relying on "magic" interfaces to supposedly reduce developer burden. the function is very unambiguously written and the language did nothing wrong
I also support using whatever language you and your team prefer when you can. that's the glory of backend: no restrictions on what you can run. but sometimes you need to write client software, and those are strictly easier to manage in the platform's native tongue: Swift, Kotlin, JS, and so on
My jaw dropped.
This reminds me of functions that rely on properties, but the properties could, themselves, be functions (in Swift, we call them "computed properties," and they are indistinguishable, on the outside, from stored properties).
C++ can also have a lot of tripwires, from overloaded operators (same with Swift, but people don't overload operators very often, in Swift. They compute properties, all the time, though).
This is why you should:
- Write functional tests, not unit tests
- Not use compilers or other systems that do a lot of black magic (like changing the type signature of your functions (!))
I almost never write single function unit tests. There's usually some subset of the codebase that's self contained that makes sense to be the "unit" you're testing, but it's almost always a handful of functions with a clear entry point.
My general rule is to never mock or remove anything that has no side effects from the execution path of a test, even if it's some utility function that's already tested in isolation in its own test suite - trying to isolate every little bit of behaviour to be tested separately is just a bunch of extra work for questionable benefit.
I still call these tests "unit tests", and I think a lot of people do also. But there are the dogmatic people to whom only a test covering a single function with all dependencies mocked out is a true unit test.
It also helps to not use languages with truthy / falsey values.
I think you're looking for `import 'server-only'` and not "use server";. Use server exposes functions as endpoints to the client. I.e. they are simply obfuscated api endpoints without boilerplate. Their main use is for mutations such as a form submission from the client.
Since pages are, by default, SSR, you don't need to have the server call out to itself to run an endpoint to check permissions. Instead, the server should just run the function.
I'm pretty sure Next does some behind the scenes black magic optimizations and doesn't actually make an API request over the wire, but it's still running through some layer of abstractions (causing it to be async) to run the function when instead it could simply be a synchronous function if implemented properly.
These abstractions make sense if you know how to use them properly, but I honestly blame Nextjs for the whole server action confusion. I remember when they first came out and seeing how almost every single question on /r/nextjs was about being confused about server actions. All these footguns and confusion to avoid some boilerplate... I'm not sure if they've improved it since, but I've moved to Svelte and haven't looked back.
Yes you are right and after our learning we changed the code to not use `use server` anymore for this kind of operations.
The documentation and tooling definitely got better and I don't think that such a situation is possible with the latest versions.
I just hope that some people who are still running the specific Next.js version won't fall into this as we did.
SSR = Server-side rendered
> It is written as if this only applies to functions you mark with use server and not the whole file.
The demonstration code illustrating the problem uses a file-level "use server" directive (and doesn't have other functions in the same file with the problem isOwner function); if they were using function level "use server" directives and it impacted a different function in the same file, I would say this is clearly surprising and unexpected (and even buggy) behavior, but this seems to be using a clearly documented feature and getting exactly what is advertised.
I don't know next.js, can someone explain how the client side can call a server function inside of an IF on the client side for security? It seems like there would be a trivial bypass of the security from the client side.
It opaquely makes a network call. You call it from the client-side and it abstracts away the network round-trip, but inside the function context you're running code on the server.
Under the hood it opens up an endpoint and the function calls it via a HTTP request.
always nice to read about these things. i like the note on 'all tests were green'. it sounds like the test of this function only test for the good case right? it should also test a false case? or am i missing something here?
We tested for both, the "happy" and the negative path. But the Javascript unit tests are run without the framework in between. So our function was returning expected results when run in isolation. Only when running it with Next.js the function became async which led to this dilemma.
> The snippet above was called as a server function. This is React's new way of calling server side code from the client side.
Tangential to the post, but mixing client-side and server-side code sounds like a recipe for disaster. There are already one too many services that perform authorization client-side, and I have a feeling making it harder to tell what runs where only makes the situation worse.
Doesn't the 'use client'/'use server' directives tell you this?
And still there are coders who prefer non-statically typed languages, tsk tsk...
Explain how static typing would avoid this problem.
If you use the standard typescript linter, it will fail if you pass a Promise to an if statement.
https://typescript-eslint.io/rules/no-misused-promises/
The problem was the framework's bundler was transforming those functions to return promises, a linter would need to understand next.js's specific transforms to catch this.
IMO, the idea of trying to blur the line between client and server is a big mistake. I worked on WebSocket frameworks in the Node.js space so I was also tempted but I completely abandoned this approach years ago. Though with Node.js, I do often reuse utility functions between client and server, I reject any framework which tries to hide the separation. I demand to have complete understanding of where the code is executing and how. I need to know what is being executed, where and how.
I also avoid technologies where the code I write is different from the code being executed. This is why I've been avoiding TypeScript. It performs code transformations which obfuscate my understanding of the logic which will be executed. With TS, the code I write is not the code which gets executed. This is scary to me. My code gets compiled into some messy junk-looking code and then that messy junk gets executed in ways I don't fully comprehend.
> I also avoid technologies where the code I write is different from the code being executed.
Not to be snarky, but as opposed to writing assembly? Where do you draw the line if you don't allow TypeScript (which is, with limited exceptions, only type erasure unless you specifically request polyfill), but allow other forms of compilation? Would you define JVM bytecode or IR as messy-looking junk code?
It's hard to see how a principled line could be drawn here.
You can use (ts 5.8 and above) a compiler flag `erasableSyntaxOnly` which enforces only TS features that can be dropped without modifying the underlying JavaScript. Unfortunately one of my favourite TS features, parameter properties, isn't erasable although it doesn't change runtime semantics.
My first thought was is JavaScript === case insensitive for string comps, because while I do use minimal JavaScript to enhance some web pages functionality it’s all basic vanilla JS.
But the answer is actually batshit crazy.
"The Power of 10 Rules" (2006, Gerard J. Holzmann)
https://en.wikipedia.org/wiki/The_Power_of_10:_Rules_for_Dev...
Generally wise advice. =3
> Let's look at the following Javascript snippet:
Title checks out, say no more.
> The snippet above was called as a _server_ function.
A semantic shift in what a function is.. lovely. Is this Javascript or Python?