-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Don't treat async functions as components #989
Conversation
This seems good, but please note that in React Fiber, this probably won't be the case, as you'll be able to use a promise for a component as if it were 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.
LGTM pending parser change
' return <div>Hello {props.name}</div>;', | ||
'}' | ||
].join('\n'), | ||
parser: 'babel-eslint' |
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.
there's no need to use babel-eslint here - let's test this with eslint's default parser, which fully support async function
now.
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.
I was following the convention down the file in
}, {
code: [
'var Hello = function(props) {',
' return <div>Hello {props.name}</div>;',
'}'
].join('\n'),
parser: 'babel-eslint',
errors: [{
message: '\'name\' is missing in props validation'
}]
}, {
code: [
'function Hello(props) {',
' return <div>Hello {props.name}</div>;',
'}'
].join('\n'),
parser: 'babel-eslint',
errors: [{
message: '\'name\' is missing in props validation'
}]
}, {
code: [
'var Hello = (props) => {',
' return <div>Hello {props.name}</div>;',
'}'
].join('\n'),
parser: 'babel-eslint',
errors: [{
message: '\'name\' is missing in props validation'
}]
}, {
Should those be changed as well?
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.
Actually, given "eslint": "^2.0.0 || ^3.0.0"
in package.json, i think we probably do need to stick with babel-eslint here - we'd have to raise the peer dep requirement to be able to rely on the default parser. Let's leave this as-is for now.
A promise for a component or a promise for an element? |
@taion a promise for a component, is my understanding. |
I'm not sure what you mean. A promise for a component class? These aren't that, and there are no component instances involved here anyway. These would be component classes that try to render a promise for elements, which wouldn't be the same as the parent of one of these components trying to render a promise for elements. |
Merged, thanks! |
This is just a silly little thing – there's no way for async functions to be components.