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

Effect runs when there was no change at the end #56071

Open
Harpush opened this issue May 24, 2024 · 6 comments
Open

Effect runs when there was no change at the end #56071

Harpush opened this issue May 24, 2024 · 6 comments
Assignees
Labels
area: core Issues related to the framework runtime core: reactivity Work related to fine-grained reactivity in the core framework cross-cutting: signals
Milestone

Comments

@Harpush
Copy link

Harpush commented May 24, 2024

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

core

Is this a regression?

No

Description

If I have a signal and an effect:

name = signal<string | undefined>('Angular');

effect(() => console.log(this.name()));

And later I do: this.name.set('Angular'); the effect won't run as expected.

But doing (in sync):

this.name.set(undefined);
this.name.set('Angular');

Will trigger the effect even tough nothing actually changed.
I would expect it not to run...

Please provide a link to a minimal reproduction of the bug

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

Please provide the exception or error you saw

The effect ran and console log happened

Please provide the environment you discovered this bug in (run ng version)

Angular 17 windows 10

Anything else?

No response

@JeanMeche
Copy link
Member

Effects have no sense of value history.
When a signal is updated, it marks its consummers as dirty. When an effect that consumes a signal is markAsDirty, it will schedule itself.
This why you are observing that the effect re-run even if the signal value appears to be the same.

If you need to tree each value independently, signal are the wrong pattern and you should look at using observables.

@Harpush
Copy link
Author

Harpush commented May 24, 2024

Effects have no sense of value history. When a signal is updated, it marks its consummers as dirty. When an effect that consumes a signal is markAsDirty, it will schedule itself. This why you are observing that the effect re-run even if the signal value appears to be the same.

If you need to tree each value independently, signal are the wrong pattern and you should look at using observables.

I wonder when is it expected for the effect to run if the signal didn't change? I mean the logic will probably execute the same as nothing actually changed.
A bit weird this is wanted...

@JeanMeche
Copy link
Member

Effects schedule themeselves via a microtask (+ some extract behaviors in the context of components).

Your signal did change twice:

  1. 'angular => undefined
  2. undefined => 'angular'
    Both those value changes mark the consumers as dirty, because the value did actually change ('angular' => 'angular' wouldn't have mark them as dirty).

Signals are about being notified eventually when a state is dirty, not individual value updates.

@Harpush Harpush changed the title Effect runs when there was not change at the end Effect runs when there was no change at the end May 26, 2024
@Harpush
Copy link
Author

Harpush commented May 26, 2024

Effects schedule themeselves via a microtask (+ some extract behaviors in the context of components).

Your signal did change twice:

  1. 'angular => undefined
  2. undefined => 'angular'
    Both those value changes mark the consumers as dirty, because the value did actually change ('angular' => 'angular' wouldn't have mark them as dirty).

Signals are about being notified eventually when a state is dirty, not individual value updates.

That's unfortunate... Like computed won't compute if there was no change at the end I thought effect should too... I still can't see a use case for it to be honest

@JeanMeche
Copy link
Member

Computed also re-run with the same usecase.

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

@pkozlowski-opensource pkozlowski-opensource added area: core Issues related to the framework runtime core: reactivity Work related to fine-grained reactivity in the core framework cross-cutting: signals labels May 27, 2024
@ngbot ngbot bot modified the milestone: needsTriage May 27, 2024
@Harpush
Copy link
Author

Harpush commented May 28, 2024

At least it is consistent although in my opinion consistently bad.
It means for computed as state if two updates happen which result in no change the computed will still run unnecessarily and might trigger further computed runs which will be more expensive. And you can't hail out of it.
For effect you can't rely on it as a real change notifier and must take all deps as observables with distinctUntilChanged... For example emit output on computed change must be through observable and other cases too.

To sum it - this seems unexpected

@pkozlowski-opensource pkozlowski-opensource self-assigned this May 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: core Issues related to the framework runtime core: reactivity Work related to fine-grained reactivity in the core framework cross-cutting: signals
Projects
None yet
Development

No branches or pull requests

3 participants