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

Add warning when passing an action to autorun #576

Closed
jeffijoe opened this issue Sep 24, 2016 · 8 comments
Closed

Add warning when passing an action to autorun #576

jeffijoe opened this issue Sep 24, 2016 · 8 comments

Comments

@jeffijoe
Copy link
Contributor

I spent 30 minutes today wondering why my autorun wasn't triggering. Turns out it was because I was doing this:

autorun(action(() => {}))

and actions are untracked (as I found out in #490). I think it would be a good idea to add a development-only warning when attempting to set up autorun with an action.

Implementing it shouldn't be too difficult - the function being returned from action could have a tag (a Symbol or just a privatized key) that is checked when creating an autorun.

Example warning:

Warning: attempted to pass an action to autorun. Actions are untracked and will not trigger on state changes. Use `reaction` or wrap only your state modification code in an action.
@mweststrate
Copy link
Member

Would you mind creating a PR? Should be pretty straightforward, since isAction already exists

@jeffijoe
Copy link
Contributor Author

I can try, hehe. :)

@jeffijoe
Copy link
Contributor Author

@mweststrate adding this check breaks existing tests.

image

This is all I added:

image

@jeffijoe
Copy link
Contributor Author

It appears to be related to using spies.

@mweststrate
Copy link
Member

can you turn it into a PR, then I'll verify. might be error in the test

Op zo 25 sep. 2016 16:09 schreef Jeff Hansen [email protected]:

It appears to be related to using spies.


You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#576 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABvGhCGKCeSlmHjICtpprGZFACeV3_YNks5qtoCIgaJpZM4KFvow
.

@jeffijoe
Copy link
Contributor Author

Will do!

@mweststrate
Copy link
Member

Released as part of 2.6.0

@jeffijoe
Copy link
Contributor Author

jeffijoe commented Oct 4, 2016

Sweet!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants