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

Fix drizzle-zod and drizzle-typebox mapping refinements incorrectly #3737

Merged
merged 7 commits into from
Dec 19, 2024

Conversation

L-Mario564
Copy link
Collaborator

@L-Mario564 L-Mario564 commented Dec 10, 2024

Addresses #3735, #3756, #3751 & #3734.

Title. Also removed some unnecessary @internal JSDoc comments.

@L-Mario564 L-Mario564 changed the title Fix drizzle-zod mapping refinements incorrectly Fix drizzle-zod and drizzle-typebox mapping refinements incorrectly Dec 10, 2024
@AndriiSherman
Copy link
Member

@L-Mario564 let's add tests for all such cases, to make sure we won't hit it in the future

@L-Mario564 L-Mario564 changed the base branch from main to beta December 11, 2024 17:52
@jikyu
Copy link
Contributor

jikyu commented Dec 16, 2024

@L-Mario564 Do you know if this PR will fix #3732 and #3747?

@L-Mario564
Copy link
Collaborator Author

@L-Mario564 Do you know if this PR will fix #3732 and #3747?

I'm not sure, and it's difficult to verify such since the bug only happens after building it. If it's not fixed by removing the internal JSDoc comments (done in this PR) then we likely need to just expose internal types, which we'd do in a separate PR soon.

@tacomanator
Copy link
Contributor

tacomanator commented Dec 17, 2024

I added some example code on #3734. It may be helpful for reproduction and testing, assuming it's the same issue mentioned by OP.

@L-Mario564
Copy link
Collaborator Author

Managed to fix #3751 for drizzle-valibot.

@rakibatomnixima
Copy link

Any updates on this?

@AndriiSherman AndriiSherman merged commit 6492b1d into drizzle-team:beta Dec 19, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants