-
-
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
Repo restructuring to remove circular dependencies #227
Comments
I don't know how to do this without putting everything into one file. Circular dependencies are supposed to be allowed as long as you're careful about loading order. There's a reason |
I believe the two points in the RFC should completely remove all circular deps we have in the project.
Personally, I don't believe it's possible to get rid of circular deps with the
|
Zod has been building/bundling and working fine for a long time with circular dependencies. The most recent issue with bundling is #222 and it was due to a dumb mistake on my part: using The ESM stuff is a different beast and has other issues, but at the end of the day circular dependencies are possible in both module systems (commonjs or ESM). The hard part is structuring the code so both are happy. When I naively added ESM support last week, the code built fine but broke in runtime because the loading order stuff wasn't correct. Madge is too strict since it doesn't allow any cycles. A better way to check for issues in the built version of Zod is to run the tests using the post-build version of Zod, instead of the .ts source. Doing that would have caught the ESM errors last week before they got published to NPM. |
I believe I understand your position on circular dependencies:
I have a different opinion, but you know better what's best for zod.
And as always, these are just what I, a mere individual, can think of right away. |
I just don't think it's possible to remove circular dependencies without combining everything back into one file. Look at the imports in |
@colinhacks I have tried and resolved most of them before locally, but then I read the recommendation to follow the Medium article and decided to demonstrate first that article doesn't resolve the issue.
|
FYI, I have been hit by the same issue today using
And then at runtime in the browser:
Would be great to resolve this issue! |
Re: modularization and circular dependencies - one trick is to use dynamic |
@o-alexandrov zod@3.0.0-alpha.7 now contains no circular dependencies. |
Related issues
RFC
base
s shouldn't import other modulessrc/types/base.ts
should be reconsideredsrc/parser.ts
should be split into two filessrc/parser/index.ts
&src/parser/types.ts
so the circular dependency between ZodError & parser is removedThe text was updated successfully, but these errors were encountered: