Skip to content
This repository was archived by the owner on Nov 20, 2025. It is now read-only.

fix: Circular Dependencies Issue#1936

Merged
d-goog merged 4 commits intomainfrom
circle-dep-fix
Feb 5, 2025
Merged

fix: Circular Dependencies Issue#1936
d-goog merged 4 commits intomainfrom
circle-dep-fix

Conversation

@d-goog
Copy link
Contributor

@d-goog d-goog commented Feb 5, 2025

Fixing some circular dependency issues in both:

  • crypto (moved shared items into shared)
  • pluggable auth (moved a custom error into the handler file)

Without this change, ESM cannot be supported in the library in the future. Some tools do not properly build this library due to this bug.

We would have implicit testing for this in the future through the introduction of either ESM support or esbuild.

Fixes #1539 🦕

@d-goog d-goog requested a review from a team February 5, 2025 02:55
@d-goog d-goog requested a review from a team as a code owner February 5, 2025 02:55
@product-auto-label product-auto-label bot added the size: m Pull request size is medium. label Feb 5, 2025
@d-goog d-goog changed the title Circle-dep-fix fix: Circular Dependencies Issue Feb 5, 2025
@d-goog d-goog added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 5, 2025
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 5, 2025
@sofisl
Copy link
Contributor

sofisl commented Feb 5, 2025

Seems reasonable to me, and I see why adding a test won't really be helpful, but I guess I can't know for sure if this solves the circular dependency issue since it's hard to follow the graph without visualizing it. It might be good just to test locally and make sure this does solve the problem with something like madge, but NB.

@d-goog d-goog merged commit aea893c into main Feb 5, 2025
16 of 17 checks passed
@d-goog d-goog deleted the circle-dep-fix branch February 5, 2025 17:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

size: m Pull request size is medium.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor!: Circular dependencies - src/crypto/crypto.js and src/auth/pluggable-auth-client.js

3 participants