Skip to content

fix: preserve initial array length during stringify - #371

Merged
jordanbtucker merged 1 commit into
json5:mainfrom
Aurantium-47:fix/stringify-array-length
Oct 15, 2026
Merged

jordanbtucker merged 1 commit into
json5:mainfrom
Aurantium-47:fix/stringify-array-length

Conversation

@Aurantium-47

Copy link
Copy Markdown
Contributor

JSON5.stringify currently rereads an array's length after each element. A replacer that appends 2 while serializing [1] produces [1,2]; a replacer that truncates [1,2] to length 1 produces [1]. Native JSON.stringify produces [1] and [1,null], respectively.

Capture the array length before visiting its elements, consistent with the documented JSON-compatible API and ECMAScript's array serialization algorithm. Add regression tests for both mutations. Values are still read as each element is visited.

Validation:

  • Both new assertions fail with the original source and pass with the fix.
  • Full test suite: 180 passing, 100% coverage in all four metrics.
  • npm run lint passes.
  • Tested on Windows with Node 24.15.0. The local run disabled TAP 12's incompatible legacy ESM shim through a temporary external TAP configuration, and used LF for the CLI's shebang lint check. Neither workaround is included in the patch. The historical CI runtime matrix has not been run locally.

AI assistance: OpenAI Codex was used for investigation, implementation, and automated validation.

Assisted-by: OpenAI GPT-6 <noreply@openai.com>
@bybraveHQ bybraveHQ mentioned this pull request Oct 13, 2026
@jordanbtucker jordanbtucker added this to the v3.0.0 milestone Oct 14, 2026
@jordanbtucker jordanbtucker changed the title Preserve initial array length during stringify fix: preserve initial array length during stringify Oct 15, 2026
@jordanbtucker
jordanbtucker merged commit 391b290 into json5:main Oct 15, 2026
@jordanbtucker

Copy link
Copy Markdown
Member

Thanks for this fix! Would you be interested in writing a PR for #390 ? It's the parse-version of this PR.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants