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

Effects generated by Schematics cause 100% CPU / Frozen tab #1524

Closed
adamduren opened this issue Jan 24, 2019 · 8 comments · Fixed by #1530
Closed

Effects generated by Schematics cause 100% CPU / Frozen tab #1524

adamduren opened this issue Jan 24, 2019 · 8 comments · Fixed by #1530

Comments

@adamduren
Copy link

Minimal reproduction of the bug/regression with instructions:

  1. Run ng generate @ngrx/schematics:feature
  2. Start the server
  3. Open page
  4. Observe browser freeze

This is because in the generated module the generated action is dispatched. The effects listen for that dispatched action however does not filter the action so it gets dispatched as a result of passing through the effects.

Expected behavior:

Dispatch loop does not occur.

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

"@angular/core": "7.2.2",
"@ngrx/effects": "7.0.0",
"@ngrx/router-store": "^7.0.0",
"@ngrx/store": "7.0.0",
"@ngrx/store-devtools": "7.0.0",
Node v10  

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

@wesleygrimes
Copy link
Contributor

Hi @adamduren I am attempting to recreate this issue and so far haven't had any luck. Here's what I have done.

  1. Generated new app with ng new test-app
  2. Installed ngrx dependencies
  3. Setup schematics
  4. Ran ng generate @ngrx/schematics:store State --root --module app.module.ts
  5. Ran ng generate feature User --group --reducers reducers/index.ts
  6. Injected the store into the app.component.ts class file
  7. Added the following to ngOnInit
ngOnInit() {
    this.store$.pipe(select(s => console.log('User', s.user)));
    this.store$.dispatch(new LoadUsers());
}

Are you doing something differently?
Thanks!

@timdeschryver
Copy link
Member

My guess is that the schematic generates the effect:

@Effect()
loadFoos$ = this.actions$.pipe(ofType(FoosActionTypes.LoadFoos));

Which causes the famous infinite loop ☠️ .

In order to fix this I think we can:

  1. create the effect as @Effect({dispatch:false})
  2. don't generate the effect
  3. add some more logic to the effect

But I think all these options make it more complicated for our users 🤔

@wesleygrimes
Copy link
Contributor

Can confirm.... Adding UserEffects to the EffectsModule.forRoot([]) causes the behavior described.

I vote for creating the effect class with the constructor and then leaving off the loadUsers$ line. Devs can then define and implement new effects as needed.

@adamduren
Copy link
Author

I used the @Effect{dispatch: false} method.

An alternative is to add a second action FooLoaded and have the effect mapTo(new FooLoaded()) to show that the all the feature components are registered and working.

However, I'm open for whatever is clear and fixes the behavior.

@adamduren
Copy link
Author

It's got me a few times and I'm always initially stumped for why my tab gets the spinner of doom.

@brandonroberts
Copy link
Member

I'd prefer to leave the effect in the class and comment it out. Its easier to delete the code, or uncomment it rather than having to add it. Slightly related to #1529 also.

@alyalin
Copy link

alyalin commented Jan 25, 2019

After upgrading to 7.x (from 6.x) , I have exactly the same problem

@brandonroberts
Copy link
Member

How was this not the case in 6.x? Generating a feature does not dispatch the newly created action, and if you dispatch that action before doing anything with the effect you will get caught in a loop.

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.

5 participants