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

Move EventComponent state creation to complete phase + tests #15352

Merged
merged 1 commit into from
Apr 8, 2019

Conversation

trueadm
Copy link
Contributor

@trueadm trueadm commented Apr 8, 2019

Note: This is for the experimental event API.

This PR moves the creation of EventComponent state to always happen in the complete phase, rather than happen lazily upon the first event firing. This ensures that state is consistent, otherwise a component that hasn't had state created (because an event didn't fire), will likely break in the unmount phase because state is expected to exist. I added a test that validates this behaviour.

I also removed the currentResponder binding as it is no longer used anywhere and was just dead code.

@sizebot
Copy link

sizebot commented Apr 8, 2019

Details of bundled changes.

Comparing: 745baf2...e0c93ae

react-art

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-art.development.js 0.0% 0.0% 639.4 KB 639.67 KB 138.09 KB 138.14 KB UMD_DEV
react-art.production.min.js 0.0% 0.0% 97.31 KB 97.31 KB 29.77 KB 29.77 KB UMD_PROD
react-art.development.js 0.0% 0.0% 570.62 KB 570.88 KB 120.8 KB 120.86 KB NODE_DEV
react-art.production.min.js 0.0% -0.0% 62.31 KB 62.31 KB 19.04 KB 19.04 KB NODE_PROD
ReactART-dev.js 0.0% +0.1% 581.72 KB 582 KB 120.5 KB 120.56 KB FB_WWW_DEV
ReactART-prod.js 🔺+0.2% 🔺+0.2% 198.64 KB 198.95 KB 33.69 KB 33.75 KB FB_WWW_PROD

Generated by 🚫 dangerJS

@trueadm trueadm merged commit 29fb586 into facebook:master Apr 8, 2019
@trueadm trueadm deleted the responder-state-phase branch April 8, 2019 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants