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

Install madge NPM package and eliminate circular dependencies #116

Merged
merged 3 commits into from
Jun 3, 2024

Conversation

pkalita-lbl
Copy link
Collaborator

Fixes #115

Summary

These changes add a dependency on the madge package and connect it to a new check.imports script in package.json. The script is now called in the tests workflow to ensure we don't introduce new circular dependencies. In #115 I talked about using dpdm, but I realized after making the issues that madge seems to be more popular and has more people working on it.

As far as fixing the existing circular imports, all of them looked something like this:

flowchart LR
    A(Router.tsx)
    A --> B(various pages & components)
    B --> A
Loading

Where the various pages & components were importing Router.tsx to get the paths object.

So the fix is to move the paths object to a separate module and you end up with:

flowchart TD
    A(paths.ts)
    B(Router.tsx) --> A
    C(various pages & components) --> A
Loading

@pkalita-lbl pkalita-lbl requested review from yxu-lanl and eecavanna June 3, 2024 19:14
Copy link
Collaborator

@eecavanna eecavanna left a comment

Choose a reason for hiding this comment

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

I'm comfortable with this being merged in.

In contrast to Python, I haven't given much thought to circular imports in JavaScript.

@eecavanna eecavanna assigned eecavanna and pkalita-lbl and unassigned eecavanna Jun 3, 2024
@eecavanna eecavanna changed the title Issue 115 circular imports Install madge NPM package and eliminate circular dependencies Jun 3, 2024
@pkalita-lbl pkalita-lbl merged commit bc8778b into main Jun 3, 2024
1 check passed
@pkalita-lbl pkalita-lbl deleted the issue-115-circular-imports branch June 3, 2024 20:01
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.

Fix and prevent further circular imports
3 participants