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

Strange behavior when passing action function into store.dispatch #1906

Closed
SerkanSipahi opened this issue Jun 3, 2019 · 4 comments · Fixed by #1914
Closed

Strange behavior when passing action function into store.dispatch #1906

SerkanSipahi opened this issue Jun 3, 2019 · 4 comments · Fixed by #1914
Labels

Comments

@SerkanSipahi
Copy link
Contributor

SerkanSipahi commented Jun 3, 2019

Minimal reproduction of the bug/regression with instructions:

I called someAction (see below simple example code) without function call "()" when it passed to store.dipatch(....)! The problem, it has not become an error. The action even arrived the reducer. Unfortunately it was not logged in the ReduxDevTools (it freezed, all other actions e.g. router will be ignored in the ReduxDevTools). Only with a MetaReducer logger I see what happens. After I called closeMainSidebarAction() with "()" the ReduxDevTools worked again.

We should prevent this behavior by throwing an error.

That was really frustrating! I think it took me more than 2 hours to realize that I forgot to call the function call. I think that the potential is high that another one will run into this strange behavior.

I will create a PR for it!

Code Example:

const someAction = createAction('Some Action');
store.dispatch(closeMainSidebarAction);

Expected behavior:

It should throw an error that to pass a function into dispatch is not allwed?

Versions of NgRx, Angular, Node, affected browser(s) and operating system(s):

ngrx v8 rc0

Other information:

I would be willing to submit a PR to fix this issue

[x] Yes (Assistance is provided if you need help submitting a pull request)
[ ] No

@brandonroberts brandonroberts added 8.x and removed 8.x labels Jun 3, 2019
@MikeRyanDev
Copy link
Member

It is likely that the same thing could happen in effects. Whatever the fix ends up being for store.dispatch should be done for effects as well.

@SerkanSipahi
Copy link
Contributor Author

@MikeRyanDev alright

@timdeschryver
Copy link
Member

Feel free to take this one @SerkanSipahi 👍

@timdeschryver timdeschryver added Accepting PRs community watch Someone from the community is working this issue/PR labels Jun 3, 2019
@SerkanSipahi
Copy link
Contributor Author

Thanks, i take it :)

SerkanSipahi added a commit to SerkanSipahi/platform that referenced this issue Jun 4, 2019
SerkanSipahi added a commit to SerkanSipahi/platform that referenced this issue Jun 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants