Routes and Layout Containers #38
Conversation
| @@ -0,0 +1,23 @@ | |||
| import React from 'react'; | |||
| @@ -0,0 +1,23 @@ | |||
| import React from 'react'; | |||
| import { Router, Route, Redirect, IndexRoute, browserHistory } from 'react-router'; | |||
| </Route> | ||
| ); | ||
|
|
||
| const AppRouter = () => <Router history={browserHistory} routes={routes} />; |
There was a problem hiding this comment.
you could just export default that function if you want
|
|
||
| const AppRouter = () => <Router history={browserHistory} routes={routes} />; | ||
|
|
||
| export default AppRouter; |
| render() { | ||
| return ( | ||
| <div> | ||
| <h1>Community</h1> |
There was a problem hiding this comment.
also why is this not a stateless component?
There was a problem hiding this comment.
oh, because im developing it in another branch and I think its not going to be stateless
|
|
||
| LayoutContainer.propTypes = {}; | ||
|
|
||
| const mapStateToProps = state => { |
There was a problem hiding this comment.
you could just:
const mapStateToProps = () => ({ data: {} })
| }; | ||
| }; | ||
|
|
||
| const mapDispatchToProps = (dispatch, ownProps) => ({ |
There was a problem hiding this comment.
same here
const mapDispatchToProps = dispatch => ({ dispatch })
| dispatch | ||
| }); | ||
|
|
||
| export default connect(mapStateToProps, mapDispatchToProps, null)(LayoutContainer); |
There was a problem hiding this comment.
extra semicolon abd probably the third param as null has no effect
| @@ -63,7 +62,7 @@ class ModerationQueue extends React.Component { | |||
| console.log('moderation queue', styles) | |||
There was a problem hiding this comment.
can we remove this log?
| new webpack.DefinePlugin({ | ||
| 'process.env': { | ||
| 'NODE_ENV': JSON.stringify('production'), | ||
| 'VERSION': JSON.stringify(require("./package.json").version) |
There was a problem hiding this comment.
what's the effect of stringifying a string? 🤔
There was a problem hiding this comment.
"production" produces the value production
JSON.stringify("production") produces the value "production"
It should be the same as writing '"production"'
It's a common practice while using env vars
There was a problem hiding this comment.
oh, you use that instead of doing
`"${thevalue}"`
makes more sense not to involve a json parser for that IMO
There was a problem hiding this comment.
I can update those I wasn't sure about using es6 in webpack config
There was a problem hiding this comment.
updated! @impronunciable
|
please solve conflicts |
What does this PR do?
How do I test this PR?
npm run startandnpm run buildhttp://localhost:3000@coralproject/tech