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

[React Native] Fix for view config registrations #16821

Merged
merged 1 commit into from
Sep 18, 2019

Conversation

rickhanlonii
Copy link
Member

@rickhanlonii rickhanlonii commented Sep 18, 2019

Overview

This change fixes a behavior in the React Native view config registry which would hide errors when registering components.

Details

Before this change, even if we failed to register the view config for any reason (e.g. the events are invalid), we would still clear the callback for the view config.

This means, the next time that a user renders the component and we check for the view config, we would show this error:

View config not found for name Slider

This error is wrong. The real error is masked (and may never be shown, if the registration happened before error handling is set up), and may be something like:

Event cannot be both direct and bubbling: topChange

This PR fixes this behavior by clearing the view config callback only after the view config is successfully registered.

Screen

Screen Shot 2019-09-18 at 2 28 04 PM

Test

  • Added test that fails without this change

@sizebot
Copy link

sizebot commented Sep 18, 2019

No significant bundle size changes to report.

Generated by 🚫 dangerJS against 10b4ced

Copy link
Collaborator

@gaearon gaearon left a comment

Choose a reason for hiding this comment

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

Sure

Copy link
Member

@elicwhite elicwhite left a comment

Choose a reason for hiding this comment

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

Great. Thanks for adding tests for this.

// fail so that we don't mask the above failure.
expect(() => ReactNative.render(<InvalidEvents />, 1)).toThrow(
'Event cannot be both direct and bubbling: topChange',
);
Copy link
Member Author

Choose a reason for hiding this comment

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

Without the change in this PR, this test will fail because the view config is not found

Screen Shot 2019-09-18 at 3 30 12 PM

@rickhanlonii rickhanlonii merged commit 6ecfa90 into facebook:master Sep 18, 2019
gaearon pushed a commit to gaearon/react that referenced this pull request Sep 18, 2019
@rickhanlonii rickhanlonii deleted the rh-rn-view-config-fix branch October 9, 2019 09:17
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.

5 participants