fix: merge enumerable symbol properties

[kricsleo]

resolves #145

Summary by CodeRabbit

• New Features

  • Symbol-keyed properties are now recognized and merged alongside string-keyed properties, preserving enumerable symbol entries during merges.

• Tests

  • Added test coverage validating symbol property merging and TypeScript type inference for merged results.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 82340b13-018d-49ac-875b-75921942692f

📥 Commits

Reviewing files that changed from the base of the PR and between 2a1cdf1 and fe5bd09.

📒 Files selected for processing (2)

• src/defu.ts
• test/defu.test.ts

✅ Files skipped from review due to trivial changes (1)

• test/defu.test.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• src/defu.ts

---

📝 Walkthrough

Walkthrough

The _defu implementation now centralizes merge logic into an internal merge(key) helper and iterates both string keys and enumerable symbol keys (from Object.getOwnPropertySymbols) to apply the same null/undefined filtering, merger handling, and recursive merge behavior.

Changes

Cohort / File(s)	Summary
Core merge logic src/defu.ts	Introduced an internal merge(key) helper; replaced inline property loop with calls to merge for Object.keys(baseObject) and separately for enumerable symbols via Object.getOwnPropertySymbols(baseObject). Preserves existing array/plain-object/merger checks.
Tests test/defu.test.ts	Added a test that verifies merging preserves symbol-keyed properties and that TypeScript types reflect symbol index signatures for the merged result.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 I nibble through keys both bright and dim,
Symbols and strings now dance on a whim,
merge() hums softly, joins each gate,
No proto tricks, no keys left late.
A happy hop for every trait. 🥕✨

🚥 Pre-merge checks | ✅ 5

✅ 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 'fix: merge enumerable symbol properties' directly describes the main code change—adding enumerable symbol property merging to the defu function.
Linked Issues check	✅ Passed	The PR successfully implements the fix for issue #145: symbol-keyed properties are now iterated via Object.getOwnPropertySymbols, enumerable symbols are filtered, and merged through the same merge helper as string keys, preserving symbol values and enabling array concatenation for symbol-keyed arrays.
Out of Scope Changes check	✅ Passed	All changes are scoped to implementing symbol property merging: refactoring the merge loop into a helper function and adding symbol iteration/filtering. No unrelated modifications are present.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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]

🧹 Nitpick comments (1)

test/defu.test.ts (1)

34-40: Test covers basic symbol merging but misses array concatenation scenario from the issue.

The linked issue (#145) specifically mentions that "arrays under symbol keys not being concatenated as expected." Consider adding test cases to verify:

1. Array concatenation with symbol keys
2. Nested object merging with symbol keys

🧪 Suggested additional test coverage

   it("should copy Symbol properties", () => {
     const a = Symbol("a");
     const b = Symbol("b");
     const result = defu({ [a]: "a" }, { [b]: "b" });
     expect(result).toEqual({ [a]: "a", [b]: "b" });
     expectTypeOf(result).toMatchTypeOf<{ [k: symbol]: string }>();
   });
+
+  it("should concat arrays with Symbol keys", () => {
+    const key = Symbol("arr");
+    const result = defu({ [key]: ["a", "b"] }, { [key]: ["c", "d"] });
+    expect(result).toEqual({ [key]: ["a", "b", "c", "d"] });
+  });
+
+  it("should merge nested objects with Symbol keys", () => {
+    const key = Symbol("nested");
+    const result = defu({ [key]: { a: 1 } }, { [key]: { b: 2 } });
+    expect(result).toEqual({ [key]: { a: 1, b: 2 } });
+  });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/defu.test.ts` around lines 34 - 40, Extend the existing symbol-property
test for defu by adding two assertions: (1) verify arrays under symbol keys are
concatenated (e.g., call defu with { [sym]: [1] } and { [sym]: [2] } and expect
[1,2]) to reproduce the `#145` scenario, and (2) verify nested merging when symbol
keys point to objects (e.g., defu({ [sym]: { x: [1] } }, { [sym]: { x: [2] } })
yields concatenated arrays and merged objects); update or add tests adjacent to
the "should copy Symbol properties" spec using the same symbol identifiers (a, b
or new sym) and assert both structural equality and types via expectTypeOf as
done currently.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@test/defu.test.ts`:
- Around line 34-40: Extend the existing symbol-property test for defu by adding
two assertions: (1) verify arrays under symbol keys are concatenated (e.g., call
defu with { [sym]: [1] } and { [sym]: [2] } and expect [1,2]) to reproduce the
`#145` scenario, and (2) verify nested merging when symbol keys point to objects
(e.g., defu({ [sym]: { x: [1] } }, { [sym]: { x: [2] } }) yields concatenated
arrays and merged objects); update or add tests adjacent to the "should copy
Symbol properties" spec using the same symbol identifiers (a, b or new sym) and
assert both structural equality and types via expectTypeOf as done currently.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6bca44bd-c2e0-4061-b343-f115e50534d1

📥 Commits

Reviewing files that changed from the base of the PR and between 89df6bb and 2a1cdf1.

📒 Files selected for processing (2)

• src/defu.ts
• test/defu.test.ts

[codecov]

Codecov Report

❌ Patch coverage is 90.90909% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 98.07%. Comparing base (70cffe5) to head (fe5bd09).
⚠️ Report is 11 commits behind head on main.

Files with missing lines	Patch %	Lines
src/defu.ts	90.90%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main     #146       +/-   ##
===========================================
+ Coverage   45.73%   98.07%   +52.33%     
===========================================
  Files           4        2        -2     
  Lines         223       52      -171     
  Branches       35       19       -16     
===========================================
- Hits          102       51       -51     
+ Misses        119        1      -118     
+ Partials        2        0        -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[kricsleo]

I've resolved the conflicts.