From f2b903fceaa0a02cefd77f6b4b123c679605aae9 Mon Sep 17 00:00:00 2001 From: AKIBUZZAMAN AKIB Date: Wed, 6 May 2026 00:05:17 +0600 Subject: [PATCH] refactor(utils): use WeakSet for cycle detection in toJSONObject (#10832) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor(utils): use WeakSet for cycle detection in toJSONObject Replace fixed-size Array(10)+indexOf stack with WeakSet using add-on-descent/delete-on-ascent to preserve path (ancestor-only) semantics. The delete on ascent is the critical correctness detail — a naive WeakSet swap without delete treats shared DAG siblings as already-seen and drops them as undefined, the exact regression introduced in #7230. Perf is a secondary benefit: WeakSet.has is O(1) amortised vs O(n) Array.indexOf, measurable on deeply nested objects. - Compress verbose 3-line comment to one line (references #7230) - Group new DAG regression tests under describe('cycle / DAG handling') Fixes #10807 * chore: added should serialize non-cyclic structures deeper than the old Array(10) cap --------- Co-authored-by: AKIBUZZAMAN AKIB <> Co-authored-by: Jay --- lib/utils.js | 15 ++++++------ tests/unit/utils.test.js | 50 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 7 deletions(-) diff --git a/lib/utils.js b/lib/utils.js index ab6b11f4..a869a938 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -763,11 +763,11 @@ function isSpecCompliantForm(thing) { * @returns {Object} The JSON-compatible object. */ const toJSONObject = (obj) => { - const stack = new Array(10); + const visited = new WeakSet(); - const visit = (source, i) => { + const visit = (source) => { if (isObject(source)) { - if (stack.indexOf(source) >= 0) { + if (visited.has(source)) { return; } @@ -777,15 +777,16 @@ const toJSONObject = (obj) => { } if (!('toJSON' in source)) { - stack[i] = source; + // add-on descent / delete-on-ascent: preserves path semantics, so DAG nodes serialise at every occurrence (see #7230). + visited.add(source); const target = isArray(source) ? [] : {}; forEach(source, (value, key) => { - const reducedValue = visit(value, i + 1); + const reducedValue = visit(value); !isUndefined(reducedValue) && (target[key] = reducedValue); }); - stack[i] = undefined; + visited.delete(source); return target; } @@ -794,7 +795,7 @@ const toJSONObject = (obj) => { return source; }; - return visit(obj, 0); + return visit(obj); }; /** diff --git a/tests/unit/utils.test.js b/tests/unit/utils.test.js index 7979de81..a8d15c27 100644 --- a/tests/unit/utils.test.js +++ b/tests/unit/utils.test.js @@ -86,6 +86,56 @@ describe('utils', () => { JSON.stringify({ x: 1, y: 2, obj: { ok: 1 } }) ); }); + + describe('cycle / DAG handling', () => { + it('should serialize a shared sibling object at every occurrence (DAG, not cycle)', () => { + const shared = { val: 42 }; + const source = { x: shared, y: shared }; + + const result = utils.toJSONObject(source); + + // Both branches must serialize — shared reference is not a cycle + assert.deepStrictEqual(result, { x: { val: 42 }, y: { val: 42 } }); + }); + + it('should serialize a shared sibling array at every occurrence (DAG, not cycle)', () => { + const shared = [1, 2, 3]; + const source = { a: shared, b: shared }; + + const result = utils.toJSONObject(source); + + assert.deepStrictEqual(result, { a: [1, 2, 3], b: [1, 2, 3] }); + }); + + it('should serialize shared sibling that itself contains a self-cycle', () => { + const shared = { v: 1 }; + shared.self = shared; // self-cycle inside the shared node + const source = { x: shared, y: shared }; + + const result = utils.toJSONObject(source); + + // The self-cycle is stripped, but both x and y must be serialized + assert.deepStrictEqual(result, { x: { v: 1 }, y: { v: 1 } }); + }); + + it('should serialize non-cyclic structures deeper than the old Array(10) cap', () => { + // The previous implementation used a fixed-size Array(10) for path tracking. + // A non-cyclic chain deeper than 10 levels must serialise end-to-end. + let leaf = { v: 'leaf' }; + let source = leaf; + for (let i = 0; i < 25; i++) { + source = { next: source }; + } + + const result = utils.toJSONObject(source); + + let cursor = result; + for (let i = 0; i < 25; i++) { + cursor = cursor.next; + } + assert.deepStrictEqual(cursor, { v: 'leaf' }); + }); + }); }); describe('Buffer RangeError Fix', () => {