Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Metadata without withMeta() #1719

Merged
merged 25 commits into from
May 4, 2024
Merged

Metadata without withMeta() #1719

merged 25 commits into from
May 4, 2024

Conversation

RobinTail
Copy link
Owner

According to the Zod author's PR, it's kinda OK to alter Zod's prototypes:

https://github.com/colinhacks/zod/pull/3445/files#diff-d8a41ab20c64f92092513f153b78e7bbf5dc929c92909fad25e03d53c7f15bb7R21-R37

I tried to avoid that when I created withMeta(), but since Colin is doing it themselves, this approach could be beneficial for consumers.

@RobinTail RobinTail changed the title Metadata using Zod prototypes Experiment: Metadata using Zod prototypes May 1, 2024
@RobinTail RobinTail added the refactoring The better way to achieve the same result label May 1, 2024
@RobinTail RobinTail closed this May 1, 2024
@RobinTail RobinTail reopened this May 1, 2024
@RobinTail RobinTail closed this May 1, 2024
src/metadata.ts Outdated Show resolved Hide resolved
@RobinTail RobinTail added the miracle Mysterious events are happening here label May 1, 2024
@RobinTail RobinTail reopened this May 1, 2024
Copy link

coveralls-official bot commented May 1, 2024

Coverage Status

coverage: 100.0%. remained the same
when pulling f853cbd on meta-via-prototype
into e8eaced on master.

@RobinTail RobinTail changed the title Experiment: Metadata using Zod prototypes Experiment: Metadata without withMeta May 2, 2024
@RobinTail
Copy link
Owner Author

The approach on altering prototypes of ZodType is confirmed by Colin:
colinhacks/zod#3445 (comment)

The final plugin implementation is here (slightly changed):
https://github.com/colinhacks/zod/blob/90efe7fa6135119224412c7081bd12ef0bccef26/plugin/effect/src/index.ts#L21-L51

@RobinTail RobinTail changed the title Experiment: Metadata without withMeta Ref: Metadata without withMeta May 4, 2024
@RobinTail RobinTail marked this pull request as ready for review May 4, 2024 10:00
Copy link
Owner Author

@RobinTail RobinTail left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

@RobinTail RobinTail changed the title Ref: Metadata without withMeta Metadata without withMeta May 4, 2024
@RobinTail RobinTail changed the title Metadata without withMeta Metadata without withMeta() May 4, 2024
@RobinTail
Copy link
Owner Author

✅ QA passed

@RobinTail RobinTail merged commit 0628464 into master May 4, 2024
12 checks passed
@RobinTail RobinTail deleted the meta-via-prototype branch May 4, 2024 11:19
RobinTail added a commit that referenced this pull request May 4, 2024
RobinTail added a commit that referenced this pull request May 9, 2024
Due to #1721,
colinhacks/zod#2860 (comment), and
#1719 I'm exploring the possibility to alter the behaviour of `.brand()`
for storing the `brand` property as a way to distinguish the proprietary
schemas in runtime.

This can replace the `proprietary()` function with a call of `.brand()`.

However, so far it turned out to be breaking because `ZodBranded` does
not expose the methods of the wrapped schema, such as `.extend()`, which
is used in `accept-raw.ts` endpoint for possible route params.

——

After a series of serious considerations I realized that exposing brand
to consumers of `express-zod-api` could be a beneficial feature.
Runtime brand can be accessed via
`._def[Symbol.for("express-zod-api")].brand`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
miracle Mysterious events are happening here refactoring The better way to achieve the same result
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant