switch to a stable router + route guards

[theAnuragMishra]

basically , we take out the state logic from route creation into the components.

[theAnuragMishra]

@theborakompanioni sorry for the delay. do you like the direction this is going in? the idea basically is to create route components unconditionally and use state variables in these components to redirect or keep on the page. this makes the router independent of state. the work is not fully complete yet. i'd like to get some review before i finalize.

[theborakompanioni]

Hey @theAnuragMishra!
I am not an expert so please go on an propose the changes you think will have a positive effect and solve a distinct problem. If the code is more robust as a result or fixes a problem, it'll be merged. Good for you?

Edit: Do you know the benefits of route-modules over component-routes?

[theAnuragMishra]

@theborakompanioni
tbh I didn't know that react-router has support for route modules but after reading the docs, this is very close to what newer frameworks like tanstack start do.
the main difference is that with route modules you get a lot of nice thigns like loaders and actions.
so with component routes, you'd do something like useEffect(()=>fetchData(), []) which is not the nicest way to fetch data because the effect runs after the component is mounted and thsu you have to maintain loading states, etc.
with route modules, you'd pass a loader function instead which runs before the component mounts and fetches the initial data the component needs.
there are also other benefits like actions which do server side mutations (like submitting a form to update something in the db) and revalidate/reload the related data automatically.

are you planning to switch to route modules? as i can see jam already uses the latest react-rotuer so migration will not be very hard. if you want i can go in that direction as well.

[theborakompanioni]

@theborakompanioni tbh I didn't know that react-router has support for route modules but after reading the docs, this is very close to what newer frameworks like tanstack start do. the main difference is that with route modules you get a lot of nice thigns like loaders and actions. so with component routes, you'd do something like useEffect(()=>fetchData(), []) which is not the nicest way to fetch data because the effect runs after the component is mounted and thsu you have to maintain loading states, etc. with route modules, you'd pass a loader function instead which runs before the component mounts and fetches the initial data the component needs. there are also other benefits like actions which do server side mutations (like submitting a form to update something in the db) and revalidate/reload the related data automatically.

💪

are you planning to switch to route modules? as i can see jam already uses the latest react-rotuer so migration will not be very hard. if you want i can go in that direction as well.

Yeah, sounds reasonable. But if you want, you can do that in a follow-up PR. First, let's bring your idea in a state that can be reviewed and tested. Sounds okay?

[theAnuragMishra]

@theborakompanioni i think the pr is ready for review. i have run the tests and manually tested the ui.

To summarize the changes:
Our current setup is something like <Route path="/xyz" element={someCondition ? <Element1/> : <Element2/>}/>. The problem with this is that the Router depends on state variables and thus is destroyed and recreated on any state change. Apart from this, some functions can be memoized.
This pr will change the setup to something like this:

function ComponentThatHandlesConditionalRendering(){
return someCondition ? <Element1/> : <Element2/>
}
<Route path="/xyz" element={<ComponentThatHandlesConditionalRendering/>}/>

Now the router itself doesn't depend on any state and renders a fixed component. This component manages what to do depending on the state. The router is stable 😀

From my side I have tried to follow all the contribution guideliens. Only minimally used AI for some suggestiosn, most of the work is my own. If anything needs any further change, please tell :)

[theAnuragMishra]

CI runs clean on my fork.

also since we are already using tanstack-query, we should probably stick to Component routes.

[theborakompanioni]

Hey @theAnuragMishra ..
Have you once visually tested it?
It seems JamSessionInfoContextProvider and JamWalletInfoContextProvider are not defined for protected routes, can you check?

[theAnuragMishra]

@theborakompanioni i did visually test it but not the entire flow. sorry i gave the context to protected routes without navbar but missed those with navbar.
fixed now, and also tested visually every route i could from my understanding. also gave another look at the changes. everything looks good to me.