-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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 preprocessor handling with sibling errors #2719
Conversation
✅ Deploy Preview for guileless-rolypoly-866f8a ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
@@ -721,7 +721,7 @@ z.string().regex(regex); | |||
z.string().includes(string); | |||
z.string().startsWith(string); | |||
z.string().endsWith(string); | |||
z.string().datetime(); // defaults to UTC, see below for options | |||
z.string().datetime(); // ISO 8601; default is without UTC offset, see below for options |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unsure if PRs should include deno build artifacts. If no I can pull.
Hey @colinhacks would you be able to merge this fix? It's affecting multiple projects... |
Same here, we can't upgrade to |
I would be glad having it merged! Do you have a fork @kpdecker ? |
Hey, any update on this? Do we plan to merge this? I can help if needed. |
Please merge this, I can't upgrade Zod otherwise 😅 |
@colinhacks Is there something blocking this PR? Can we help somehow? |
It seems like (according to this comment: #2820 (comment)), the team here uses "thumbs up on the root comment" as a measure to gauge popularity of a PR. If you'd like to see this PR considered, it could help to give a 👍 to the first comment on this thread. |
@kpdecker As a non-contributor who would love to see this merged, may I suggest some improvements to reduce the change size? #2912 is a duplicate by @yukukotani with a simpler fix: #3124 is another duplicate by @selimb with a simpler test (which itself could be further simplified): I'd also suggest removing the unrelated |
As @aaronadamsCA said, my small PR is enough to fix this. I've just copied the test from this PR and it's passed. 619b985 But no matter how the PR is small, nothing happens while colinhacks is too busy to maintain zod. We need additional maintainers. |
I unfortunately don't have time to support this PR as we've transitioned away from Zod. Closing to let one of the active community members take over. |
Here is a one-line fix you can apply as a local patch to the Zod package: I would suggest lending your 👍 to #2912 to encourage a fix to be merged. |
…ect (#2912) * add failing test * refer parse status instead of issues length * copy test from #2719 * Improve preprocess addIssue behavior * Clean up * Move tests --------- Co-authored-by: Colin McDonnell <[email protected]>
Fixes #2687