When 'perfect' code fails
30 points by mrk
30 points by mrk
The post doesn't blame Javascript's loose type-system, but I think that's also a place this could have been avoided.
In stronger typed languages, if (isOwner(...)) would be compiler error, if condition must be Bool but was type Promise<Bool>.
Sure "truthy" and "falsy" values let you save a ton of typing for "null" checks and are convenient sometimes, but this is a great example where a runtime exception or typescript compiler error would have been a way better outcome than "eh, I guess a function value is true".
Personally I do blame Javascript for this too. But you are right, I might not have been clear enough that not only Next.js (React) magic were at fault here but also Javascripts loose typesystem and especially in combination.
Javascripts loose typesystem
Surely Typescript would have prevented this, or not?
It would, but only if using strict (or the specific rule that implies, I guess):
$ cat test.ts
async function isOwner(): Promise<boolean> { return false; }
if (isOwner()) { console.log("access granted"); }
$ tsc --lib es2024,dom test.ts && node test.js
access granted
$ tsc --lib es2024,dom --strict test.ts && node test.js
test.ts:2:5 - error TS2801: This condition will always return true since this 'Promise<boolean>' is always defined.
2 if (isOwner()) { console.log("access granted"); }
~~~~~~~~~
test.ts:2:5
2 if (isOwner()) { console.log("access granted"); }
~~~~~~~~~
Did you forget to use 'await'?
Found 1 error in test.ts:2
the problem is, that the functions signature was synchronous. it was just treated as an async function during runtime or converted to async after building it. newer versions of Next.js do catch this though.
In TypeScript there is a linter configuration that disallows unawaited promises. I’m not familiar with the setup the author is using with React and Next.js where it gets converted into an async function behind the scenes; I’m not sure if the linter would pick it up since it seems the function isn’t marked as async.
Read more about the linter config here https://stackoverflow.com/a/64380428
At work this is required on all TS repos.
there is a linter configuration
This reminds me of PHP which "does not have to be insecure if you know what you're doing".
I don't know if this would work since this is not really a typescript issue. Newer versions of Next.js do catch this behaviour though.
> Build error occurred
Error: Turbopack build failed with 1 errors:
./lib/server.ts:3:14
Ecmascript file had an error
1 | "use server";
2 |
> 3 | export const isOwner = (userMail: string, ownerMail: string) => {
| ^^^^^^^
4 | return userMail === ownerMail;
5 | };
6 |
Server Actions must be async functions.
Unfortunately when you play rpc games you win rpc prizes. I’m sorry react next.js is peddling this.
See:
I'm not sure this is to be blamed on the React team. They push some silly ideas, but silently converting sync functions to async is the kind of nightmare that I wouldn't expect from anybody but NextJS.
Thank you for the links, I will give them a look and maybe include them in the article if they fit.
An excellent little blog post, I enjoyed it.
I'm going to tease a little bit: you have to admit the headline would hit differently if it was "When React code fails" :P
But no, I shouldn't tease, I support your original headline: that function truly looks simple-therefore-perfect, and it's surprising[1] that React managed to fuck it up.
[1] Surprising, but perhaps inevitable? [2] Which makes it a good plot twist, so no wonder I enjoyed your post.
[2] For 'surprising, yet inevitable' I credit Howard Tayler of Writing Excuses and Schlock Mercenary.
Isn't the lesson here that you should know your framework on a deep level?
This reminds me of Rails, which actualy has a pretty steep learning curve and in the beginning all of the magic feels more like an obstacle and full of footguns, but...once you actually read the manual and docs, it makes sense and the magic becomes productive, not an obstacle (dito for Bash).
Maybe the issue here is that next.js makes everything so smooth in general, that most user don't see a need to read the docs and then get caught by magic? Haven't used it in years, so maybe someone with more next.js experience can chime in on this.
Yes, this is the ideal scenario and we all should strive to do so. Unfortunately when server actions were released the documentation was sparse and also not very clear. I would argue that the documentation still lacks today. Fortunately newer versions of Next.js do catch this bug during build.