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

UpdateEffects during prod build not behaving as intended #1447

Closed
timdeschryver opened this issue Nov 27, 2018 · 5 comments · Fixed by #1467
Closed

UpdateEffects during prod build not behaving as intended #1447

timdeschryver opened this issue Nov 27, 2018 · 5 comments · Fixed by #1467

Comments

@timdeschryver
Copy link
Member

Minimal reproduction of the bug/regression with instructions:

The following snippet works while working in dev mode, but fails in prod mode.
This is because constructor names are mangled by the CLI during a build, thus the check action.effects.includes('SomeEffectsClass') would always fail.

@Effect()
init = this.actions.pipe(
  ofType(UPDATE_EFFECTS),
  filter(action => action.effects.includes('SomeEffectsClass')),
  map(action => ...)
);

Expected behavior:

In order to get this working we should dispatch the UPDATE_EFFECTS action with an array of effect instances instead of the constructor names. This could be compared to the Angular router events. The above code snippet would become:

@Effect()
init = this.actions.pipe(
  ofType(UPDATE_EFFECTS),
  filter(action => action.effects.some(x => x instanceof SomeEffectsClass)),
  map(action => ...)
);

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

NgRx 7.0.0-beta.0

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

@timdeschryver timdeschryver changed the title UpdateEffects during prod build not working UpdateEffects during prod build not behaving as intended Nov 27, 2018
@timdeschryver
Copy link
Member Author

timdeschryver commented Nov 27, 2018

The downside of this would be that the action payload would be an effect, resulting in a big payload. E.g. the AuthEffects from the example app would have a 125K lines payload 😅 .

@timdeschryver
Copy link
Member Author

Another approach would be to use something as mentioned in #683 (comment):

EffectsModule.forFeature([
  Effect1,
  Effect2
], {
  initAction: { type: 'EffectsRegistedAction' }
})

@timdeschryver
Copy link
Member Author

timdeschryver commented Nov 27, 2018

Or to add a initAction property in the effect self, which will be used to dispatch the init action:

@Injectable()
export class AuthEffects {
  initAction = 'MY_INIT_ACTION';
  ...
}

@brandonroberts
Copy link
Member

We're going into lifecycle madness with all these hooks 😂 . Are there other options are there to reasonably getting the Effects class name in prod? I'm hesitant to introduce another stream for "Effects Events" as init would be the only one.

@timdeschryver
Copy link
Member Author

Not from what I saw when I encountered this, but I could be wrong.

My personal favorite would be a lifecycle method returning a user defined action. Because this would lean more to the effect methods (where we also dispatch the last action from the stream), and also because we already have some hooks now.

For example:

@Injectable()
export class AuthEffects implements OnInitEffects {
  ngrxOnInitEffects() {
     return { type: 'MY_CUSTOM_ACTION' };
  }
}

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