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

'Bind' will result in errors in onClick handler #6306

Closed
superddr opened this issue Mar 21, 2016 · 9 comments
Closed

'Bind' will result in errors in onClick handler #6306

superddr opened this issue Mar 21, 2016 · 9 comments

Comments

@superddr
Copy link

If you create any react component, and add this attribute to it:
onClick={this.setState.bind(this, {smKey:smValue} )}

then you click it, it will report an error:

Uncaught Invariant Violation: enqueueCallback(...): You called setProps, replaceProps, setState, replaceState, or forceUpdate with a callback that isn't callable.

but actually it's working, the state will be correctly updated, nothing wrong, just the annoying error.

I think this is a bug, but I can not image how could it be, when this can work:
onClick={()=>this.setState({smKey,smValue})}

to me they are exactly the same things.

@gaearon
Copy link
Collaborator

gaearon commented Mar 21, 2016

Would you like to investigate why this happens?

@petetnt
Copy link
Contributor

petetnt commented Mar 21, 2016

Related to #5172 (comment)?

@syranide
Copy link
Contributor

This happens because the API is this.setState(data, callback), when you bind it and pass it to an event callback the event argument ends up in the callback parameter and it's obviously not callable.

@gaearon
Copy link
Collaborator

gaearon commented Mar 21, 2016

Ah, right. The problem is you bind only the first argument, the second one becomes the event from the event handler. As a result setState thinks it’s the callback argument, and fails. You can fix this by using setState.bind(this, { ... }, null) so the second argument is ignored.

We can probably provide a better invariant message when we receive an event instead of the callback because bind() is a very common use case where people encounter this.

@gaearon
Copy link
Collaborator

gaearon commented Mar 21, 2016

I would like to reopen until we provide a better error message or decide it is not necessary.

@gaearon gaearon reopened this Mar 21, 2016
@gaearon
Copy link
Collaborator

gaearon commented Mar 21, 2016

It appears that a better error message was added in #5193.
However I would still argue that

Uncaught Invariant Violation: enqueueCallback(...): You called setProps, replaceProps, setState, replaceState, or forceUpdate with a callback of type object. A function is expected

is not much better for this particular case, and we can explain the problem much more specifically.

@zpao
Copy link
Member

zpao commented Mar 21, 2016

I think this is a good argument for having pages on our site for each invariant / warning where we can go much more in depth than we can in a string. We can show specific cases like this where we show common "bad" code that might be triggering it. Adding all the code to React that does all of the possible debugging gets tricky. Especially in a case like there where you could probably just do an instanceof SyntheticEvent check, but that breaks the boundary between the core and DOM, so you would need to go about it in a roundabout way.

@gaearon
Copy link
Collaborator

gaearon commented Mar 21, 2016

Put up #6310 to address this.

@gaearon
Copy link
Collaborator

gaearon commented Mar 22, 2016

I will close this as we can continue the discussion in #6310.

@gaearon gaearon closed this as completed Mar 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants