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(web): circular imports #1415

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

fix(web): circular imports #1415

wants to merge 4 commits into from

Conversation

kaiserbh
Copy link
Collaborator

@kaiserbh kaiserbh commented Feb 17, 2024

• Circular Dependencies ✅ Congratulations, no circular dependency was found in your project.

@stacksmash76 ;)

Probably a temporary fix tbh.

A better fix would be refactoring the whole routes.. I went through Tanstack router and should definitely follow what they suggest in particular:

⚠️ Note: The following example shows how to configure routes using code, and for simplicity's sake is in a single file for this demo. While code-based generation allows you to declare many routes and even the router instance in a single file, we recommend splitting your routes into separate files for better organization and performance as your application grows.

closes #1408

Probably a temporary fix tbh.
web/src/ctx/auth.tsx Dismissed Show dismissed Hide dismissed
@martylukyy martylukyy added this to the v1.39.0 milestone Feb 18, 2024
@martylukyy martylukyy added the web label Feb 18, 2024
Copy link
Collaborator

@martylukyy martylukyy left a comment

Choose a reason for hiding this comment

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

Thank you for jumping in and fixing this! 😄

@zze0s zze0s changed the title fix(deps): fixed circular deps. fix(web): circular imports Feb 20, 2024
Copy link
Collaborator

@zze0s zze0s left a comment

Choose a reason for hiding this comment

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

Good job!

This is a lot of changes just to get rid of warnings tho.

I am however not a fan of the hardcoded route strings like /auth/authenticated-routes/filters/$filterId, feels like half of the point of the router is gone when there's hardcoded route path strings like that? 🤔

@zze0s zze0s removed this from the v1.39.0 milestone Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[smell] circular imports on frontend
3 participants