-
Notifications
You must be signed in to change notification settings - Fork 46.9k
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
Adds more scaffolding for experimental event API #15112
Conversation
ReactDOM: size: 🔺+0.1%, gzip: 🔺+0.1% Details of bundled changes.Comparing: daeda44...fc45944 react-dom
react-art
react-native-renderer
react-test-renderer
react-noop-renderer
react-reconciler
Generated by 🚫 dangerJS |
I've addressed the feedback. I'll follow up on any other concerns in subsequent PRs. :) |
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.
Would be good to have some just very basics tests for these cases. Not only does help expose bugs/regressions, but it's also documenting where you're going with this.
@sebmarkbage I've added tests and moved all the logic checking into host context. This simplifies a bunch of cases and moves the checking logic into the renderer, which makes far more sense. |
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.
Hm. The mutation thing won't work with Suspense etc.
@sebmarkbage I had to make the test |
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.
Two minor nits but looks good to go otherwise.
* Adds more scaffolding for experimental event API
This PR is a follow up to #15108. This adds the rest of the experimental event API scaffolding logic. Like the previous PR, this is only intended for internal FB testing right now. I also added flags around everything, so bundle size shouldn't regress other than a few bytes (if it does, I've done something wrong with DCE).
I've also added relevant fiber tests to ensure the warnings fire as expected.