Hello.
I appreciate the intent and your work.

What do you think about changing effect()'s API a little, to run its function always outside of the reactivity context:

const a = signal(0);
const b = signal(0);

effect([a, b], ( [a, b] ) => {
  if (a < 1) {
    console.log(b);
  }
});

The first argument is the list of signals to listen.

It is still possible to trigger an endless loop here (if allowSignalWrites: true), but all of the potential endless scenarios you described in this pull request comment - they will all be eliminated.

Hello.

I appreciate the intent and your work.

What do you think about changing effect()'s API a little, to run its function always outside of the reactivity context:

const a = signal(0);

const b = signal(0);



effect([a, b], ( [a, b] ) => {

  if (a < 1) {

    console.log(b);

  }

});

The first argument is the list of signals to listen.

It is still possible to trigger an endless loop here (if allowSignalWrites: true), but all of the potential endless scenarios you described in this pull request comment - they will all be eliminated.

This is great for library code as there nothing Angular needs to do to support this API in the outside.

Hello. I appreciate the intent and your work.

What do you think about changing effect()'s API a little, to run its function always outside of the reactivity context:

const a = signal(0);
const b = signal(0);

effect([a, b], ( [a, b] ) => {
  if (a < 1) {
    console.log(b);
  }
});

The first argument is the list of signals to listen.

It is still possible to trigger an endless loop here (if allowSignalWrites: true), but all of the potential endless scenarios you described in this pull request comment - they will all be eliminated.

Actually this can be implemented now in user code... although I agree it should be inside angular core as it is useful:

type SignalsArrayValues<T extends Signal<any>[]> = {
  [Index in keyof T]: T[Index] extends Signal<infer P> ? P : never;
};

function explicitEffect<T extends [Signal<any>, ...Signal<any>[]]>(
  signalDeps: T,
  effectFn: (
    ...params: [
      ...deps: SignalsArrayValues<T>,
      onCleanup: EffectCleanupRegisterFn
    ]
  ) => void,
  options?: CreateEffectOptions | undefined
) {
  return effect((onCleanup) => {
    const deps = signalDeps.map((signalDep) =>
      signalDep()
    ) as SignalsArrayValues<T>;

    untracked(() => effectFn(...deps, onCleanup));
  }, options);
}

// Usage:
const a = signal(2);
const b = signal('s');
const c = signal(true);

explicitEffect([a, b], (a, b) => console.log(a, b, c()));

setTimeout(() => {
  // Triggers
  a.set(7);
}, 2000);

setTimeout(() => {
  // Won't trigger
  c.set(false);
}, 4000);

My idea is about a paradigm shift for effect().

Automatic tracking of dependencies is one of the selling points of Angular Signals.
But, so far, only auto-tracking in computed() works safely (if a user is not trying to intentionally bypass safety limitations).

If effect() would track dependencies explicitly, not automatically, it would be a move away from that paradigm.

But there would be a positive outcome too:

1. All the awful things, described in this PR would disappear;
2. computed() would remain a happy (and the most suitable) place for automatic deps tracking;
3. Users would learn about two different ways of deps tracking and their trade-offs;
4. Because effect() would become relatively safe, documentation would not need to discourage users from using effect(), and all you would need is to explain how they can cause an endless loop. You could even check in runtime, if effect() is trying to write into the signals it uses in the dependencies list.

I support @e-oz in tracking dependencies explicitly. It could also be done via a fourth function, as suggested by @markostanimirovic in #54548 (comment).

Personally, I'd prefer overloading effect. For implicit tracking, I would require the user to enable it "explicitly." Just to be sure, they are aware of what they are doing:

const a = signal(0);
const b = signal(0);

effect({implicitTracking: true}, () => {
  if (a() < 1) {
    console.log(b());
  }
});

I would also mark the current signature of effect as deprecated.

Having explicit tracking by default can avoid a lot of confusion. I also think we should teach alternatives to effect but there are some situations where effects cannot be prevented.

The current effect() function is causing a lot of discussion within my team.

For example, let's consider a simple use case: We have a signal articleId, and when it changes, some data need to be fetched and a loading state has to be managed. What is the best practice in Angular 17? The loading state cannot be a signal because, by default, within an effect, signals cannot be updated.

I saw on reddit almost same disuccsion with a lot of different answers: https://www.reddit.com/r/Angular2/comments/1b0in0y/react_to_state_change_in_angular/

With Signals we have another option to manage state. It should be more clear what the best practices are.
For myself I like the idea of a watch function of @markostanimirovic in #54548 (comment).
Then effect can be used for console.log etc and watch for updating other signals

@hannie I feel you.

I think, in most cases, you can just use the event that leads to the change in the articleId. It could be a click event or some RxJS observable telling you that the current selection changed.

Saying this, I think there are a few corner cases where this strategy does not work, and all solutions like toObservable or other awesome community-based solutions like rxMethod in SignalStore or connect in ngxtension also use effects in the one or other way underneath the covers.

This PR was merged into the repository by commit ffad7b8.

@pkozlowski-opensource, @alxhub, I think we had a good discussion about a potential future for the effect here. Is there a GitHub issue, which is dedicated to that topic and where we could continue?

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.