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
Improve cryptic error message when creating a component starting with a lowercase letter #12421
Conversation
|
@hramos @fkgozali @sebmarkbage Does this PR look ok? I’d like to get it into the next React release if possible. |
|
I'll need to defer to the React team here. |
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.
Let's remove the edit to codes.json and use "wrote" instead of "made".
I also think we should turn this into two different invariants.
if (typeof callback !== 'function') {
if (typeof name[0] === 'string' && /[a-z]/i.test(name[0])) {
invariant(false, 'View config not found for %s. Make sure you begin all component names from a capital letter.', name);
} else {
invariant(false, 'View config not found for %s.', name);
}
}So that we don't show it if it's not relevant.
scripts/error-codes/codes.json
Outdated
| @@ -221,7 +221,7 @@ | |||
| "219": "getInspectorDataForViewTag() is not available in production", | |||
| "220": "Container does not support insertBefore operation", | |||
| "221": "Tried to register two views with the same name %s", | |||
| "222": "View config not found for name %s", | |||
| "222": "View config not found for %s. If you made a component starting with a lowercase letter, make it uppercase.", | |||
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.
Please don't edit this file. It's generated automatically on releases. Editing it now means people on older versions won't be able to find this error anymore.
| @@ -44,7 +44,7 @@ export function get(name: string): ReactNativeBaseComponentViewConfig { | |||
| const callback = viewConfigCallbacks.get(name); | |||
| invariant( | |||
| typeof callback === 'function', | |||
| 'View config not found for name %s', | |||
| 'View config not found for %s. If you made a component starting with a lowercase letter, make it uppercase.', | |||
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.
"made a component" is a bit odd, maybe "wrote"?
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.
How about we skip the verb entirely?
"If your component starts with a lowercase letter, make it uppercase."
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.
Sounds good
|
@gaearon I made your requested changes and reverted my changes to |
| viewConfigCallbacks.set(name, null); | ||
| viewConfig = callback(); | ||
| viewConfigs.set(name, viewConfig); | ||
| } else { | ||
| viewConfig = viewConfigs.get(name); | ||
| } | ||
| invariant(viewConfig, 'View config not found for name %s', name); | ||
| if (typeof callback !== 'function') { |
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.
This condition doesn't match what it used to be.
|
Thanks! |
This PR adds a recovery suggestion to the error that appears when creating a component starting with a lowercase letter. (e.g. if you try to render
<myComponent/>instead of<MyComponent/>)The change passes both react and react-native test suites locally.