NGRX/Store is super difficult to adopt in real projects when Signal is used internally

[montella1507]

Which @ngrx/* package(s) are the source of the bug?

store

Minimal reproduction of the bug/regression with instructions

I know everyone is super-hyped about signals, the problem is, NGRX/STORE@16 is forcing developers to use signals with all of their disadvantages without actual usage of the signals from developers themselves.

In every second application the developer is doing something like:

this.store.select(a).pipe(
tap(()=> this.store.dispatch(ActionChangingB())
)

Specially when defining observables async pipe is subscribing to.
so it is almost impossible to mirgate to NGRX16 without hundreds of hours invetsed to migration.

You are basically forcing developers to follow signal ideas and their problems.

Minimal reproduction of the bug/regression with instructions

Just modify the state in tap() action...

this.store.select(x => x).pipe(
tap(()=> this.store.dispatch())
);
And Subscribe to this observable in template.

you can find this code in every bigger than demo Angular applicaiton.

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

Latest NGRX/STORE.

Other information

Please, let the developers decide if they want to use Signals internally.... They ARE NOT "drop-in" replacement.

There could be lot of time spent to mke it working atleast in @effects, with that toggle boolean.. but this is only one place and it is still SO huge amount of work.

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

• Yes
• No

[montella1507]

Related:
angular/angular#50382

[Armenvardanyan95]

Let me drop my opinion on this.

1. Well, to get started, I do not believe NgRx (and even Angular for that matter) forces anyone to use signals. They are in developer preview, meaning you probably should not use them (at least extensively) in production codebases. As far as NgRx goes, it just provided us with a selectSignal method to play with signals rather than Observables.
2. Angular has provided an official, clear-cut, and well-defined way of interoperability between Signals and NgRx. Using the tap operator or a similar approach is, in essence, ditching the official way and doing an experiment. I don't think it is reasonable to think that library authors are going to implement measures that prevent developers from making "bad" (in someone's opinion) experiments with the code, just provide a "clean" way and official guidance
3. NgRx also has an eslint plugin that allows preventing some code like the one you mentioned. For example, using tap on a selected property, and so on. Maybe we can discuss the possibility of adding a new ruleset specifically for signal-based NgRx Store
4. Please notice the code you provided could be easily replicated without signals at all - it is neither a signals-specific problem nor even an NgRx problem - we can create such situations by using purely RxJS
5. I don't think this is an "issue" - rather a specific that may or may not have become widespread among developers. If anything, maybe you want to add guidelines to the team, or write an article on best practices? That could really be helpful.

Hope this helps

[montella1507]

1. ngrx/store uses signal internally and as far as they Are based on "Ambient context" where signal Is inside And effect on top level, IT doesnt matter if u use signals or not, their limitation (like in this case) affect you no matter what

2. i dont see a connection

3. nope IT cannot, exception Is throw because, And ONLY because signal Is set inside effect(), there Is specific exception throw tightly coupled with signals.

The problém Is, angular team came with something marked as "optional" And in "developer preview" And IT breaks 100% working code FOR YEARS WITHOUT ANY problem.

And as we can see in all of these issues, they Are not trying to find Real solution, but they are rather going to MARK ALL OF THOSE WORKING SOLUTIONS as "bad practice" And malé them stop working at all.

[montella1507]

This Is 100% legit code with no issues:

@Component({ selector: 'my-app', standalone: true, imports: [CommonModule], template: {{obs$ | async}}, }) export class App { sig = signal(3); obs$ = of(3).pipe(tap(this.sig.set)); }

And the reality IT throws error Is really not "optional signals" And "non-breaking" developer preview.

There Is a problém with hidden effect() from framework side. IT is used somewhere to wrap template even though,.there Is NOT any signál used in template

This Is what i am talking about. Signals And their leaky implementation.

[brandonroberts]

Moving to a discussion as its a duplicate of #3892

[Armenvardanyan95]

@montella1507 so you mean the fact that internally the state is stored as a signal? I made a small demo without zero usage of signals and dispatching action via tap from a selected state seems to work: https://stackblitz.com/edit/angular-d6yvcn?file=src%2Fmain.ts

However, your next example mentions setting a signal inside the tap. Is that what really causes problems? Because if it is, it is easily fixable (albeit probably still a bug on NgRx side of things)

[montella1507]

https://stackblitz.com/edit/angular-ehcsk4?file=src%2Fmain.ts check this example:

• 0 signals from "my / developer" side
• 0 effects from "my / developer" side
• just simple select from NGRX store + dispatch in tap.. so it is exactly THE SAME user scenario as mapping one action to another in NGRX/EFFect (pattern mentioned from several NGRX creators)

And... Still error in the console:

ERROR
Error: NG0600: Writing to signals is not allowed in a computed or an effect by default. Use allowSignalWrites in the CreateEffectOptions to enable this inside effects.

SO NOPE. There is NO SIGNAL, NO EFFECT, NO setting signal from "my/developer" side.

[montella1507]

  id$ = this.store
    .select(appFeature.selectId)
    .pipe(tap((id) => this.store.dispatch(Actions.setDataID({ dataId: id }))));

= error in console.

[Armenvardanyan95]

@montella1507 upon further investigation I noticed that your issue really is only reproducible when setting a signal from "inside" of an observable (any observable, not just NgRx). While this is a bug, it doesn't really "force" you to use signals - you can still build with NgRx and Angular without them quite easily; also, this could be avoided by proper rxjs-interop usage (use toSignal instead of setting the signal from tap); also, this is RxJS + Signals problem, as far as I can see, nothing that indicates there is a problem stemming from the NgRx implementation

[montella1507]

Please check this example https://stackblitz.com/edit/angular-ehcsk4?file=src%2Fmain.ts mentioned above. It is reproducible without any "my" usage of signals / effects. Just NGRX store.

[e-oz]

But why are you mutating the state in a selector??

[montella1507]

This Is operator on observable.

[e-oz]

You are calling “select” here and “a” is memoized selector. Every subscriber will dispatch an action, which can cause an effect, which can modify the state. Let me know if I understood it wrong.

[montella1507]

Yes but this is not "mutating the state in a selector".

Mutating state in selector would be something like

const x = createSelector(selectFeature, (state) => {
  store.dispatch();
  return state;
})

what am i doing here is sideeffect, but this is super common. This is EXACTLY the same, like NGRX/effects are doing...

There is even special operator directly form NGRX/TEAM (concatLatestFrom) which let you react on action, then get latest state from store and then, you can trigger side effect...

and This problem has nothing to do with selector() - look i have created example for you 👍

https://stackblitz.com/edit/angular-np1ley?file=src%2Fmain.ts

  id$ = of(5).pipe(
    tap((id) => this.store.dispatch(Actions.setDataID({ dataId: id })))
  );

• The problem is not selector
• Problem is, that signals are "ambient context" based implementation, that means, it is basically a magic.

NOW, we are in situation, when:

• developer has been developing his Applications for X years and they used to work without any issues
• something NEW (signals) have leaky implementation, are added as "optional"
  = new hard to debug and hard to fix bugs

And this all, for not real reason.

[Armenvardanyan95]

Not I get the problem, although to mention dispatching in select is not a blog practice. It could cause problems in purely RxJS scenarios too (affect the same state as the one selected, for instance). You mention you are doing a side effect - then why not place it in effects?

Anyway, this comes from a problem inside Angular's signals implementation. Maybe NgRx rushed a bit with making the state itself a signal, but anyway, you can downgrade for a while - you are not using selectSignal anyway

[montella1507]

Yes, HOWEVER IT is nothing bad, when you Are listening to changes of A and patching B. This Is Common practice in case of - fór the instance, cascade loadings (load A, then load children of A, then children od their children etc).

---

Yes, this Is exactly what i want to say, do not Rush to developer previews, because they may have leaky.implementation And IT May break entire application.

Signals Are overhyped And not yet ready to adopt in serious releases of libraries.

[montella1507]

The real example behind this is easy. For the instance:

• cascade loading

LoadA -> when loaded (data in store) -> dispatch load of child entity based on parent entity data (And set loading state to store).

Please do not suggest workarounds... Of course IT could Have been.rewriten in another way.. but IT Will také hundreds of Hours because And ONLY because there Is somewhere unneccessary effect() in angular (even when there is no signal used in template at all) in "optional signals" and signal () Is Now used in store....

[marko-jobcloud]

Signal support won't be dropped in order to support bad practices that worked before.

I shared the quick fix in this comment which is pretty straightforward => schedule store.dispatch calls to the micro/macrotask queue:

@Component({
  template: 'slice$ | async'
})
export class MyComponent {
  private store = inject(Store);

  slice$ = this.store.select(selectSlice).pipe(
    tap(() => {
-      this.store.dispatch(myAction());
+     asapScheduler.schedule(() => this.store.dispatch(myAction()));
    })
  ); 
}

[brandonroberts]

@montella1507 continually complaining about this is not productive. If you have a solution or recommendation in mind, lets keep the discussion on topic.

[marko-jobcloud]

@montella1507 You need to decide what you want:

• If you want a quick fix, you can use the scheduler.
• If you want to avoid bad practices and improve your codebase then take a look at the linked comment again, it contains the example of how to separate state reads via async pipe from side effects.

And don't be cynical. You don't have to thank people who try to help you, but at least show some respect.

[montella1507]

Ok, you are both correct and i should have acted differently. I am sorry of that.

Please, let's discuss about 2 things.

1. why is that practice considered bad
2. if it is considered bad, is it OK to "let developers behind" and make them to fix big part of their applications because of the really early adoption of "developers preview" like Angular Signals?

---

Let's have a situation where we need to cascade load several entities:
Parent (have ids of children)
-> Children (have ids of their children)
-> Grand children

---

Without NGRX/Store and NGRX/Effects, developer will do something like.

this.data$ =  this.dataService.loadParent().pipe(
  switchMap(parent => this.dataService.loadChildren(parent)),
  switchMap(children => this.dataService.loadGrandChildren(children)) 
);

This is not anything bad, right? And what if the developer wants to have "cascade loading in UI" too?
= show parent info when parent is loaded.. then continue loading children.. after the children have been loaded, he wants to show children etc... To implement this, the easiest and maybe only? possible way how to do this is "tap" to state?

this.data$ =  this.dataService.loadParent().pipe(
  tap(parent => this.parent = parent, parentLoad = false,..),
  switchMap(parent => this.dataService.loadChildren(parent)),
  tap(parent => this.children = children,  childrenLoad = false,..),
  switchMap(children => this.dataService.loadGrandChildren(children))
);

Is this bad? And how is it different from the ngrx/component-store and effect?
https://ngrx.io/guide/component-store/effect

Why is this antipattern:

this.data$ =  this.dataService.loadParent().pipe(
  tap(parent => this.parent = parent, parentLoad = false,..),
  switchMap(parent => this.dataService.loadChildren(parent)),
  tap(parent => this.children = children,  childrenLoad = false,..),
  switchMap(children => this.dataService.loadGrandChildren(children))
);

And why is this - from your point of view this OK? (just little bit edited example from documentation)

this.effect((req$: Observable<void>) => {
    return req$.pipe(
         tap(parent => this.patchState( {parent: parent, parentLoad : false,..}),
         switchMap(parent => this.dataervice.loadChildren(parent)),
      tap(children => this.patchState( {children: children, childrenLoad : false,..}),
         switchMap(children => this.dataService.loadGrandChildren(children))
      )),
    );
  });

Where is the difference? Why is this considered "best practice" and "dispatching" to store inside component is anti-pattern?

this.data$ = req$.pipe(
         tap(parent => this.store.dispatch( ...)),
         switchMap(parent => this.dataervice.loadChildren(parent)),
         tap(children => this.store.dispatch( ...)),
         switchMap(children => this.dataService.loadGrandChildren(children))
      )),
    );
  });

[montella1507]

Because... from my POV, it is exactly the same thing. Only little bit different api (tap => patchState() inside component/store VS. store.dispatch() inside component).

And the situation would be pretty similar if i take in @ngrx/effects but i didn't use them because they are further from component, so i compared "component solution" to "component store".

[xtephan]

@marko-jobcloud can you please elaborate on why is it a bad practice?

In our app we have a lot of situation where, for performance reason, we only fetch the minimal information on page load, followed by sub-entities on demand, similar to what @montella1507 is describing.

I am genuinely curious how this cases are supposed to be handled in ordered to be considered a good practice.

For the time being, we ended up just rolling back to ng 15 because of this issue

[matiasfs12]

Not sure if you @montella1507 have tried that, using *LetDirective ngrxLet, runtime error is gone, I´m wondering why. (#3892 (comment) thanks @hoi4)

I work on a really large application, with tons of obs.pipe(tap(() => this.store.dispatch(someAction))) and the fix that @marko-jobcloud proposed above, it does not feel safe to me, I might be wrong though.
I´ll try to correct the "bad practice", but it will take to my team quite a few time.
For now I will switch to LetDirective as a temporary workaround.

EDIT: I´ve updated your Stackblitz sample https://stackblitz.com/edit/angular-ardhq7

[montella1507]

I am waiting for the response from NGRX guys on this comment of mine:

#3903 (reply in thread)

because i still do not think we are doing anything different than NGRX is doing itself inside ngrx/component-store examples

Respectively, only one difference i see is, that ngrx/component-store is using queueScheduler inside.

[brandonroberts]

This may resolve the issue we're seeing with the select/dispatch/async pipe combination.

angular/angular#50522

Will try it out after the PR lands

[brandonroberts]

Angular v16.0.4 has been released, including the fix to resolve the original issue. We've also released NgRx 16.0.1, including the fix for the @ngrx/component package.