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

visx-axis AxisRenderer.tsx: resolve circular dependency #1762

Merged
merged 1 commit into from
Oct 26, 2023

Conversation

ithrforu
Copy link
Contributor

@ithrforu ithrforu commented Oct 23, 2023

🚀 Enhancements

Resolve circular dependencies in packages/visx-axis/src/axis.

Neighboring files (Axis.tsx#10, AxisBottom.tsx#4, AxisLeft.tsx#4 etc.) import the Orientation constant from ../constants/orientation, but for some reason AxisRenderer.tsx import Orientation from the public API (../index.ts). This is the cause of circular dependencies.

Before (webpack's circular-dependency-plugin):

ERROR in Circular dependency detected:
node_modules/@visx/axis/esm/axis/Axis.js -> node_modules/@visx/axis/esm/axis/AxisRenderer.js -> node_modules/@visx/axis/esm/index.js -> node_modules/@visx/axis/esm/axis/Axis.js

ERROR in Circular dependency detected:
node_modules/@visx/axis/esm/axis/AxisBottom.js -> node_modules/@visx/axis/esm/axis/Axis.js -> node_modules/@visx/axis/esm/axis/AxisRenderer.js -> node_modules/@visx/axis/esm/index.js -> node_modules/@visx/axis/esm/axis/AxisBottom.js

ERROR in Circular dependency detected:
node_modules/@visx/axis/esm/axis/AxisLeft.js -> node_modules/@visx/axis/esm/axis/Axis.js -> node_modules/@visx/axis/esm/axis/AxisRenderer.js -> node_modules/@visx/axis/esm/index.js -> node_modules/@visx/axis/esm/axis/AxisLeft.js

ERROR in Circular dependency detected:
node_modules/@visx/axis/esm/axis/AxisRenderer.js -> node_modules/@visx/axis/esm/index.js -> node_modules/@visx/axis/esm/axis/Axis.js -> node_modules/@visx/axis/esm/axis/AxisRenderer.js

ERROR in Circular dependency detected:
node_modules/@visx/axis/esm/axis/AxisRight.js -> node_modules/@visx/axis/esm/axis/Axis.js -> node_modules/@visx/axis/esm/axis/AxisRenderer.js -> node_modules/@visx/axis/esm/index.js -> node_modules/@visx/axis/esm/axis/AxisRight.js

ERROR in Circular dependency detected:
node_modules/@visx/axis/esm/axis/AxisTop.js -> node_modules/@visx/axis/esm/axis/Axis.js -> node_modules/@visx/axis/esm/axis/AxisRenderer.js -> node_modules/@visx/axis/esm/index.js -> node_modules/@visx/axis/esm/axis/AxisTop.js

ERROR in Circular dependency detected:
node_modules/@visx/axis/esm/index.js -> node_modules/@visx/axis/esm/axis/Axis.js -> node_modules/@visx/axis/esm/axis/AxisRenderer.js -> node_modules/@visx/axis/esm/index.js

After:
There are no circular dependencies in @visx/axis.

Copy link
Collaborator

@williaster williaster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @ithrforu thanks for looking into this and for contributing the fix! Really appreciate the detailed logs/context 🙏

Will get this in once CI finishes 🚀

@ithrforu
Copy link
Contributor Author

ithrforu commented Oct 25, 2023

@williaster, thanks for the feedback! The Happo check is failed 🤔

@williaster
Copy link
Collaborator

@ithrforu resolved!

@williaster williaster merged commit ef3d8a6 into airbnb:master Oct 26, 2023
2 checks passed
@github-actions
Copy link

🎉 This PR is included in version v3.4.1 of the packages modified 🎉

@ithrforu ithrforu deleted the patch-1 branch October 27, 2023 07:19
Onxi95 pushed a commit to Onxi95/visx that referenced this pull request Nov 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants