Skip to content

Sync upstream BridgeAR/safe-stable-stringify (bdd585f): typed array, maximumDepth, and negative space fixes plus perf wo - #3

Merged
kitschpatrol merged 11 commits into
mainfrom
fork-tender/BridgeAR-safe-stable-stringify
Nov 2, 2026
Merged

kitschpatrol merged 11 commits into
mainfrom
fork-tender/BridgeAR-safe-stable-stringify

Conversation

@claude

@claude claude Bot commented Nov 2, 2026

Copy link
Copy Markdown

Summary

Merges 10 upstream commits (merge base 8a02137, upstream tip bdd585f). Upstream's changes are ported into the fork's TypeScript source (src/index.ts) and Vitest suite (test/index.test.ts). The fork keeps its ESM-only package, dist/ exports, and the default export plus the named stringify and configure exports, stringify.configure, stringify.stringify, and the public types. Upstream's default circular marker change is not adopted (see below).

What's new upstream, and what was ported

Upstream commit Status in this fork
b8aad14 fix: do not throw on a negative number space (BridgeAR#65) Ported. stringify(x, null, -1) used to throw a RangeError from ' '.repeat(-1). It now produces no indentation, like JSON.stringify.
a8df6a1 fix: enforce maximumDepth for nested objects with array replacers (BridgeAR#64) Ported. With an array replacer, objects past maximumDepth now serialize as "[Object]" instead of being expanded. The existing maximumDepth config test was updated to the new expectation ({"a":{"a":"[Array]","b":"[Object]"}}), and a maximumDepth: 1 case was added. This changes output for array-replacer users who set maximumDepth.
25b3110 fix(typed-array): serialize non-finite floating-point values (BridgeAR#73) Ported. Float16Array/Float32Array/Float64Array entries that are NaN/Infinity used to produce invalid JSON (NaN). They now serialize as null, and strict mode throws for them. Typed arrays are detected by their intrinsic type through the %TypedArray%.prototype[Symbol.toStringTag] getter, so subclasses, cross-realm arrays, and spoofed Symbol.toStringTag values are handled. Detection is browser-safe: ArrayBuffer.isView + Reflect.get, with no Node APIs.
e8e5b3b perf(arrays): defer index string conversion (BridgeAR#75) Ported. Array indexes are passed as numbers and converted to strings only when a toJSON method needs them.
8ed2781 perf(sort): bound insertion sort work (BridgeAR#76) Ported into sortStrings.
bdd585f perf: reduce primitive serialization overhead (BridgeAR#77) Ported. Adds a stringifyPrimitive fast path and splits object handling out of the per-value switch in every mode. Safe mode wraps the new serializer the same way upstream does. The function-replacer path uses the unwrapped primitive serializer to avoid double wrapping. This includes upstream's runtime-dependent native-escape threshold (128 chars on Node.js 22+, otherwise 5000). It's guarded with typeof process === 'object' and optional reads, so browsers and partial process shims fall back to 5000. Output is unchanged; only which code path escapes the string differs.
318cf4f feat(circular)!: include ancestor depth in default markers (BridgeAR#70) Not adopted (fork behavior preserved). Upstream's breaking change makes the default circular marker "[Circular: N]". The fork keeps "[Circular]" so existing output doesn't change. Custom circularValue strings, including '[Circular: 0]', still work. Upstream's circular regression tests were ported with "[Circular]" expectations. A readme bullet in the fork's Changes list documents this. Maintainer decision: if you want to track upstream 3.x output exactly, this is the one remaining behavioral divergence.
c05878f feat(modules)!: share ESM and CommonJS exports (BridgeAR#72) Not adopted. It's a CommonJS/ESM dual-package fix: wrapper.cjs, an ESM wrapper, a globalThis symbol registry, and export = declarations. The fork is ESM-only, so none of it applies. The wrapper.cjs, esm/wrapper.mjs, index.d.ts, test-modules.js and test-fixtures/* files were dropped.
5dd026c docs(circular): clarify JSON escaping for custom markers (BridgeAR#74) Upstream readme docs only. The fork's readme links to the upstream readme, so nothing was copied.
0c2253c Fix merge conflict in readme (BridgeAR#63) Upstream readme only; not applicable.

Conflict resolutions

  • Upstream-only files the fork deleted (index.js, index.d.ts, esm/wrapper.mjs, test.js, benchmark.js, compare.js, .npmignore), and new upstream files (wrapper.cjs, benchmark-utils.js, test-modules.js, test-fixtures/*): kept deleted or not added. The implementation and test changes were ported by hand into src/index.ts and test/index.test.ts. The upstream benchmark scripts were not adopted; the fork's Vitest bench suite and stored baselines are unchanged.
  • package.json: kept the fork's version. Upstream's name, version (3.0.0), CJS exports, tap/standard scripts, and devDependencies were not adopted. No dependency changes, so pnpm-lock.yaml is untouched and pnpm ci passes with the existing lockfile.
  • tsconfig.json, .gitignore: kept the fork's version. Upstream's changes only cover its JS/CJS/tap layout.
  • readme.md: kept the fork's readme. Added one bullet to the fork's Changes list about keeping the "[Circular]" default. The mdat-generated sections weren't touched, and ksc-mdat lint passes.
  • changelog.md: kept the fork's link formatting in the v1.1.0 hunk. Upstream's new entries were merged into the mirrored "Upstream changelog" section as upstream wrote them, including the circular marker and CJS notes, which describe upstream rather than this fork. They were tightened into a single list for markdown lint.

Dropped protected-path changes

  • .github/workflows/test.yml (upstream: Node 16 removed, Node 26 added, checkout and setup-node actions bumped to v7). The fork deleted this workflow and uses its own ci.yml, so it stays deleted.

Tests added

These upstream regression tests were ported into test/index.test.ts, adapted to the existing Tape-compatible adapter and to the "[Circular]" default:

  • native-escape threshold vs. output equality across stubbed process shapes, using a module reload with vi.stubGlobal
  • non-finite float typed arrays, including strict and maximumBreadth
  • intrinsic typed-array type (subclass, cross-realm via node:vm, spoofed tag)
  • integer and bigint typed arrays
  • circular markers in every mode
  • circular references vs. maximumDepth
  • toJSON and function-replacer circular references
  • explicit circularValues
  • toJSON string keys in every mode
  • primitive formatting in every mode
  • safe-mode primitive escaping failures
  • strict/safe/bigint primitive failures
  • replacer/toJSON call order
  • sort boundaries
  • array replacers being read only for object roots
  • negative space

All existing fork tests are kept. The test count went from 88 to 107.

Verification

pnpm ci && pnpm build && pnpm lint && pnpm test passes:

  • tsdown build, attw (esm-only) and publint: clean
  • ksc lint: 8/8
  • Vitest: 107/107

Benchmarks

I ran pnpm bench:compare on the CI runner. I didn't commit the regenerated test/benchmarks/compare.json or touch the stored baselines.

  • Against upstream safe-stable-stringify 3.0.0 in the same run (npm 3.0.0 already includes these perf commits): the synced fork ranges from 0.97x to 1.07x of upstream throughput. For example: simple/simple object 1.01x, simple/circular 1.07x, function replacer/circular 1.03x, array replacer/circular 1.06x, indentation/deep 0.99x, implementation comparison 0.98x.
  • Against the stored baselines: every case is about 0.55-0.75x of the baseline. Upstream drops by the same amount in the same run, so this is the difference between the CI runner and the M3 Max the baselines were recorded on, not a regression.
  • Not measured: a same-machine before/after against pre-sync main. That would need a second worktree, which this run wasn't permitted to create. Please run pnpm bench:compare locally against the stored baselines before merging.

Review notes

  • Behavior changes users can see: array replacer + maximumDepth now truncates nested objects; non-finite float typed-array values become null or throw in strict; a negative space no longer throws.
  • src/index.ts now references process behind a typeof guard. This is feature detection, not a runtime dependency, but some legacy bundlers can inject a process polyfill when they see that identifier.

10 new commit(s) from BridgeAR/safe-stable-stringify · upstream compare

Verification Result
Baseline (main) pass
Mechanical pass on this branch skipped

See the report above for post-resolution verification.

Automated by fork-tender · outcome: resolved · confidence: medium

spokodev and others added 11 commits October 12, 2026 04:39
`JSON.stringify` treats any numeric `space` below 1 as no indentation, but
`stringify` threw on it:

    JSON.stringify({ a: 1 }, null, -1) // '{"a":1}'
    stringify({ a: 1 }, null, -1)      // RangeError: Invalid count value: -1

The README documents the spacer as behaving "the same as with
JSON.stringify()". `space` was clamped to a maximum of 10 but not a minimum
of 0, so a negative count reached `String.prototype.repeat`, which throws
for counts <= -1. Clamp the lower bound to 0.
Looks like this was accidentally introduced in BridgeAR@132e76c
Circular references to different ancestors currently produce the same marker, which hides where each cycle closes. The new marker identifies the referenced ancestor's depth, starting at 0 for the root.

BREAKING CHANGE: The default marker changes from `[Circular]` to `[Circular: N]`. Set `circularValue: '[Circular]'` to retain the previous output.
…R#73)

* Serialize non-finite typed-array values consistently

Fixes: BridgeAR#68

* perf(typed-array): inline floating-point checks

A formatter closure adds finite checks to integer arrays and an extra call for floating-point values. Inline checks retain direct conversion for integer and BigInt arrays.

---------

Co-authored-by: Ferit <frtcm11@gmail.com>
Array indexes only need strings when they reach toJSON or function replacer
callbacks. A four-number array uses 86 ns instead of 107 ns on Node 26.3.1
(V8 14.6.202.34-node.20), a 20% CPU reduction. Measurements use 50,000 warmup
calls and seven alternating trials, with the fastest and slowest removed.

Drive-by fix: Mark intentional invalid-option assertions for TypeScript.
Disordered objects with up to 200 keys can spend most of their serialization
time shifting keys. A shuffled 200-key object uses 16.6 us instead of 45.6 us
on Node 26.3.1 (V8 14.6.202.34-node.20), a 64% CPU reduction. Measurements
use 50,000 warmup calls and seven alternating trials, with the fastest and
slowest removed. Nearly ordered keys retain the insertion path.

Drive-by fix: Mark intentional invalid-option assertions for TypeScript.
The implementation is cached after the first load, so removing its global
export leaves the other wrapper without an export. Retain a version-specific
symbol so ESM imports, CommonJS requires, and independent copies of one
version receive the same function.

BREAKING CHANGE: Require Node.js 18 or later.
Primitive array elements and object properties enter recursive serializers
even though they need no traversal. Share primitive formatting across
serialization modes while preserving toJSON ordering and per-value error
handling in safe mode.

Native string escaping becomes faster at shorter string lengths on
Node.js 22 and later. Select the cutoff once at module load so older
runtimes retain their existing fast path.
…der/BridgeAR-safe-stable-stringify

Fork-Tender: v1
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@claude claude Bot added the fork-tender Automated upstream sync by fork-tender label Nov 2, 2026
@kitschpatrol
kitschpatrol merged commit e756f37 into main Nov 2, 2026
4 of 5 checks passed
@kitschpatrol
kitschpatrol deleted the fork-tender/BridgeAR-safe-stable-stringify branch November 2, 2026 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fork-tender Automated upstream sync by fork-tender

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants