-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SyntaxError: Invalid or unexpected token 50.toString() #16040
Comments
Fixed in the PR #16041 |
Ha, that's very intriguing. What kind of bundler/minifier are you using? Tactical cc to @filipsobol, as we start to bundle ourselves in #15502. |
I use Terser (Stack: Nuxt.js 3, Vue 3, Vite, Terser). Disabling these two options of Terser makes ckeditor build correctly:
But in general, I had to disable these two options only for two rows in ckeditor lib which I've fixed in my PR. If I disable these options, the expression |
We use terser to minify bundles for the new installation methods (#15502) and it correctly converts // Input
const test = 50;
console.log(test.toString());
// Output
console.log(50..toString()); @maxmartynov Are you using the latest version of terser? Out of curiosity, I also checked swc and (when compression is enabled) it converts |
Hmm, with the options mentioned by @maxmartynov
the live REPL also produces working code:
It looks like a Terser issue (maybe older version produced wrong results), or maybe something else causes problems in your environment? TBH I don't see this as a bug on our side 🤔 Changing working code to some other implementation just because minification crashes it, looks like the tail wagging the dog. |
TLDR; Full story I use [email protected]. Perhaps this problem will go away with new installation methods (#15502)
I use (Nuxt.js uses under the hood) [email protected]. The latest version is 5.29.2.
I confirm that for me disabling the flags I've created this issue mostly to point out the fact that (warn: IMHO) using The reason why I've created this issue is to report the problem that appears in runtime only and happens NOT IN 100% CASES. My users got the error |
Fix (ckbox): Use a safer way to convert numbers to strings to avoid issues with some bundlers. Closes #16040
📝 Provide detailed reproduction steps (if any)
Some algorithms of minification turn code like this
ITEMS_PER_REQUEST.toString()
to50.toString()
- substitute the variable's value. This leads to the error:SyntaxError: Invalid or unexpected token
.The place where I personally have this problem
ckeditor5/packages/ckeditor5-ckbox/src/ckboxutils.ts
Line 228 in 54eae6a
❓ Possible solution
Following the specification of
Number.prototype.toString()
, using another way of stringifying numbers will be safer. I suggest usingString(number)
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/toString
If you'd like to see this fixed sooner, add a 👍 reaction to this post.
The text was updated successfully, but these errors were encountered: