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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

load project routes before app routes #9226

Merged
merged 5 commits into from Nov 13, 2023
Merged

load project routes before app routes #9226

merged 5 commits into from Nov 13, 2023

Conversation

aditya-mitra
Copy link
Collaborator

@aditya-mitra aditya-mitra commented Nov 7, 2023

Summary

馃 Generated by Copilot at 693d2a2

This pull request improves the routing functionality of the client package by refactoring and simplifying the custom routes component and moving it to the RouterService class. It also removes an unnecessary console log statement from the RouterService file.

References

closes #9205

Explanation

馃 Generated by Copilot at 693d2a2

  • Refactored the routing logic to use the RouterService class and allow custom routes to be defined and rendered dynamically (link, link, link, link, link)
  • Moved the CustomRoutes component from customRoutes.tsx to RouterService.ts and added a type annotation for the customRoutes prop (link)
  • Simplified the filtering and rendering of the custom routes in the CustomRoutes component by comparing the whole route and pathname strings (link)
  • Modified the RouterComp function in public.tsx to load the custom route component for each case in the switch statement, if it exists, or fall back to the default component (link)
  • Extracted the logic for loading the custom route component into a separate function called loadProjectComponent in public.tsx (link)
  • Removed the unnecessary console log statement from the getCustomRoutes function in RouterService.ts (link)
  • Removed the unnecessary curly braces around the RouteElement variable in the return statement of the RouterComp function in public.tsx (link)

馃 Generated by Copilot at 693d2a2

Sing, O Muse, of the skillful coder who refactored
The custom routes component, and made it more ordered
He removed the console.log from the getCustomRoutes function
Like Zeus hurling his thunderbolts to end the earth's corruption

QA Steps

Screenshot 2023-11-07 at 6 49 34鈥疨M

Checklist

  • If this PR is still a WIP, convert to a draft
  • When this PR is ready, mark it as "Ready for review"
  • ensure all checks pass
  • Changes have been manually QA'd
  • Changes reviewed by at least 2 approved reviewers

Copy link
Member

@hanzlamateen hanzlamateen left a comment

Choose a reason for hiding this comment

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

I am getting following error on MT project as it has redirect implemented in it.

image

@HexaField
Copy link
Member

I am getting following error on MT project as it has redirect implemented in it.

image

is this perhaps due to navigating from a tailwind route to a mui route, or vice versa?

@aditya-mitra
Copy link
Collaborator Author

It was a react-router issue with navigate. If a reload was done on the same page there would be no errors.
Using native location.replace produced no error. Best guess is probably race condition where while Engine was initialized when the page got navigated and again the Engine was reinitialized.
I have pushed a fixed for it.

@aditya-mitra aditya-mitra changed the title allow overriding app routes with project routes load project routes before app routes Nov 8, 2023
Copy link
Member

@hanzlamateen hanzlamateen left a comment

Choose a reason for hiding this comment

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

Seems to be working fine now.

@HexaField HexaField merged commit 8184782 into dev Nov 13, 2023
13 checks passed
@HexaField HexaField deleted the feat/project-routes branch November 13, 2023 05:54
DanielBelmes added a commit that referenced this pull request Nov 14, 2023
HexaField added a commit that referenced this pull request Nov 14, 2023
* Revert "load project routes before app routes (#9226)"

This reverts commit 8184782.

* revert and apply fix

* clean up log

---------

Co-authored-by: HexaField <joshfield999@gmail.com>
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.

[Bug]: Unable to override / route in a project
3 participants