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] Improve errors for invalid ViewConfig getter functions #16879

Merged

Conversation

JoshuaGross
Copy link
Contributor

Overview

Improve error messages around invalid ViewConfig getter functions.

Details

As a followup to #16821, it is unlikely but possible for ViewConfig registry entries to be corrupted. I would like to surface this as early as possible, and include a little more information in exceptions that could be thrown later on.

@sizebot
Copy link

sizebot commented Sep 24, 2019

No significant bundle size changes to report.

Generated by 🚫 dangerJS against 9eea5d5

try {
createReactNativeComponentClass('View', null)
} catch (e) {
throw new Error(e.toString());
Copy link
Member

Choose a reason for hiding this comment

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

🤔

} catch (e) {
throw new Error(e.toString());
}
}).toThrow('Invariant Violation: View config getter callback must be a function: View (received null)');
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't read super great to me.

What about:

Invariant Violation: Cannot create component View, view config getter must be a function (received null)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, that's reasonable but it's the registry that's throwing this error, not the createReactNativeComponentClass

Copy link
Member

Choose a reason for hiding this comment

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

What about

'Invariant Violation: View config getter callback for component `View` must be a function (received null)'

@JoshuaGross JoshuaGross force-pushed the rn-viewconfig-error-improvement branch 2 times, most recently from 900a6cc to f15d9dd Compare September 25, 2019 00:00
@elicwhite elicwhite merged commit 32e5c97 into facebook:master Sep 25, 2019
rickhanlonii pushed a commit to rickhanlonii/react that referenced this pull request Oct 9, 2019
…facebook#16879)

* [React Native] Improve logging for missing view configs and invalid view config getter functions

* [React Native] Improve logging for missing view configs and invalid view config getter functions
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Oct 9, 2019
Summary:
This diff is a partial sync of React into React Native.

## Source
The source branch is from my fork [here](https://github.com/facebook/react/compare/master...rickhanlonii:react-native-partial-sync-october-9?expand=1)

This branch is created from D17456249 which partially synced Dan's branch [here](facebook/react@master...gaearon:partsync).

To create my branch, I forked from Dan's branch and added two commits from these PRs:

- Joshua's PR to improve view config errors facebook/react#16879
- Eli's PR to remove setNativeProps warning facebook/react#17045

Reviewed By: gaearon

Differential Revision: D17828989

fbshipit-source-id: 75c99737f2dec4889d7d453bbdebaeb47656b5ce
youmoxiyou pushed a commit to youmoxiyou/react-native that referenced this pull request Oct 10, 2019
Summary:
This diff is a partial sync of React into React Native.

## Source
The source branch is from my fork [here](https://github.com/facebook/react/compare/master...rickhanlonii:react-native-partial-sync-october-9?expand=1)

This branch is created from D17456249 which partially synced Dan's branch [here](facebook/react@master...gaearon:partsync).

To create my branch, I forked from Dan's branch and added two commits from these PRs:

- Joshua's PR to improve view config errors facebook/react#16879
- Eli's PR to remove setNativeProps warning facebook/react#17045

Reviewed By: gaearon

Differential Revision: D17828989

fbshipit-source-id: 75c99737f2dec4889d7d453bbdebaeb47656b5ce
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