Move render function out of route handlers #1155
Conversation
static_src/routes.js
Outdated
| @@ -57,10 +51,6 @@ export function overview(next) { | |||
| }) | |||
| .then(pageActions.loadSuccess, pageActions.loadError); | |||
|
|
|||
| ReactDOM.render(<MainContainer> | |||
There was a problem hiding this comment.
is there a reason, you removed it from here and moved it up?
There was a problem hiding this comment.
No, I had originally moved it inside the callback, which obviously blocked rendering, and when I moved it out stuck it above the spaceActions fetch call for no good reason 😄
|
I like it. just had one question. |
el-mapache
force-pushed
the
ab-router-poc
branch
from
0bebf69 to
5315171
Compare
|
One more question. do you think this pattern should go into the |
|
I'm not sure, I suppose it depends on the future routes added and what the devs' rationale is at that point in time..and also if we all agree splitting the apps concerns this way is worth the changes. |
el-mapache
force-pushed
the
ab-router-poc
branch
from
15f272d to
5653a29
Compare
|
Seems like this change is at least not disliked by the team, so if someone wants to merge it that is 👍 ! |
Adds router store, actions, router jsx container * decouple react DOM rendering from route handler * stop triggering a full app repaint on every route change Remove comments/old code, default show <Loading/> Add RouteProvider component, restore next calls Add spec for router actions Also updates assertAction helper to look for presence of `data` key in action dispatch Add spec for RouterStore Fix shouldComponentUpdate logic in RouteProvider * It was always returning true Add RouteProvider spec
el-mapache
force-pushed
the
ab-router-poc
branch
from
5653a29 to
ab624b2
Compare
This is a little POC to move
ReactDOM.rendercalls out of the route handler functions and let React handle its own updates with a newRouteProvidercomponent. The main benefit is that we don't have to re-render theMainContainerevery time a route changes.It also could set us up for integrating the router within the flux architecture if we want to do more of that in the future!
Let me know what ya'll think!