-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Upgrade dev build tooling (Babel 6 -> 7, Webpack 1 -> 4) #1235
Conversation
@@ -78,7 +78,7 @@ class Todos extends React.Component { | |||
todos.some(item => { | |||
if (item.id === id) { | |||
item.completed = completed; | |||
text = item.text; | |||
text = item.title; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This happened to be broken before. I noticed while testing the new config.
test: /\.js$/, | ||
loader: 'babel-loader', | ||
options: { | ||
...JSON.parse(readFileSync(resolve(__dirname, '../../.babelrc'))), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this spread operator really needed? You could remove the object braces and set options
to the result of JSON.parse
Maybe I'm missing something
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Originally, I thought I might add some custom options to individual configs but that turned out not to be necessary.
test: /\.js$/, | ||
loader: 'babel-loader', | ||
options: { | ||
...JSON.parse(readFileSync(resolve(__dirname, '../../.babelrc'))), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same thing as above (there are more occurences below, won't comment on those).
I am going to land this. I'll take responsibility for anything that pops up as a result and fix it with a follow-up PR. I think the more interesting PR to get feedback on is #1236 which this soft blocks. |
Some time ago, when updating Babel and Webpack (PR #1235) the 'libraryTarget' Webpack setting was accidentally changed from 'umd' to default (global) which, I believe, caused the subsequent 3.5.0 react-devtools/react-devtools-core releases to break. This change restores the 'libraryTarget' value to 'umd' for the backend.' git status
Our build tooling was super outdated, which made maintenance kind of a chore. I think this step, while not totally necessary, will help with issues like #1214 and #1090.
Build sizes
I also decided (at least unless someone knows of a good reason not to do this) to stick with Webpack's default minification behavior– which is to say, do it for production builds. This resulted in the following built size reductions:
Standalone:
Browser extension:
Build times
I timed
yarn build:standalone
andyarn build:extension:chrome
in master and this branch and didn't notice any real difference in durations. (The duration varies between runs but seems to be about the same overall.)Testing
I've tested the example app, plain shell, Firefox and Chrome extensions, standalone DevTools, and the theme viewer. All seem okay after this change.
I also updated the backend/integration Webpack config, although I'm pretty sure that test hasn't been used in a long time– since the source has an invalid require (referencing a file that was moved a while back).