-
Notifications
You must be signed in to change notification settings - Fork 47k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Babel 7 #16297
Babel 7 #16297
Conversation
React: size: -0.4%, gzip: -0.2% ReactDOM: size: -0.7%, gzip: -0.7% Details of bundled changes.Comparing: d77c623...348fae3 react
react-dom
react-art
react-native-renderer
react-test-renderer
eslint-plugin-react-hooks
react-events
react-reconciler
create-subscription
Generated by 🚫 dangerJS |
…ere being imported
package.json
Outdated
"babel-plugin-check-es2015-constants": "^6.5.0", | ||
"babel-plugin-external-helpers": "^6.22.0", | ||
"babel-plugin-syntax-dynamic-import": "^6.18.0", | ||
"babel-jest": "^24.8.0", |
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.
you can remove babel-jest
entirely, it's not used
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.
Will do. Thanks!
babel.config.js
Outdated
'use strict'; | ||
|
||
module.exports = { | ||
presets: ['@babel/preset-react', '@babel/preset-flow'], |
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.
Can we remove these presets and use the plugins directly? That way unexpected stuff can't sneak in.
describe('ReactFreshBabelPlugin', () => { | ||
beforeEach(() => { |
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.
The reason Fresh throws an error if you're not in development environment is because the production build of React does not support hot reloading. React uses a process.env.NODE_ENV
check (that's what __DEV__
compiles to), not BABEL_ENV
. Technically, BABEL_ENV
and NODE_ENV
could be different, even though they're usually the same.
We should use __DEV__
/NODE_ENV
since that's what determines the version of React DOM.
if (__DEV__) {
// The tests
} else {
it('throw error if environment is not development', ...);
}
babel.config.js
Outdated
plugins: [ | ||
'@babel/plugin-syntax-jsx', | ||
'@babel/plugin-transform-react-jsx', | ||
'@babel/plugin-transform-react-display-name', |
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.
Let's remove this one. Don't need it.
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.
🎉🎉🎉🎉🎉🎉🎉🎉🎉
Ping me before you merge so we can make sure the commit gets attributed properly
Nice work @lunaruan ! |
// fail due to obsolete snapshots | ||
process.env.NODE_ENV === 'development' | ||
? '<rootDir>/packages/react-refresh/src/__tests__/ReactFreshBabelPluginProd-test.js' | ||
: '<rootDir>/packages/react-refresh/src/__tests__/ReactFreshBabelPlugin-test.js', |
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 is kinda hacky. Can we figure out another way without making exceptions at this level?
@@ -134,12 +134,26 @@ const bundles = [ | |||
entry: 'react-dom/server.browser', | |||
global: 'ReactDOMServer', | |||
externals: ['react'], | |||
babel: opts => | |||
Object.assign({}, opts, { | |||
// Include JSX |
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.
The comment doesn't match the intention.
I would merge this first #15037 |
For #12548
Upgraded from Babel 6 to Babel 7.
The only significant change seems to be the way
@babel/plugin-transform-classes
handles classes differently frombabel-plugin-transform-es2015-classes
. In regular mode, the former injects a_createClass
function that increases the bundle size, and in the latter it removes the safeguard checks. However, this is okay because we don't all classes in new features, and we want to deprecate class usage in the future in the react repo.Differences between classes in Babel 6 (left) and Babel 7 (right)
Example diffs (6 on the left and 7 on the right):
ReactDOM-prod.js (facebook-www)
ReactDOM-dev.js (facebook-www)
react-dom.development.js (UMD)
react-dom.production.js (UMD)
react-dom.development.js (CommonJS)
react-dom.production.js (CommonJS)
react-dom-server.node.production.min.js (CommonJS)
react-dom-server.browser.production.min.js (UMD)