close
Skip to content

util.inspect: fix numericSeparator for scientific notation numbers#62987

Open
AmSach wants to merge 2 commits intonodejs:mainfrom
AmSach:fix/inspect-sci-not-numeric-sep
Open

util.inspect: fix numericSeparator for scientific notation numbers#62987
AmSach wants to merge 2 commits intonodejs:mainfrom
AmSach:fix/inspect-sci-not-numeric-sep

Conversation

@AmSach
Copy link
Copy Markdown

@AmSach AmSach commented Apr 27, 2026

Fixes #62981

When numericSeparator: true is set, util.inspect produces corrupted output like '1e-.1e-_7' for numbers in scientific notation (e.g. 1e-7).

The fix adds an early return for scientific notation strings before the decimal-split + numeric separator logic runs.

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. util Issues and PRs related to the built-in util module. labels Apr 27, 2026
Copy link
Copy Markdown
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests are needed.

Comment thread lib/internal/util/inspect.js Outdated
return base;
return ctx.stylize(base, StringPrototypeToLowerCase(type));
if (constructor !== null) {
const superName = ObjectGetPrototypeOf(value).name;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why would a [[Prototype]] object necessarily have a .name? and why would a class-related change belong in a PR fixing number display?

Comment on lines -2417 to +2431
ctx.indentationLvl += 2;
let i = 0;
ctx.indentationLvl += 2;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pls revert this, it's unrelated

Copy link
Copy Markdown
Author

@AmSach AmSach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review. You're right that tests are needed - I'll add comprehensive test cases for numericSeparator with scientific notation numbers.

Regarding the unrelated changes (prototype extension logic at lines 1541-1554, and the StringPrototypeIncludes -> StringPrototypeSlice change at line 1837): those are oversights from an auto-formatting or IDE action that shouldn't have been committed. I'll revert those and keep only the numericSeparator fix.

I'll also remove the duplicate StringPrototypeIncludes(numberString, 'e') checks at lines 2211-2214 and 2222-2225 - those are redundant.

When numericSeparator: true is set, util.inspect produces corrupted output
like '1e-.1e-_7' for numbers in scientific notation.

The fix adds an early return in formatNumber() for scientific notation
numbers (containing 'e') before the decimal-split + numeric separator
logic runs.

Fixes nodejs#62981
@AmSach AmSach force-pushed the fix/inspect-sci-not-numeric-sep branch from c4cb7e2 to 167e59e Compare April 27, 2026 20:15
@ljharb
Copy link
Copy Markdown
Member

ljharb commented Apr 27, 2026

(note this is also a duplicate of #62983)

Move the scientific notation check to the start of formatNumber() to fix
corrupted output like '1e-.1e-_7' instead of '1e-7'.

Previously the check was placed AFTER integer === number which caused it
to be checked too late in the flow, allowing scientific notation numbers
to reach the numeric separator logic incorrectly.

Also adds comprehensive test cases for scientific notation with
numericSeparator: true covering various exponent formats.

Fixes nodejs#62981
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ci PRs that need a full CI run. util Issues and PRs related to the built-in util module.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

util.inspect: incorrect numericSeparator formatting for numbers in scientific notation (e.g. 1e-7)

3 participants