Task2 webpack #2
Conversation
config/webpack.config.common.ts
Outdated
| entry: resolve("src", "index.tsx"), | ||
| output: { | ||
| path: resolve("build"), | ||
| filename: "[name].[chunkhash].bundle.js", |
There was a problem hiding this comment.
Calculating content hash is relatively expensive and there's no point doing that in development mode, so, probably, this particular part should be specific to production mode only.
There was a problem hiding this comment.
Okay, will move it to prod build
| template: join("src", "index.html"), | ||
| }), | ||
| ], | ||
| optimization: { |
There was a problem hiding this comment.
That would, probably, make more sense for production mode only.
There was a problem hiding this comment.
I don't agree here. Having a separate vendor chunk is useful in dev mode as well. It's better to keep the two modes as similar as possible to avoid prod bugs which not be reproduced on dev mode.
Also, it's a generally accepted practice to have a separate vendor chunk rather then build everything in one file
package.json
Outdated
| "webpack-merge": "^5.8.0" | ||
| }, | ||
| "scripts": { | ||
| "start": "cross-env NODE_ENV=dev webpack server", |
There was a problem hiding this comment.
You should bear in mind that cross-env is no longer developed, which may be the reason of its declined popularity, so you may opt for corresponding command-line parameter, consider alternative solution or expose webpack config as a function and pass current mode as a command line parameter.
There was a problem hiding this comment.
No problem here. Should work fine for this project's life-time 🙂
Thanks for the notification. Didn't know it's deprecated.
There was a problem hiding this comment.
Specified webpack configuration instead.
Probably need to update HW description to use webpack env flag or specify config file in the script
Have DEV and PROD build commands (use env variables)
|
|
||
| ReactDOM.render( | ||
| <React.StrictMode> | ||
| <GlobalStyles /> |
There was a problem hiding this comment.
You may, possibly, want to wrap your <App /> into ThemeContext.Provider to share some common styling parameters (palette, font settings, etc) across your entire app.
There was a problem hiding this comment.
Will look into that in the scope of the next HW since I didn't work with styled-components before. Thanks
| <GlobalStyles /> | ||
| <App /> | ||
| </React.StrictMode>, | ||
| document.querySelector("#root") |
There was a problem hiding this comment.
querySelector() is drastically slower than more conventional getElementById(). It won't, probably, hurt performance of the entire app in your particular case (since used only once), however, the latter looks much more comprehensive, while the former is better suited for more sophisticated queries.
There was a problem hiding this comment.
Probably slower. But it's micro-optimization in this case, that's not an issue.
Prefer readability over performance and optimize only when needed.
For me, who worked with jQuery querySelector is more intuitive to use.
No description provided.