Skip to content

Remove dynamic modern event system flag for FB#19059

Merged
trueadm merged 2 commits intofacebook:masterfrom
trueadm:enable-modern-event-system
Jun 11, 2020
Merged

Remove dynamic modern event system flag for FB#19059
trueadm merged 2 commits intofacebook:masterfrom
trueadm:enable-modern-event-system

Conversation

@trueadm
Copy link
Copy Markdown
Contributor

@trueadm trueadm commented Jun 1, 2020

This makes the modern event system flag enabled statically for FB bundles.

@codesandbox-ci
Copy link
Copy Markdown

codesandbox-ci bot commented Jun 1, 2020

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 f0a564d:

Sandbox Source
practical-lalande-tguco Configuration

@facebook-github-bot facebook-github-bot added React Core Team Opened by a member of the React Core Team CLA Signed labels Jun 1, 2020
@sizebot
Copy link
Copy Markdown

sizebot commented Jun 1, 2020

Details of bundled changes.

Comparing: 1d85bb3...f0a564d

react-dom

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-dom.development.js 0.0% 0.0% 910.38 KB 910.38 KB 206.19 KB 206.2 KB NODE_DEV
ReactDOMForked-prod.js -8.3% -5.8% 431.35 KB 395.47 KB 77.92 KB 73.42 KB FB_WWW_PROD
react-dom-server.node.development.js 0.0% 0.0% 138.24 KB 138.24 KB 36.58 KB 36.58 KB NODE_DEV
ReactDOMForked-profiling.js -8.1% -5.6% 441.88 KB 406 KB 79.66 KB 75.18 KB FB_WWW_PROFILING
ReactDOMTesting-dev.js 0.0% 0.0% 937.33 KB 937.33 KB 209.44 KB 209.45 KB FB_WWW_DEV
react-dom-test-utils.development.js 0.0% 0.0% 70.09 KB 70.09 KB 19.6 KB 19.6 KB NODE_DEV
react-dom-test-utils.production.min.js 0.0% 0.0% 13.06 KB 13.06 KB 4.8 KB 4.81 KB NODE_PROD
react-dom-unstable-fizz.browser.development.js 0.0% +0.2% 5.36 KB 5.36 KB 1.8 KB 1.8 KB UMD_DEV
react-dom-unstable-fizz.node.production.min.js 0.0% 🔺+0.2% 1.17 KB 1.17 KB 665 B 666 B NODE_PROD
ReactTestUtils-dev.js -22.0% -18.5% 65.77 KB 51.33 KB 17.67 KB 14.39 KB FB_WWW_DEV
react-dom.development.js 0.0% 0.0% 956.26 KB 956.26 KB 208.79 KB 208.79 KB UMD_DEV
react-dom-unstable-fizz.browser.production.min.js 0.0% 🔺+0.2% 1.01 KB 1.01 KB 615 B 616 B NODE_PROD
ReactDOMForked-dev.js -6.6% -6.8% 1.02 MB 972.34 KB 233.05 KB 217.14 KB FB_WWW_DEV
react-dom.profiling.min.js 0.0% 0.0% 130.44 KB 130.44 KB 40.65 KB 40.65 KB NODE_PROFILING
ReactDOM-dev.js -6.6% -6.9% 1.01 MB 970.6 KB 230.93 KB 215.05 KB FB_WWW_DEV
ReactDOM-prod.js -8.3% -5.8% 434.04 KB 398.16 KB 76.9 KB 72.41 KB FB_WWW_PROD
ReactDOM-profiling.js -8.1% -5.7% 445.5 KB 409.63 KB 78.8 KB 74.31 KB FB_WWW_PROFILING
react-dom-server.browser.production.min.js 0.0% 0.0% 20.26 KB 20.26 KB 7.51 KB 7.51 KB NODE_PROD
ReactDOMServer-dev.js -0.0% -0.0% 142.63 KB 142.56 KB 36.29 KB 36.28 KB FB_WWW_DEV
ReactDOMServer-prod.js 0.0% 0.0% 46.58 KB 46.58 KB 10.9 KB 10.9 KB FB_WWW_PROD

Size changes (experimental)

Generated by 🚫 dangerJS against f0a564d

@sizebot
Copy link
Copy Markdown

sizebot commented Jun 1, 2020

Details of bundled changes.

Comparing: 1d85bb3...f0a564d

react-dom

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-dom.development.js 0.0% 0.0% 876.43 KB 876.43 KB 199.56 KB 199.56 KB NODE_DEV
ReactDOMForked-prod.js -8.1% -5.7% 442.54 KB 406.66 KB 79.79 KB 75.25 KB FB_WWW_PROD
ReactDOMForked-profiling.js -7.9% -5.5% 453.13 KB 417.25 KB 81.55 KB 77.03 KB FB_WWW_PROFILING
ReactDOMTesting-dev.js 0.0% 0.0% 963.12 KB 963.12 KB 214.98 KB 214.98 KB FB_WWW_DEV
react-dom-test-utils.development.js 0.0% 0.0% 70.08 KB 70.08 KB 19.59 KB 19.59 KB NODE_DEV
react-dom-test-utils.production.min.js 0.0% 0.0% 13.05 KB 13.05 KB 4.8 KB 4.8 KB NODE_PROD
ReactTestUtils-dev.js -22.0% -18.5% 65.77 KB 51.33 KB 17.66 KB 14.39 KB FB_WWW_DEV
react-dom.development.js 0.0% 0.0% 920.78 KB 920.78 KB 202.09 KB 202.1 KB UMD_DEV
ReactDOMForked-dev.js -6.4% -6.6% 1.04 MB 997.88 KB 238.74 KB 222.95 KB FB_WWW_DEV
ReactDOM-dev.js -6.4% -6.7% 1.04 MB 996.15 KB 236.52 KB 220.78 KB FB_WWW_DEV
ReactDOM-prod.js -8.1% -5.8% 445.11 KB 409.23 KB 78.75 KB 74.2 KB FB_WWW_PROD
ReactDOM-profiling.js -7.9% -5.6% 456.64 KB 420.76 KB 80.67 KB 76.13 KB FB_WWW_PROFILING
ReactDOMServer-dev.js -0.0% -0.0% 146.67 KB 146.59 KB 37.29 KB 37.28 KB FB_WWW_DEV
react-dom-test-utils.development.js 0.0% 0.0% 75.25 KB 75.25 KB 20.09 KB 20.09 KB UMD_DEV

Size changes (stable)

Generated by 🚫 dangerJS against f0a564d

@trueadm trueadm marked this pull request as ready for review June 4, 2020 10:09
@trueadm trueadm requested a review from gaearon June 4, 2020 10:09
Comment thread scripts/rollup/modules.js
'react-dom/server': HAS_NO_SIDE_EFFECTS_ON_IMPORT,
'react/jsx-dev-runtime': HAS_NO_SIDE_EFFECTS_ON_IMPORT,
'react-fetch/node': HAS_NO_SIDE_EFFECTS_ON_IMPORT,
'react-dom': HAS_NO_SIDE_EFFECTS_ON_IMPORT,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this new?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a block of code in ReactTestUtils that is wrapped in the feature flag. Inside this block we reference an import from react-dom. When we remove the block, we end up with having a dangling import to react-dom at the top of the module without it being used. In turn, this causes Rollup to fail the build.

This will organically go away when we remove the flags entirely from OSS.

@trueadm trueadm merged commit e3ccdf1 into facebook:master Jun 11, 2020
@trueadm trueadm deleted the enable-modern-event-system branch June 11, 2020 18:49
@trueadm trueadm changed the title Remove synamic modern event system flag for FB Remove dynamic modern event system flag for FB Jun 12, 2020
linjiajian999 pushed a commit to linjiajian999/react that referenced this pull request Jun 16, 2020
* Remove synamic modern event system flag for FB
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed React Core Team Opened by a member of the React Core Team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants