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.

Sign up for GitHub

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

Jump to bottom

[WIP] - attempt to use "internal" pattern to fix circular deps #231

Closed
wants to merge 2 commits into from
Closed

[WIP] - attempt to use "internal" pattern to fix circular deps #231

wants to merge 2 commits into from

Conversation

o-alexandrov
Copy link
Contributor

This PR attempts to use internal pattern that @colinhacks proposed to use in this comment.

Unfortunately, it doesn't seem to resolve the problem.
Now, the circular dependency appears to be between internal <=> modules (that depend on other modules, thru the use of internal).

@mweststrate I apologize for pinging and please feel free to ignore. However, it'd be wonderful to know the view of the author of this pattern. Thank you :)

Result:

$ yarn madge src/index.ts --circular
Processed 44 files (701ms) 

✖ Found 37 circular dependencies!

1) PseudoPromise.ts > internal.ts
2) internal.ts > ZodDef.ts > types/any.ts
3) internal.ts > ZodDef.ts > types/array.ts
4) internal.ts > ZodDef.ts > types/bigint.ts
5) internal.ts > ZodDef.ts > types/boolean.ts
6) internal.ts > ZodDef.ts > types/date.ts
7) internal.ts > ZodDef.ts > types/enum.ts
8) internal.ts > ZodDef.ts > types/function.ts
9) internal.ts > ZodDef.ts > types/intersection.ts
10) internal.ts > ZodDef.ts > types/lazy.ts
11) internal.ts > ZodDef.ts > types/literal.ts
12) internal.ts > ZodDef.ts > types/map.ts
13) internal.ts > ZodDef.ts > types/nativeEnum.ts
14) internal.ts > ZodDef.ts > types/never.ts
15) internal.ts > ZodDef.ts > types/null.ts
16) internal.ts > ZodDef.ts > types/nullable.ts
17) internal.ts > ZodDef.ts > types/number.ts
18) internal.ts > ZodDef.ts > types/object.ts
19) internal.ts > ZodDef.ts > types/optional.ts
20) internal.ts > ZodDef.ts > types/promise.ts
21) internal.ts > ZodDef.ts > types/record.ts
22) internal.ts > ZodDef.ts > types/string.ts
23) internal.ts > ZodDef.ts > types/transformer.ts
24) internal.ts > ZodDef.ts > types/tuple.ts
25) internal.ts > ZodDef.ts > types/undefined.ts
26) internal.ts > ZodDef.ts > types/union.ts
27) internal.ts > ZodDef.ts > types/unknown.ts
28) internal.ts > ZodDef.ts > types/void.ts
29) internal.ts > ZodError.ts
30) internal.ts > codegen.ts
31) internal.ts > defaultErrorMap.ts
32) internal.ts > helpers/maskUtil.ts
33) internal.ts > helpers/objectUtil.ts
34) internal.ts > helpers/partialUtil.ts
35) internal.ts > isScalar.ts
36) internal.ts > parser.ts
37) internal.ts > types/base.ts

@o-alexandrov o-alexandrov mentioned this pull request Nov 19, 2020
Closed
2 tasks
@mweststrate
Copy link

mweststrate commented Nov 19, 2020 via email

@o-alexandrov
Copy link
Contributor Author

o-alexandrov commented Nov 19, 2020 •
edited
Loading

Thank you very much for the quick answer and my apologies, I was too overhyped when read your article :D

  • even though I read it in full, it slipped from me that the only thing we are doing using this pattern is:
    • introducing a manageable way to order modules' initialization

@colinhacks please let me know your opinion on how should we proceed to fix the original problem (#215) this PR attempts to solve:

  • unpredictable breaks in the production (!) bundles

Arguments:

I suggest to completely fix circular dependencies by applying strategy described in #227

@o-alexandrov
Copy link
Contributor Author

Closing in favor of #226

@o-alexandrov o-alexandrov deleted the v2-circular-deps branch December 8, 2020 09:50
@o-alexandrov o-alexandrov mentioned this pull request Dec 11, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@o-alexandrov @mweststrate