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

Fix circular depenacies across multiple component types #5454

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

mcwinter07
Copy link
Contributor

Why

This is part of an attempt to reduce or remove the hang time for our build in CI. After reading this post on the impact of circular dependencies with Vite, this could be the cause of our unknown hang time.

Finding the circular deps

To ID the problem files I ran madge against out components src folder which returned the following:

Screenshot 2025-01-16 at 2 46 46 PM

If you want to run this you will need install or run madge via npx then run this from the root our repo:
npx madge --extensions ts,tsx --ts-config=tsconfig.json packages/components/src -c

I've left the scss function unchange as I wasn't sure how this was causing a circular dep. the -mui helper are part of legacy vendor support and ideally should be removed / shifted into kaizen legacy.

What

  • Move some types when necessary to avoid circular deps

Copy link

changeset-bot bot commented Jan 16, 2025

🦋 Changeset detected

Latest commit: a55a1c7

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@kaizen/components Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@mcwinter07 mcwinter07 force-pushed the KZN-2947/circular-dep-fixes branch from 61e1dd8 to 205382f Compare January 16, 2025 05:37
@mcwinter07 mcwinter07 marked this pull request as ready for review January 16, 2025 23:13
@mcwinter07 mcwinter07 requested a review from a team as a code owner January 16, 2025 23:13
Copy link
Contributor

github-actions bot commented Jan 17, 2025

✨ Here is your branch preview! ✨

Last updated for commit a55a1c7: Merge branch 'main' into KZN-2947/circular-dep-fixes

@mcwinter07 mcwinter07 requested a review from dougmacknz January 19, 2025 23:43
@mcwinter07 mcwinter07 enabled auto-merge (squash) January 20, 2025 22:15
@mcwinter07 mcwinter07 disabled auto-merge January 20, 2025 23:24
Comment on lines 8 to 9
type MeaningfulSVG = { 'role': 'img'; 'aria-label': string; 'alt'?: never }
type DecorativeSVG = { 'role': 'presentation'; 'aria-label'?: never; 'alt'?: never }
Copy link
Contributor

Choose a reason for hiding this comment

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

You've defined this in types.ts now, so better to export and import from there rather than have the duplication :)
Alt move all the types in there 😬

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants