-
-
Notifications
You must be signed in to change notification settings - Fork 622
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
Throw when component is missing #4
Throw when component is missing #4
Conversation
Shouldn't this be an error, rather than a warning? Otherwise the previous |
@vadimdemedes fixed. |
lib/h.js
Outdated
@@ -4,6 +4,10 @@ const flatten = require('lodash.flattendeep'); | |||
const VNode = require('./vnode'); | |||
|
|||
module.exports = (component, props, ...children) => { | |||
if (typeof component !== 'function' && typeof component !== 'string') { | |||
throw new TypeError(`ink.h: expected component to be a function, but received "${component}". You may have forgotten to export a component.`); |
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.
Could you remove ink.h
and start with Expected
?
Also, replace "${component}"
with typeof component
.
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.
Fixed
test/h.js
Outdated
@@ -163,3 +163,10 @@ test.failing('don\'t merge children of components', t => { | |||
|
|||
t.deepEqual(node.props.children, ['x', 'y']); | |||
}); | |||
|
|||
test('warns on invalid type', t => { | |||
t.throws(() => { |
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.
=> t.throws(() => h(), 'Expected component to be a function, got undefined. You may have forgotten to export a component.');
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.
Fixed
test/h.js
Outdated
@@ -163,3 +163,10 @@ test.failing('don\'t merge children of components', t => { | |||
|
|||
t.deepEqual(node.props.children, ['x', 'y']); | |||
}); | |||
|
|||
test('warns on invalid type', t => { |
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.
warns
=> throws
invalid type
=> missing component
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.
Fixed
Thanks! |
React has a similar warning. Ran into it when working on #3