Simplify state handling #5
Conversation
Nirei
force-pushed
the
refactor/simplify-state-handling
branch
from
8709f88
to
e0c8327
Compare
Nirei
force-pushed
the
refactor/simplify-state-handling
branch
from
e0c8327
to
a726c76
Compare
|
|
||
| const login = () => { | ||
| setContext({auth: {logued: true, username: 'notadmin'}}); | ||
| //Optional, force refresh on login | ||
| setLocation(location); |
There was a problem hiding this comment.
No longer necessary to do manual refresh, since we are now fully reactive to changes in context.
|
|
||
| // Listen to location changes | ||
| useEffect(() => { |
There was a problem hiding this comment.
This effect is now the getComponentFromRoute method.
|
|
||
| if (!route) { |
There was a problem hiding this comment.
This will help catch bugs in client applications.
| }, []); | ||
| export const Router = ({ routes, children }: IRouterProps) => { | ||
| const [location, setLocation] = useState<string>(browserHistory.location.pathname) | ||
| const [context, setContext] = useState<object>({}) |
There was a problem hiding this comment.
I minimized the state necessary to run the router.
There was a problem hiding this comment.
Nice! With this every change in context will trigger the router logic. The only concern I have is if it could be useful to change the router context without triggering a reroute?
| routedElement: undefined, | ||
| forceRefresh: 0 | ||
| }; | ||
| const browserHistory = createBrowserHistory() |
There was a problem hiding this comment.
This now lives in a constant since the only reason for it being a ref was because it lived inside the component so it needed to mutate once. Now it's a singleton for the life of the module.
|
|
||
| }, [state.location, state.forceRefresh]); | ||
| const params = Path.parse(route.path, location) | ||
| const component = getComponentFromRoute(route, { location, setLocation, context, setContext, params }) |
There was a problem hiding this comment.
Both this and the route computation could be memoized but I think every re-render will need to recompute it anyway so I think it's safe to just roll with it.
On very heavy routings (hundreds of thousands of routes times hundreds of thousands of guards per route, this could be slow) but if the case ever comes up we could just optimize it.
This greatly simplifies the
Routercomponent. Every calculated property has been removed from state. State has been separated into its individual parts (with the added benefit of referentially stable setters provided by default by React). Simplificable effects have been transformed to normal computations and theBrowserHistorymutable reference has been transformed to a component constant.Fixes #4 as a side effect.