Update boilerplate #52
Conversation
kenwheeler
commented
Jan 12, 2017
kenwheeler
commented
- Updates dependencies
- Swaps Airbnb for eslint-config-formidable
- Adds PWA prerequisites
- Swaps Airbnb for eslint-config-formidable - Adds PWA prerequisites
1 similar comment
ryan-roemer
left a comment
ryan-roemer
left a comment
There was a problem hiding this comment.
Mostly minor stuff and future tickets. Nice work Ken!
| extends: 'airbnb', | ||
| extends: 'formidable/configurations/es6-react', | ||
| env: { | ||
| browser: true, |
There was a problem hiding this comment.
Future work -- this eslint should be split and more nuanced. It combines both the Test and App eslint configs. In the future, remove the test stuff and add test/.eslintrc or something...
There was a problem hiding this comment.
Should probably split for server code too huh?
| options: { | ||
| eslint: { | ||
| configFile: path.resolve('./configuration/eslint/eslint.js'), | ||
| useEslintrc: false |
There was a problem hiding this comment.
I think we should use eslintrc as a future ticket so editors can pick things up, etc.
There was a problem hiding this comment.
It works in my editor (VSCode). Which editors are you aware of it not working in?
There was a problem hiding this comment.
I was just thinking editors probably pick up DIR/.eslintrc more easily but without any actual research on my part 😛
Whatever you think is best as long as we have "good" editor support.
There was a problem hiding this comment.
And when I made comment, I didn't see your package.json entry. If we split directories we might need to switch to one-off's anyways....
But, feel free to keep a "base" in ./configuration/eslint/** if you'd like and otherwise need to split .eslintrc's across dirs!
| new HtmlWebpackInlineSourcePlugin(), | ||
| new webpack.LoaderOptionsPlugin({ | ||
| options: { | ||
| eslint: { |
| <meta name="viewport" content="width=device-width, initial-scale=1"> | ||
| <meta name="theme-color" content="#db5945"> | ||
| <title>Formidable React App Starter</title> | ||
| <link rel="manifest" href="/manifest.webmanifest"> |
| "description": "A modern ReactJS starter template.", | ||
| "icons": [ | ||
| { | ||
| "src": "/icons/icon-72x72.png", |
| "coveralls": "^2.11.12", | ||
| "cross-env": "^2.0.0", | ||
| "css-loader": "^0.23.1", | ||
| "cross-env": "^3.1.4", |
There was a problem hiding this comment.
Future ticket: remove cross-env, use builder --env='{"NAME":"value"}'
| </AppContainer>, | ||
| document.getElementById('root') | ||
| ); | ||
| if (process.env.NODE_ENV !== 'development') { |
There was a problem hiding this comment.
Do we want to make this stronger and actually production|staging? Just thinking about test, etc.
| if (module.hot) { | ||
| module.hot.accept('./routes', () => { | ||
| // TODO: Remove console override when | ||
| // https://github.com/reactjs/react-router/issues/2704 is fixed |
There was a problem hiding this comment.
Please add a tracking issue in our repo too.
