Enhancements in routing - Closes #499 #509
Conversation
src/components/app/index.js
Outdated
| const App = () => ( | ||
| <section className={styles['body-wrapper']}> | ||
| <Header /> | ||
| <Link to='/transactions'>Transactions</Link> |
There was a problem hiding this comment.
these links should be in src/components/defaultLayout/index.js. They will be replaced by the tabs in #505
src/components/app/index.test.js
Outdated
| }); | ||
| }); | ||
| }); | ||
| describe.skip('allow to render private components after logged in', () => { |
There was a problem hiding this comment.
Remove the skip or add a comment on why it needs to be skipped
| @@ -70,7 +70,7 @@ module.exports = (env) => { | |||
| exclude: /node_modules/, | |||
| loader: 'babel-loader', | |||
| options: { | |||
| presets: ['es2015', 'react'], | |||
| presets: ['es2015', 'react', 'stage-3'], | |||
There was a problem hiding this comment.
What do you use stage-3 for? I tried to read some basics about what it is, but I cannot find what new features you use.
There was a problem hiding this comment.
Now I see it is because of { component: Component, ...rest }
| const DefaultLayout = ({ component: Component, ...rest }) => ( | ||
| <PrivateRoute {...rest} component={ matchProps => ( | ||
| <main> | ||
| <Account /> |
There was a problem hiding this comment.
The Links component was moved on @slaweet request. I think that the performance change here is negligible especially if you will move the logic from account component and keep it dumb and allow only render the passed props. I do not know how the app will be looked after rebranding, and I think that the layout can help combine some components for differents views.
If you think that is a bad solution than I can return the nested route.
| const DefaultLayout = ({ component: Component, ...rest }) => ( | ||
| <PrivateRoute {...rest} component={ matchProps => ( | ||
| <main> | ||
| <Account /> |
There was a problem hiding this comment.
I noticed you render Account component and Link components every time changing the route, I think rending them once will help improving performance. to do so, I’d suggest find a way to keep those components mounted once or adapting unit tests. nested routes are used in many apps. there should be some other applications facing this situation.
| const Public = () => <h1>Public</h1>; | ||
| const Private = () => <h1>Private</h1>; | ||
|
|
||
| describe('<PrivateRouteRender />', () => { |
There was a problem hiding this comment.
Please use just PrivateRouteRender to make it consistent with other Components' describes
<PrivateRoute />component.babelstage-3Metronome.init(). Moved it fromAppcomponent.Appcomponent tests. They are not cover all cases because it is hard to test root component with many dependencies (Provider, Router)close Enhancements in routing #499