Manual React setup #26
Conversation
| "plugins": [ | ||
| "@babel/proposal-class-properties", | ||
| "@babel/proposal-object-rest-spread" | ||
| ] |
There was a problem hiding this comment.
These two can be good additions to this -
"@babel/plugin-proposal-optional-chaining",
"@babel/plugin-proposal-nullish-coalescing-operator"
| public/ | ||
| webpack.config.js |
There was a problem hiding this comment.
Why should webpack.config.js be ignored? How can other developers build it locally?
There was a problem hiding this comment.
I just thought wepack.config.js should not be linted
There was a problem hiding this comment.
Ah, my bad. Read the filename as gitignore
| "html-webpack-plugin": "^4.3.0", | ||
| "style-loader": "^1.2.1", | ||
| "typescript": "^3.9.3", | ||
| "webpack": "^4.43.0", |
There was a problem hiding this comment.
Just a thought, since we've started with this config - why not use webpack 5?
There was a problem hiding this comment.
As far as i know, webpack 5.0 is still in beta.
https://github.com/webpack/webpack/releases
public/manifest.json
Outdated
| "theme_color": "#000000", | ||
| "background_color": "#ffffff" | ||
| "short_name": "vMMX", | ||
| "name": "virtual Marbe Machine X", |
tsconfig.json
Outdated
| "noFallthroughCasesInSwitch": true, | ||
| "noImplicitAny": true, | ||
| "noUnusedLocals": true, | ||
| "outDir": "./dist/", | ||
| "target": "es5", | ||
| "lib": ["dom", "dom.iterable", "esnext"], | ||
| "allowJs": true, |
There was a problem hiding this comment.
Additionally use this -
"exclude": ["node_modules"]
There was a problem hiding this comment.
I will add it, but is it really necessary when using: "include": ["src"]?
There was a problem hiding this comment.
Yes. No harm in explicitly ignoring node_modules I think
| const HtmlWebpackPlugin = require('html-webpack-plugin'); | ||
| module.exports = { | ||
| // webpack will take the files from ./src/index | ||
| entry: './src/index', |
There was a problem hiding this comment.
does it work without specifying the extension?
webpack.config.js
Outdated
| // and output it into /dist as bundle.js | ||
| output: { | ||
| path: path.join(__dirname, '/dist'), | ||
| filename: 'bundle.js', |
There was a problem hiding this comment.
Probably better to use filename : 'bundle.[hash].js' so that user won't have to hard reload for changes to take effect
|
Overall this stuff shall improve the collaborative developer experience by a good magnitude 😄 |
package.json
Outdated
| }, | ||
| "scripts": { | ||
| "bundle": "webpack", | ||
| "test": "echo \"Error: no test specified\" && exit 1", |
There was a problem hiding this comment.
Make it exit 0 for now to pass the checks
| }, | ||
| "lint-staged": { | ||
| "*.{js,ts,tsx}": "eslint --cache --fix --ignore-path .eslintignore", | ||
| "*.{css,md}": "prettier --write --ignore-path .eslintignore" |
There was a problem hiding this comment.
prettier should apply to js/jsx/tsx too-
"lint-staged": {
"*.{js,ts,tsx}": "eslint --cache --fix --ignore-path .eslintignore",
"*.{js,ts,tsx,css,md}": "prettier --write --ignore-path .eslintignore"
},
There was a problem hiding this comment.
This plugin "plugin:prettier/recommended" in .eslintrc starts prettier with eslint.
So prettier indirectly applies to js/jsx/tsx.
There was a problem hiding this comment.
Okay, wasn't aware of that feat. Cool!
public/index.html
Outdated
| <noscript>You need to enable JavaScript to run this app.</noscript> | ||
| <div id="root"></div> | ||
| <!-- | ||
| <title>React App</title> |
There was a problem hiding this comment.
Do we really want to keep this title?
There was a problem hiding this comment.
Thank you for the review!
I replaced it with vMMX in 6e66da9.
Do you have suggestions for a better title?
There was a problem hiding this comment.
vMMX sounds good for me - maybe we can later extend it with the currently loaded song.
There was a problem hiding this comment.
Let's now add a favicon too now that we have a new logo 😄
There was a problem hiding this comment.
Okay nvm it's already added
- added babel plugins - fixed eslintrc, tsconfig, webpack.config - fixed spelling issues in manifest and index - updated dependencies
|
Merged into dev branch #31 |
|
If this PR is no longer nececcary, you can close it, @Udayraj123 |
As discussed on discord, i tried to change te setup from create-react-app to a manual setup
If you have any suggestions or corrections, please leave a comment.