Conversation
WalkthroughAdds an internal helper that filters undefined mappings, validates duplicate target keys, and returns a rename transformer; updates Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as remap(caller)
participant Make as makeTransformer
participant Validator as duplicateCheck
participant Ramda as R.renameKeys
participant Transformer as transformer
Caller->>Make: pass mapping (object)
Make->>Validator: filter undefined, collect targets
alt duplicates found
Validator-->>Make: throw Error
Make-->>Caller: Error
else no duplicates
Validator->>Ramda: build rename transformer
Ramda-->>Make: transformer
Make-->>Caller: transformer
Caller->>Transformer: apply transformer to input
Transformer-->>Caller: transformed object
end
Note over Caller: If `tool` is a function (Mapper), remap uses it directly (no Make step)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
zod-plugin/src/remap.ts (1)
30-36: LGTM! Consider enhancing the error message.The duplicate detection logic is correct and the validation approach is sound. The TODO comment acknowledges the generic naming.
Consider making the error message more actionable by identifying the specific duplicate target:
const ensure = (tool: Record<string, string>) => { const targets = Object.values(tool); if (new Set(targets).size !== targets.length) - throw new Error("remap(): duplicate target keys", { cause: tool }); + { + const duplicates = targets.filter((val, idx) => targets.indexOf(val) !== idx); + throw new Error(`remap(): duplicate target keys: ${[...new Set(duplicates)].join(", ")}`, { cause: tool }); + } return tool; };zod-plugin/tests/runtime.spec.ts (1)
120-125: LGTM! Comprehensive test coverage.The test properly validates the duplicate target key detection. The error pattern matching is appropriate and the test case clearly demonstrates the expected behavior.
Consider adding an edge case test for multiple duplicates (e.g.,
{ a: "x", b: "x", c: "y", d: "y" }) to ensure the validation catches all duplicates, not just the first one.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
zod-plugin/src/remap.ts(1 hunks)zod-plugin/tests/runtime.spec.ts(1 hunks)
🔇 Additional comments (2)
zod-plugin/src/remap.ts (2)
41-41: LGTM! Type signature improvement.The change to
Partial<Record<string, string>>properly reflects the runtime behavior where undefined values are filtered out withR.reject(R.isNil, tool). This provides better type safety and flexibility for partial mappings.
44-46: Consider validating function mappers as well.The implementation correctly filters nil values before validation. However, when
toolis a function (Mapper), the duplicate target validation is bypassed. If a custom mapper function produces duplicate targets, the resulting schema might exhibit unexpected behavior.Should custom mapper functions be validated for duplicate targets after execution? This could be done by wrapping the function result with
ensure()or documenting this as a caller responsibility.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
zod-plugin/CHANGELOG.md (1)
5-7: Clarify the phrasing for the duplicate target key validation behavior.The changelog entry uses past tense ("would throw") which could be ambiguous. Consider rephrasing to present tense ("now throws") for clarity about the new behavior.
Suggested revision:
- `ZodObject::remap()` would throw an `Error` if duplicate target keys found in its argument. + `ZodObject::remap()` now throws an `Error` if duplicate target keys are found in its argument.Additionally, please verify that this changelog entry accurately reflects the implementation in
zod-plugin/src/remap.tsby reviewing the actual validation logic and corresponding test case inzod-plugin/tests/runtime.spec.ts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
zod-plugin/CHANGELOG.md(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-05T14:43:24.702Z
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2869
File: zod-plugin/package.json:42-44
Timestamp: 2025-08-05T14:43:24.702Z
Learning: Zod version 4 has been officially released and is stable. As of August 2025, the latest published version is 4.0.14 (published July 30, 2025). Zod v4 is available on npm and can be used in peer dependencies with version constraints like "^4.0.0".
Applied to files:
zod-plugin/CHANGELOG.md
📚 Learning: 2025-05-27T19:27:13.492Z
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2546
File: express-zod-api/tests/form-schema.spec.ts:31-31
Timestamp: 2025-05-27T19:27:13.492Z
Learning: Zod version 3.25.0 and later expose the Zod v4 API through the special import paths "zod/v4" and "zod/v4/core", allowing v4 features like .loose() to be used even when the package.json dependency shows a 3.x version.
Applied to files:
zod-plugin/CHANGELOG.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: report
1 participant
#2878 (comment)
Summary by CodeRabbit
Bug Fixes
Tests
Documentation