Skip to content
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

Modern Event System: refactor legacy FB support logic #18336

Merged
merged 2 commits into from
Mar 19, 2020

Conversation

trueadm
Copy link
Contributor

@trueadm trueadm commented Mar 18, 2020

This is a pretty big refactor of the FB event support logic in the modern event system. Previously, the logic worked for only Primer, but on more internal cases occuring, it seems we should probably delegated all "click" events to after the internal tooling runs, which fixes all the known issues with the legacy FB event systems. To ensure this works better, I've also made all the modern event system tests run with and without the internal FB flag so we catch more cases with this change.

@sizebot
Copy link

sizebot commented Mar 18, 2020

No significant bundle size changes to report.

Size changes (stable)

Generated by 🚫 dangerJS against 7cc9fca

@sizebot
Copy link

sizebot commented Mar 18, 2020

Details of bundled changes.

Comparing: 1cf4a17...7cc9fca

react-dom

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-dom-unstable-fizz.browser.development.js 0.0% -0.2% 3.35 KB 3.35 KB 1.27 KB 1.27 KB UMD_DEV
ReactDOM-dev.js 0.0% 0.0% 937.35 KB 937.35 KB 208.52 KB 208.53 KB FB_WWW_DEV
react-dom-unstable-fizz.browser.production.min.js 0.0% -0.1% 1.19 KB 1.19 KB 698 B 697 B UMD_PROD
ReactDOM-prod.js 🔺+0.1% 🔺+0.1% 383.69 KB 384.1 KB 69.82 KB 69.85 KB FB_WWW_PROD
ReactDOM-profiling.js +0.1% 0.0% 401.28 KB 401.69 KB 72.85 KB 72.88 KB FB_WWW_PROFILING
react-dom-unstable-fizz.browser.development.js 0.0% -0.2% 2.95 KB 2.95 KB 1.18 KB 1.18 KB NODE_DEV
ReactDOMTesting-dev.js 0.0% 0.0% 892.29 KB 892.32 KB 199.42 KB 199.42 KB FB_WWW_DEV
ReactDOMTesting-prod.js 🔺+0.1% 🔺+0.1% 376.98 KB 377.4 KB 68.92 KB 68.96 KB FB_WWW_PROD
react-dom-test-utils.development.js 0.0% -0.0% 54.78 KB 54.78 KB 15.08 KB 15.08 KB NODE_DEV
ReactDOMTesting-profiling.js +0.1% +0.1% 376.98 KB 377.4 KB 68.92 KB 68.96 KB FB_WWW_PROFILING
react-dom-server.node.development.js 0.0% -0.0% 130.95 KB 130.95 KB 34.78 KB 34.78 KB NODE_DEV
react-dom-test-utils.production.min.js 0.0% -0.0% 12.13 KB 12.13 KB 4.48 KB 4.48 KB NODE_PROD
react-dom-server.browser.development.js 0.0% -0.0% 129.73 KB 129.73 KB 34.53 KB 34.53 KB NODE_DEV
react-dom.development.js 0.0% 0.0% 933.76 KB 933.81 KB 203.95 KB 203.96 KB UMD_DEV
ReactDOMForked-dev.js 0.0% 0.0% 937.35 KB 937.35 KB 208.52 KB 208.53 KB FB_WWW_DEV
ReactDOMForked-prod.js 🔺+0.1% 🔺+0.1% 383.69 KB 384.1 KB 69.82 KB 69.85 KB FB_WWW_PROD
react-dom.development.js 0.0% 0.0% 888.91 KB 888.95 KB 201.45 KB 201.46 KB NODE_DEV
ReactDOMForked-profiling.js +0.1% 0.0% 401.28 KB 401.69 KB 72.85 KB 72.88 KB FB_WWW_PROFILING
react-dom-unstable-native-dependencies.development.js 0.0% 0.0% 54.72 KB 54.72 KB 14.14 KB 14.14 KB NODE_DEV
react-dom-unstable-fizz.node.development.js 0.0% -0.2% 3.69 KB 3.69 KB 1.34 KB 1.34 KB NODE_DEV
react-dom-unstable-fizz.node.production.min.js 0.0% -0.2% 1.16 KB 1.16 KB 662 B 661 B NODE_PROD

Size changes (experimental)

Generated by 🚫 dangerJS against 7cc9fca

@codesandbox-ci
Copy link

codesandbox-ci bot commented Mar 18, 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 7cc9fca:

Sandbox Source
modest-ramanujan-2opg2 Configuration

@trueadm trueadm force-pushed the fix-fb-listener branch 2 times, most recently from 0f5e333 to 66f7c8d Compare March 18, 2020 13:23
Copy link
Contributor

@threepointone threepointone left a comment

Choose a reason for hiding this comment

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

will stamp if you can clarify the comment about eventSystemFlags & LEGACY_FB_SUPPORT) === 0, please and thanks!

Copy link
Contributor

@threepointone threepointone left a comment

Choose a reason for hiding this comment

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

one last nit, but lgtm!

@trueadm trueadm merged commit 8311cb5 into facebook:master Mar 19, 2020
@trueadm trueadm deleted the fix-fb-listener branch March 19, 2020 13:22
sthagen added a commit to sthagen/facebook-react that referenced this pull request Mar 19, 2020
Modern Event System: refactor legacy FB support logic (facebook#18336)
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