feat(zod-plugin): Abstract methods pack() and unpack()#2878
feat(zod-plugin): Abstract methods pack() and unpack()#2878
pack() and unpack()#2878Conversation
WalkthroughThis change removes the custom Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ZodSchema
participant packer
participant brand
User->>ZodSchema: create schema
User->>packer: pack(schema, { brand })
packer->>ZodSchema: attach bag (metadata)
User->>brand: setBrand.call(schema, brand)
brand->>packer: pack(schema, { brand })
User->>brand: getBrand(schema)
brand->>packer: unpack(schema)
packer-->>brand: return bag.brand
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
✨ Finishing Touches
🧪 Generate 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (10)
zod-plugin/packer.ts (5)
13-15: Mark $Packer as internal to avoid leaking implementation detailsThis interface is an internal check shape; exporting it expands the public API surface unnecessarily. Either remove
exportor mark it@internalso it won’t be relied upon by consumers.-export interface $Packer<B extends object> extends z.core.$ZodCheck { +/** @internal */ +interface $Packer<B extends object> extends z.core.$ZodCheck { _zod: $PackerInternals<B>; }
17-21: Polish the JSDoc: keep it impersonal and version-agnosticAvoid personal references and hard-coded version tags in source comments; keep it about intent/behavior. Suggest concise wording:
-/** - * This approach was suggested to me by Colin in a PM on Twitter. - * Refrained from using Metadata because the data should withstand refinements. - * @since 1.1.0 I generalized it to accept any object instead of only brand. - * */ +/** + * Attaches an arbitrary metadata "bag" to a schema using a no-op $ZodCheck. + * We avoid vanilla Zod metadata so the data survives refinements/describe/etc. + */
22-36: Preserve existing bag type in the return and align the generic with z.core.$ZodTypeTwo improvements:
- Return type should reflect that we’re merging with the existing bag, not replacing it.
- Using
z.core.$ZodTypematches how the plugin API is surfaced and clarifies.check()availability.-export const pack = <T extends z.ZodType, B extends object>( +export const pack = <T extends z.core.$ZodType, B extends object>( subject: T, bag: B, ) => { const Cls = z.core.$constructor<$Packer<B>>("$Packer", (inst, def) => { z.core.$ZodCheck.init(inst, def); inst._zod.onattach.push((schema) => { - Object.assign(schema._zod.bag, def.bag); + // Defensive init (in case bag is absent on some exotic Zod internals) + // and then merge the provided bag. + if (!schema._zod.bag) schema._zod.bag = {}; + Object.assign(schema._zod.bag, def.bag); }); inst._zod.check = () => {}; }); - return subject.check(new Cls({ check: "$Packer", bag })) as T & { - _zod: { bag: B }; - }; + return subject.check(new Cls({ check: "$Packer", bag })) as T & { + _zod: { bag: T["_zod"]["bag"] & B }; + }; };
26-33: Defensive bag initialization in onattachIf
schema._zod.bagis ever undefined for any schema shape,Object.assignwould throw. A tiny guard keeps this robust across Zod internals.- inst._zod.onattach.push((schema) => { - Object.assign(schema._zod.bag, def.bag); - }); + inst._zod.onattach.push((schema) => { + if (!schema._zod.bag) schema._zod.bag = {}; + Object.assign(schema._zod.bag, def.bag); + });
38-40: Overload unpack() to propagate the bag type when knownProviding an overload makes
unpack()return the preciseBwhen the input carries that information, while keeping the fallback signature.-export const unpack = <T extends z.core.$ZodType>( - subject: T, -): T["_zod"]["bag"] => subject._zod.bag; +export function unpack<B extends object>( + subject: z.core.$ZodType & { _zod: { bag: B } }, +): B; +export function unpack<T extends z.core.$ZodType>( + subject: T, +): T["_zod"]["bag"]; +export function unpack(subject: z.core.$ZodType) { + return subject._zod.bag; +}zod-plugin/packer.spec.ts (1)
13-21: Consider adding tests for repeated packing and precedenceTwo extra checks would harden expectations:
- Packing twice with overlapping keys should ensure “last write wins”.
- Packing must not affect parse semantics.
Example additions:
test("repeated pack merges with last write wins", () => { const s1 = pack(z.string(), { a: 1, b: 1 }); const s2 = pack(s1, { b: 2, c: 3 }); expect(s2._zod.bag).toMatchObject({ a: 1, b: 2, c: 3 }); }); test("pack does not change parse behavior", () => { const base = z.string().min(2); const packed = pack(base, { tag: "x" }); expect(() => packed.parse("x")).toThrow(); // still fails min(2) expect(packed.parse("ok")).toBe("ok"); });zod-plugin/brand.spec.ts (2)
14-18: Restore the spy to avoid cross-test leakage
packMockremains active after the test completes, which can cause false positives/negatives in later suites that also spy onpacker.pack.
Add a cleanup step:test("calls pack() with given brand", () => { const packMock = vi.spyOn(packer, "pack"); ... expect(packMock).toHaveBeenCalledWith(schema, { brand: "test" }); + + packMock.mockRestore(); });
16-16: Drop the optional-chaining –setBrandis always defined
setBrandis a named import from./brand; if it were undefined the test (and runtime) would already throw.
Removing?.clarifies intent and lets TypeScript catch accidental renames.- setBrand?.call(schema, "test"); + setBrand.call(schema, "test");zod-plugin/brand.ts (2)
5-5: Mark the constant as a literal withas constThis preserves the string literal type
"brand"instead of broadening tostring, improving
type-safety for callers that usekeyoftricks.-export const brandProperty = "brand"; +export const brandProperty = "brand" as const;
8-10: Avoid writing an explicitundefinedentryStoring the key when
brandisundefinedclutters the internal bag and forces
downstream checks to handle an additional case. Skip the property altogether:-export const setBrand = function (this: z.ZodType, brand?: PropertyKey) { - return pack(this, { [brandProperty]: brand }); +export const setBrand = function (this: z.ZodType, brand?: PropertyKey) { + return brand === undefined + ? this + : pack(this, { [brandProperty]: brand }); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
zod-plugin/brand-check.ts(0 hunks)zod-plugin/brand.spec.ts(1 hunks)zod-plugin/brand.ts(1 hunks)zod-plugin/helpers.spec.ts(0 hunks)zod-plugin/helpers.ts(0 hunks)zod-plugin/index.spec.ts(1 hunks)zod-plugin/index.ts(1 hunks)zod-plugin/packer.spec.ts(1 hunks)zod-plugin/packer.ts(1 hunks)zod-plugin/remap.ts(2 hunks)zod-plugin/runtime.spec.ts(1 hunks)zod-plugin/runtime.ts(3 hunks)
💤 Files with no reviewable changes (3)
- zod-plugin/helpers.spec.ts
- zod-plugin/helpers.ts
- zod-plugin/brand-check.ts
🧰 Additional context used
🧠 Learnings (17)
📓 Common learnings
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2697
File: CHANGELOG.md:5-5
Timestamp: 2025-06-02T21:11:20.768Z
Learning: In the express-zod-api repository, RobinTail follows a release workflow where package.json version is only updated on the master branch after merging all planned release changes. Changelog entries may show future version numbers while package.json remains at the previous version during feature development, and this is intentional workflow, not a version inconsistency that needs to be flagged.
Learnt from: RobinTail
PR: RobinTail/express-zod-api#0
File: :0-0
Timestamp: 2025-08-01T09:48:13.742Z
Learning: In express-zod-api, when migrating from Zod v3 to v4, the correct approach for internal type imports is to change from `import type { $ZodType } from "zod/v4/core"` to `import { z } from "zod"` and then use `z.core.$ZodType`. The zod/v4/core module is reexported as z.core by the main zod package, making this a valid and working approach.
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2546
File: express-zod-api/src/documentation-helpers.ts:508-512
Timestamp: 2025-05-28T07:58:09.853Z
Learning: In express-zod-api, when working with Zod's JSON schema override callbacks, using `delete` to mutate `ctx.jsonSchema` is the recommended approach per Zod's official documentation, even if it triggers performance linting warnings. This is preferable to creating copies with `undefined` values, especially for snapshot testing.
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.
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2546
File: express-zod-api/src/metadata.ts:5-13
Timestamp: 2025-05-27T20:08:50.699Z
Learning: The `_zod` property in Zod v4 schemas is officially documented and recommended for library authors to differentiate between Zod 3 and Zod 4 schemas at runtime, despite the underscore prefix. This is explicitly mentioned in Zod's library authors documentation and is not a private internal property.
📚 Learning: 2025-08-01T09:48:13.742Z
Learnt from: RobinTail
PR: RobinTail/express-zod-api#0
File: :0-0
Timestamp: 2025-08-01T09:48:13.742Z
Learning: In express-zod-api, when migrating from Zod v3 to v4, the correct approach for internal type imports is to change from `import type { $ZodType } from "zod/v4/core"` to `import { z } from "zod"` and then use `z.core.$ZodType`. The zod/v4/core module is reexported as z.core by the main zod package, making this a valid and working approach.
Applied to files:
zod-plugin/runtime.spec.tszod-plugin/index.spec.tszod-plugin/index.tszod-plugin/runtime.tszod-plugin/packer.spec.tszod-plugin/brand.spec.tszod-plugin/brand.tszod-plugin/remap.tszod-plugin/packer.ts
📚 Learning: 2025-05-28T18:58:10.064Z
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2428
File: express-zod-api/src/index.ts:44-44
Timestamp: 2025-05-28T18:58:10.064Z
Learning: The type-only import `import type {} from "qs";` in express-zod-api/src/index.ts is necessary to avoid TS2742 errors for exported functions like attachRouting, makeRequestMock, testEndpoint, and testMiddleware that have types depending on types/qs. This import provides the reference TypeScript needs to infer portable type names.
Applied to files:
zod-plugin/runtime.spec.tszod-plugin/index.spec.tszod-plugin/index.tszod-plugin/runtime.tszod-plugin/packer.spec.tszod-plugin/brand.spec.tszod-plugin/brand.tszod-plugin/remap.tszod-plugin/packer.ts
📚 Learning: 2025-06-14T16:42:52.972Z
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2736
File: express-zod-api/tsup.config.ts:12-26
Timestamp: 2025-06-14T16:42:52.972Z
Learning: In express-zod-api tsup configurations, the direct mutation of `options.supported` in the `esbuildOptions` callback is intentional behavior and should not be flagged as a side effect issue.
Applied to files:
zod-plugin/runtime.spec.tszod-plugin/index.spec.tszod-plugin/index.tszod-plugin/runtime.tszod-plugin/brand.tszod-plugin/remap.ts
📚 Learning: 2025-05-27T19:35:57.357Z
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2546
File: express-zod-api/tests/buffer-schema.spec.ts:32-37
Timestamp: 2025-05-27T19:35:57.357Z
Learning: In the express-zod-api project, tests are run from the `express-zod-api` workspace directory, and the project uses an ESM-first environment without `__dirname`. Relative paths like `../logo.svg` in test files correctly resolve to the repository root due to this test execution context.
Applied to files:
zod-plugin/runtime.spec.tszod-plugin/index.spec.tszod-plugin/index.tszod-plugin/runtime.tszod-plugin/packer.spec.tszod-plugin/brand.spec.ts
📚 Learning: 2025-05-27T20:27:17.015Z
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2546
File: express-zod-api/src/json-schema-helpers.ts:1-3
Timestamp: 2025-05-27T20:27:17.015Z
Learning: Ramda is correctly listed as a dependency in express-zod-api/package.json, so imports of ramda utilities are properly supported.
Applied to files:
zod-plugin/runtime.spec.tszod-plugin/index.spec.tszod-plugin/runtime.tszod-plugin/remap.ts
📚 Learning: 2025-05-27T20:27:17.015Z
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2546
File: express-zod-api/src/json-schema-helpers.ts:1-3
Timestamp: 2025-05-27T20:27:17.015Z
Learning: The JSONSchema type is not exported from the main "zod" module and must be imported from "zod/v4/core" when using Zod v4. This is acceptable for type-only imports as they don't create runtime dependencies.
Applied to files:
zod-plugin/runtime.spec.tszod-plugin/index.tszod-plugin/runtime.tszod-plugin/packer.spec.tszod-plugin/brand.spec.tszod-plugin/brand.tszod-plugin/remap.ts
📚 Learning: 2025-05-27T20:03:34.213Z
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2546
File: example/factories.ts:35-42
Timestamp: 2025-05-27T20:03:34.213Z
Learning: The `./example` directory in the express-zod-api repository contains demonstration code for educational purposes only, not intended for production use. Example code can make simplified assumptions for brevity and clarity, and should not be flagged for missing production-level error handling, security measures, or edge case handling.
Applied to files:
zod-plugin/runtime.spec.tszod-plugin/runtime.ts
📚 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/runtime.spec.tszod-plugin/index.tszod-plugin/runtime.tszod-plugin/remap.ts
📚 Learning: 2025-05-27T20:22:30.428Z
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2546
File: express-zod-api/tests/zts.spec.ts:160-162
Timestamp: 2025-05-27T20:22:30.428Z
Learning: In express-zod-api/tests/zts.spec.ts, the `Fruits` enum intentionally contains both string and numeric members (Apple = "apple", Banana = "banana", Cantaloupe = "cantaloupe", A = 5) and is used with `z.enum(Fruits)` to test how the system handles mixed enum types. This is by design for testing purposes.
Applied to files:
zod-plugin/runtime.spec.tszod-plugin/index.spec.tszod-plugin/runtime.tszod-plugin/packer.spec.tszod-plugin/brand.spec.ts
📚 Learning: 2025-06-02T21:08:56.475Z
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2697
File: CHANGELOG.md:5-5
Timestamp: 2025-06-02T21:08:56.475Z
Learning: The `cjs-test` directory in the express-zod-api repository is a test workspace and should be excluded when checking for main project version consistency with changelog entries.
Applied to files:
zod-plugin/index.spec.tszod-plugin/index.tszod-plugin/packer.spec.tszod-plugin/brand.spec.ts
📚 Learning: 2025-05-28T07:58:09.853Z
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2546
File: express-zod-api/src/documentation-helpers.ts:508-512
Timestamp: 2025-05-28T07:58:09.853Z
Learning: In express-zod-api, when working with Zod's JSON schema override callbacks, using `delete` to mutate `ctx.jsonSchema` is the recommended approach per Zod's official documentation, even if it triggers performance linting warnings. This is preferable to creating copies with `undefined` values, especially for snapshot testing.
Applied to files:
zod-plugin/runtime.tszod-plugin/remap.ts
📚 Learning: 2025-05-27T20:40:19.548Z
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2546
File: express-zod-api/src/json-schema-helpers.ts:75-87
Timestamp: 2025-05-27T20:40:19.548Z
Learning: In express-zod-api's `flattenIO` function in json-schema-helpers.ts, the `additionalProperties` field is used as a template to generate property schemas for literal property names extracted from `propertyNames.const` and `propertyNames.enum`. Converting boolean `additionalProperties` values to empty objects `{}` via `Object(entry.additionalProperties)` is intentional behavior, as the function only needs property schema templates, not the boolean semantics of `additionalProperties`.
Applied to files:
zod-plugin/runtime.tszod-plugin/brand.tszod-plugin/remap.ts
📚 Learning: 2025-05-27T20:08:50.699Z
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2546
File: express-zod-api/src/metadata.ts:5-13
Timestamp: 2025-05-27T20:08:50.699Z
Learning: The `_zod` property in Zod v4 schemas is officially documented and recommended for library authors to differentiate between Zod 3 and Zod 4 schemas at runtime, despite the underscore prefix. This is explicitly mentioned in Zod's library authors documentation and is not a private internal property.
Applied to files:
zod-plugin/runtime.tszod-plugin/brand.spec.tszod-plugin/brand.ts
📚 Learning: 2025-05-28T05:04:40.327Z
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2546
File: express-zod-api/src/io-schema.ts:5-8
Timestamp: 2025-05-28T05:04:40.327Z
Learning: The Base type `object & { [Symbol.iterator]?: never }` in express-zod-api correctly excludes arrays and other iterables from IOSchema. Arrays are properly rejected because they have a defined Symbol.iterator method which conflicts with the never constraint, as demonstrated by TypeScript compiler errors when attempting to assign z.array() to IOSchema.
Applied to files:
zod-plugin/runtime.ts
📚 Learning: 2025-07-20T11:09:58.980Z
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2833
File: express-zod-api/src/method.ts:3-3
Timestamp: 2025-07-20T11:09:58.980Z
Learning: In express-zod-api, the `SomeMethod` type (defined as `Lowercase<string>`) is intentionally broad to represent raw, unvalidated HTTP methods from requests. The narrower `Method` type is used after validation with `isMethod()`. This defensive programming pattern separates raw external input types from validated types, allowing graceful handling of unknown methods.
Applied to files:
zod-plugin/runtime.ts
📚 Learning: 2025-05-27T19:30:51.885Z
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2546
File: compat-test/sample.ts:1-1
Timestamp: 2025-05-27T19:30:51.885Z
Learning: Files in compat-test/ directories, especially those named sample.ts or similar, are often test fixtures for migration scripts and may intentionally contain deprecated or "incorrect" code that the migration tooling is designed to fix. These should not be flagged as issues.
Applied to files:
zod-plugin/packer.spec.ts
🧬 Code Graph Analysis (2)
zod-plugin/runtime.ts (2)
zod-plugin/brand.ts (1)
setBrand(8-10)zod-plugin/remap.ts (1)
remap(30-42)
zod-plugin/brand.spec.ts (1)
zod-plugin/brand.ts (1)
setBrand(8-10)
🔇 Additional comments (6)
zod-plugin/runtime.spec.ts (1)
3-3: Import path change for getBrand is correct and no stale helper imports remainI ran a search for any remaining imports from
./helpersand found none:rg -n 'from\s+["'\'']\./helpers["'\'']|require\(["'\'']\./helpers["'\'']\)' || true # no matchesEverything looks good—no further changes needed.
zod-plugin/index.spec.ts (1)
21-22: Export surface test updated appropriatelyAsserting
packandunpackalongsidegetBrandreflects the new public API.zod-plugin/index.ts (1)
3-4: Index exports are aligned with the refactorRe-exporting
pack/unpackand movinggetBrandto"./brand"looks good. Side-effect imports remain intact.zod-plugin/packer.spec.ts (2)
7-11: Good coverage of pack(): type and value behaviorNice validation of both runtime bag content and type inference.
24-30: unpack() type assertion is on pointVerifying the inferred shape via
expectTypeOfis great. With the suggested overloads, this should remain precise even when combining multiple packs.zod-plugin/runtime.ts (1)
55-56:remapis added as a non-enumerable, non-configurable method – deliberate?
writable: falsedisallows hot-patching (useful in tests) and the default
enumerable: falsehides the method fromfor…in/Object.keys. Confirm this
matches the public API expectations; otherwise setenumerable: true, configurable: true.
@coderabbitai, this suggestion is not correct, because I must ensure that brand setter is immutable (always calls |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain@RobinTail Thanks for the correction—acknowledged. For the brand setter to remain immutable and always trigger 🏁 Script executed: Length of output: 741 @RobinTail You’re right—keeping the explicit undefined entry is necessary here. It ensures:
I’ll avoid suggesting that change in this context. Thanks for the clarification. 🏁 Script executed: Length of output: 9318 @RobinTail Agreed. The code confirms this:
I’ll refrain from suggesting removal of the explicit undefined entry here. Also noted this aligns with our earlier learning about not memoizing the brand getter. (•̀ᴗ•́)و ̑̑ 🏁 Script executed: Length of output: 841 @RobinTail You’re right—keeping the explicit undefined entry is necessary:
I’ll withdraw that suggestion and won’t flag this pattern again. Noted alongside our prior learning about not memoizing the brand getter. 🏁 Script executed: Length of output: 5063 @RobinTail You’re right—keeping the explicit undefined is required to preserve immutability and unbranding:
I’m withdrawing that suggestion and won’t flag this pattern again. This also aligns with your earlier guidance to avoid memoizing the brand getter. ✏️ Learnings added
🧠 Learnings used |
pack() and unpack()
|
✅ worked well for ✅ QA passed |
#2878 (comment) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Remap now detects and rejects duplicate target keys in mappings to prevent key collisions. * **Tests** * Added a runtime test asserting an error is thrown for duplicate mapping targets. * **Documentation** * CHANGELOG updated to document the remap duplicate-target fix (v2.1.0). <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This should simplify creating more advanced branded schemas, such as function one.
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Tests
Chores
Documentation