fix: add seen weakset during mergeDeep

[abhishekg999]

Attempt to resolve: #1587

This doesn't handle the plugin deduplication for plugins of plugins which the above issue also points out, but this tries to fix the recursion in mergeDeep by maintaining a seen set.

Summary by CodeRabbit

• Bug Fixes

  • Prevented infinite recursion when merging objects with circular references; merging now safely detects and short-circuits already-seen sources and preserves shared circular structures.

• Tests

  • Added comprehensive tests for circular reference handling, covering nested cycles, shared references across branches, and deduplication in plugin-like scenarios.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Added cycle-guarding to mergeDeep by introducing an optional seen?: WeakSet<object> option, short-circuiting when a source object is already seen and cleaning up the set after recursion. Also added three unit tests validating circular and shared-reference scenarios.

Changes

Cohort / File(s)	Change Summary
mergeDeep cycle-guard src/utils.ts	Updated mergeDeep signature to accept seen?: WeakSet<object>. Added detection of previously-seen source objects with early return, add/remove of source to seen around recursion, and propagation of seen through recursive calls ({ skipKeys, override, mergeArray, seen }).
Tests for circular/shared refs test/units/merge-deep.test.ts	Added three tests: circular reference handling between objects, preserving shared references across branches, and deduplication/consistency in a plugin/module scenario with circular decorators.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Areas to inspect:
  • Correct use of WeakSet (membership semantics) and that objects are added/removed precisely once per call stack to avoid false-positive short-circuits.
  • Ensure cleanup (removal from seen) always runs (consider try/finally behavior).
  • Verify recursive propagation of the seen option doesn't break previous callers and types align with exports.
  • Confirm tests accurately assert identity/preservation of circular/shared references.

Possibly related PRs

• 1.4.6 patch #1411 — Modifies src/utils.ts's mergeDeep behavior (array-merge defaults and frozen-object guards); likely related at the mergeDeep implementation level.

🐰 I hopped through loops both wide and deep,
I marked the seen so merges sleep,
No more chases round the bend,
Shared roots stay friends,
Hooray — safe merges for every heap! 🥕✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: adding a WeakSet to track seen objects during mergeDeep recursion to prevent exponential complexity.
Linked Issues check	✅ Passed	The PR addresses the core issue by implementing cycle detection in mergeDeep using a WeakSet to prevent exponential recursion, matching the primary requirement.
Out of Scope Changes check	✅ Passed	All changes are focused on fixing the mergeDeep recursion issue; test additions validate the fix appropriately without introducing unrelated modifications.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

src/utils.ts (1)

105-110: Call-stack tracking pattern may still exhibit quadratic behavior for shared references.

The pattern of adding source to seen before recursion and deleting it after (line 110) creates a "call-stack tracker" rather than a "global seen set." This prevents infinite recursion but allows the same object to be merged multiple times when it appears in different branches of the object graph.

Consider this scenario:

const shared = { nested: { deep: { value: 1 } } }
const source = { a: shared, b: shared, c: shared }

With the current implementation, shared (and its nested objects) will be processed once for each branch (a, b, c), potentially leading to O(n²) behavior for wide graphs with many shared references.

This is still a significant improvement over the exponential/infinite recursion reported in issue #1587, and the approach is valid for preventing stack overflow. However, if performance issues persist with complex object graphs, consider using a WeakMap to cache merged results per source object.

test/units/merge-deep.test.ts (1)

90-110: Consider verifying circular structure is preserved.

The test validates that nested values are accessible but doesn't verify that the circular reference itself is preserved (i.e., that result.prop.toB.toA === result.prop).

Given the implementation's behavior of direct assignment when keys don't exist in the target, circular structures should be preserved in this scenario. Adding an assertion would make this explicit:

 expect(result.prop.x).toBe(1)
 expect(result.prop.toB?.y).toBe(2)
+expect(result.prop.toB?.toA).toBe(result.prop)

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fc68e41 and 1819ba3.

📒 Files selected for processing (2)

• src/utils.ts (2 hunks)
• test/units/merge-deep.test.ts (1 hunks)

🔇 Additional comments (2)

src/utils.ts (1)

61-67: Cycle detection approach effectively prevents infinite recursion.

The use of WeakSet<object> to track seen sources is appropriate and prevents infinite recursion when encountering circular references. WeakSet allows automatic garbage collection and provides O(1) lookup/insertion.

The early return on line 71 when seen.has(source) is true correctly short-circuits recursive processing for objects already on the call stack.

Also applies to: 71-72

test/units/merge-deep.test.ts (1)

123-162: Excellent real-world test coverage for the circular decorator scenario.

This test comprehensively validates the fix for issue #1587 by reproducing the plugin-with-circular-decorators scenario across multiple modules and routes. The assertions confirm that values remain consistent and accessible across all endpoints, demonstrating that the cycle detection prevents the exponential call count reported in the issue.