-
Notifications
You must be signed in to change notification settings - Fork 47.2k
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
Update Rollup version to 1.19.4 and fix breaking changes #15037
Update Rollup version to 1.19.4 and fix breaking changes #15037
Conversation
There is another attempt already. |
Thanks for the reference, I see it is from 2018 Aug-ish, maybe that PR is in idle state? |
I have taken the legacy code changes from #13356 and merged into this branche. |
EDIT: This is no longer relevant. I merged the latest master into this feature branch, and now it fails due to
This is used by The getters transformer is due to supporting FB bundle and because the
|
React: size: -0.7%, gzip: -0.5% ReactDOM: size: -0.1%, gzip: -0.0% Details of bundled changes.Comparing: 34aaec6...58861bc react
react-dom
react-art
react-native-renderer
react-cache
scheduler
react-events
react-test-renderer
jest-react
react-is
react-reconciler
create-subscription
react-noop-renderer
react-stream
react-refresh
Generated by 🚫 dangerJS |
Hi @gaearon. Do you have some time to take a look at this? |
Details of bundled changes.Comparing: b789060...af015e1 react
react-dom
react-flight-dom-webpack
react-interactions
react-art
react-refresh
react-test-renderer
react-native-renderer
react-reconciler
create-subscription
jest-react
scheduler
react-cache
react-noop-renderer
react-flight
react-server
react-is
ReactDOM: size: -1.0%, gzip: -0.6% React: size: -0.8%, gzip: -0.7% Size changes (experimental) |
Details of bundled changes.Comparing: b789060...af015e1 react
react-dom
react-flight-dom-webpack
react-art
react-test-renderer
react-interactions
react-native-renderer
scheduler
react-flight
create-subscription
react-is
react-noop-renderer
react-reconciler
react-refresh
react-cache
react-server
jest-react
ReactDOM: size: 🔺+0.3%, gzip: 🔺+0.5% React: size: -1.4%, gzip: -1.1% Size changes (stable) |
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 6070bf0:
|
Since you're now publishing an alpha build for React, do you think now would be a good time to get this merged in? |
Is there anything I can do to help move this forward? |
Can you post the bundle diff? What has actually changed in the output? We usually do |
I have updated https://github.com/kunukn/reactjs-rollup-upgrade branch: rollup.0.52.1
Based on react source code master commit: b789060 There are a lot of files to compare. Here is Rollup 0.52.1 compared to Rollup 1.19.4 E.g. |
Great work. Thanks for keeping it up to date. I haven’t read the complete diff but what I’ve seen looks good to me. Let’s try it. |
Purpose:
Stay up to date with build tool.
Main changes:
package.json
Use latest rollup version and update the version of some of the rollup plugins.
Fix and use the rollup API breaking changes, mostly API naming changes.
https://rollupjs.org/guide/en#deprecated-hooks
Re-add
legacy behaviour
using externalLiveBindings=false Rollup flag. It was deprecated by newer Rollup version.definition of legacy option:
Rollup does not output getters when in legacy mode,[BREAKING] Deprecate the legacy option and thus IE8 support
https://github.com/rollup/rollup/blob/master/CHANGELOG.md#0600
Add ignore rollup warnings, it was breaking the build.
Rollup warning:
CIRCULAR_DEPENDENCY
BUILD
yarn build react/index,react-dom/index --type=FB_WWW_PROD
Rollup changelog.
https://github.com/rollup/rollup/blob/master/CHANGELOG.md
getter description.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/get
Babel plugin handbook
https://github.com/jamiebuilds/babel-handbook/blob/master/translations/en/plugin-handbook.md