fix(parser): remove non-decimal prefix from bigint - #18215
Conversation
JLHwung
added
PR: Bug Fix 🐛
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/62087 |
|
commit: |
There was a problem hiding this comment.
🟡 Changes recommended
The updated diff formatting can throw when formatting undefined/function values because serialize assumes JSON.stringify returns a string.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR fixes ESTree BigInt literal serialization so that the bigint field is emitted as a base-10 string for zero-valued non-decimal literals (e.g. 0x0n → "0"), and adds fixtures covering additional edge cases. It also adjusts test diff rendering to use the suite’s custom serializer so BigInt-containing structures can be formatted safely in failure messages.
Changes:
- Update
parseBigIntLiteralin the ESTree parser plugin to avoid the falsy0nfallback and always stringify non-null BigInt values in decimal. - Update fixture diff formatting to use the custom
serializehelper (so BigInt-containing values don’t break message generation). - Add ESTree BigInt fixtures for zero-valued non-decimal forms, numeric separators, unary minus, and an invalid BigInt literal.
File summaries
| File | Description |
|---|---|
| packages/babel-parser/src/plugins/estree.ts | Fix BigInt literal bigint field computation to avoid radix-prefixed output for zero. |
| packages/babel-parser/test/helpers/difference.js | Switch diff value formatting to use custom serializer for BigInt-friendly output. |
| packages/babel-parser/test/helpers/serialization.js | Update comments describing where BigInt extended serialization is used. |
| packages/babel-parser/test/fixtures/estree/bigInt/octal-zero/input.js | Add octal zero BigInt input fixture. |
| packages/babel-parser/test/fixtures/estree/bigInt/octal-zero/output.extended.json | Add expected ESTree output for octal zero BigInt (decimal bigint). |
| packages/babel-parser/test/fixtures/estree/bigInt/invalid-float/input.js | Add invalid BigInt literal input fixture. |
| packages/babel-parser/test/fixtures/estree/bigInt/invalid-float/output.json | Add expected error output for invalid BigInt literal. |
| packages/babel-parser/test/fixtures/estree/bigInt/hex/input.js | Add hex BigInt input fixture. |
| packages/babel-parser/test/fixtures/estree/bigInt/hex/output.extended.json | Add expected ESTree output for hex BigInt (decimal bigint). |
| packages/babel-parser/test/fixtures/estree/bigInt/hex-zero/input.js | Add hex zero BigInt regression input fixture. |
| packages/babel-parser/test/fixtures/estree/bigInt/hex-zero/output.extended.json | Add expected ESTree output for hex zero BigInt (decimal bigint: \"0\"). |
| packages/babel-parser/test/fixtures/estree/bigInt/hex-negative-zero/input.js | Add unary-minus hex zero BigInt input fixture. |
| packages/babel-parser/test/fixtures/estree/bigInt/hex-negative-zero/output.extended.json | Add expected ESTree output for unary-minus hex zero BigInt. |
| packages/babel-parser/test/fixtures/estree/bigInt/binary-double-zero-with-numeric-separator/input.js | Add binary BigInt with numeric separator input fixture. |
| packages/babel-parser/test/fixtures/estree/bigInt/binary-double-zero-with-numeric-separator/output.extended.json | Add expected ESTree output for binary BigInt with separators (decimal bigint). |
Review details
- Files reviewed: 15/15 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| : type === "bigint" | ||
| ? `${value}n` | ||
| : Object.is(value, -0) | ||
| ? "-0" | ||
| : JSON.stringify(value); | ||
| : serialize(value)[1]; |
There was a problem hiding this comment.
Good catch, this is fixed in the latest commit.
There was a problem hiding this comment.
🟢 Approval recommended
The functional fix is small and well-covered by new regression fixtures, with only a minor doc-comment accuracy nit noted.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
packages/babel-parser/test/helpers/serialization.js:14
- The updated comment claims BigInt is only serialized in the ESTree
bigIntfixtures, but this helper is also used by other fixtures that serialize BigInt (e.g.es2020/bigintand TypeScriptliteral-bigint). This makes the comment misleading/outdated.
- Files reviewed: 15/15 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🟢 Approval recommended
The change correctly fixes BigInt 0n serialization in ESTree output and is backed by targeted fixture coverage, with the test-runner adjustments preventing known failure-reporting crashes.
Review details
- Files reviewed: 15/15 changed files
- Comments generated: 0 new
- Review effort level: Lite
This change ensures that when the AST contains BigInt/RegExp, the test runner can still write to output.extended.json. Previously it will throw a TypeError because the builtin JSON.stringify does not know how to serialize bigint
For Babel 8, the minimal supported Node.js version already supports BigInt.
This PR fixes the bug revealed in the PR #18203, which was closed due to violation to our AI policy.
When working on this PR, we also spot a bug in the fixture test runner.