Repository navigation
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What / Why
Repeatedly incrementing the same
SemVerinstance can stop advancing when a numeric prerelease identifier crosses the safe-integer storage boundary. The constructor keeps numeric prerelease identifiers at or aboveMAX_SAFE_INTEGERas strings, but the increment loop leaves them as numbers. After reaching 2^53, another increment can round back to the same number.Minimal reproduction:
1.2.3-beta.90071992547409911.2.3-beta.90071992547409911.2.3-beta.90071992547409921.2.3-beta.9007199254740991.01.2.3-beta.90071992547409921.2.3-beta.9007199254740991.11.2.3-beta.90071992547409921.2.3-beta.9007199254740991.2Change and scope
The four added source lines convert incremented identifiers at or above
MAX_SAFE_INTEGER(9007199254740991) to strings, matching the constructor. Subsequent calls use the existing appended-counter behavior and agree with incrementing reparsed versions.This is a rare boundary case involving extremely large counters and repeated use of the same instance. It does not add arbitrary-precision arithmetic or change the version comparison algorithm, main-version calculations, dependencies, or tooling.
Regression coverage checks plain numeric identifiers, prefixed identifiers, and numeric identifiers followed by text, with four consecutive increments for each. Each step must increase precedence, match reparsed behavior, and preserve constructor representation.
Validation
On macOS / Node 22.23.0, npm 10.9.8, against base
6e05b7637396ac66522cff8731f07cfe0ef49a29:git diff --checkpass.npm test -- --reporter=tersecommand still exits 1 at the finaltemplate-oss-check. It reports existing template mismatches in.github/dependabot.yml,.github/settings.yml,.github/workflows/ci.yml,.github/workflows/codeql-analysis.yml, and.github/workflows/release.yml.npm run postlintin a clean local clone of the base commit, using the same dependencies, reproduces the same five mismatches with a clean worktree. Those automatically maintained files are excluded from this patch, in accordance with the contribution guidelines.References and assistance
Related to #880's large numeric identifier topic, but that PR changes comparison precision in
internal/identifiers.js; this change fixesSemVer.inc()and does not depend on it. No matching increment fix was found in the pre-submission searches.Prepared with Codex assistance and submitted as a draft for review. Remote CI and maintainer review results are not included in the local validation claims.