toSignal() can not be called inside computed()

[e-oz]

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

core

Is this a regression?

No

Description

toSignal() can not set internal state (state.set() when called from computed(), because of NG0600 ("Writing to signals is not allowed in a computed or an effect by default").

Please provide a link to a minimal reproduction of the bug

https://stackblitz.com/edit/angular-hjphb7?devToolsHeight=33&file=src/main.ts

Please provide the exception or error you saw

ERROR
Error: NG0601: `toSignal()` called with `requireSync` but `Observable` did not emit synchronously.

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.

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

@angular/core 16.0.0-rc.4

Anything else?

It is reproducible without Component's template, so the issue is not caused by the template: https://stackblitz.com/edit/angular-rbp4uq?devToolsHeight=33&file=src/main.ts

[pkozlowski-opensource]

This is by design - the computed function should be a simple, pure computation. In this sense the error catches a scenario which we explicitly don't think should be possible.

Having said this I would be curious to hear more about your use-case, specific tasks that you are trying to cover.

[pkozlowski-opensource]

Obviously you can always do https://stackblitz.com/edit/angular-hvbbhq?devToolsHeight=33&file=src%2Fmain.ts

[e-oz]

I like the design aspect where compute() doesn't allow modifying Signals.

I use compute() for a simple, pure computation. It's not my "sin" that toSignal() uses a Signal under the hood and tries to modify it. It should return a Signal, and this Signal I want to use inside the computed() - just to read it, for a simple pure computation.

My use-case

I have a library, ngx-collection.

It has a lot of observables (17), users are supposed to pick just a few of them to use what is needed.

I want to provide signal-based alternatives for every observable. It is better to do this from my side, to don't make users think about things like initial value and injection context.

If I will use toSignal() in the field declaration, then it will instantly subscribe to the observable. So I will have 17 subscriptions, and maybe just a couple of them will be actually used.

To avoid that, I use getters:

export class CollectionService<T> {
  // ...

  public get updatingItems(): Signal<T[]> {
    return toSignal(this.updatingItems$, {initialValue: [] as T[], injector: this.injector});
  }

  public get deletingItems(): Signal<T[]> {
    return toSignal(this.deletingItems$, {initialValue: [] as T[], injector: this.injector});
  }

  public get mutatingItems(): Signal<T[]> {
    return computed(() => ([
      ...this.updatingItems(),
      ...this.deletingItems()
    ]));
  }

  // ...
}

As you can see, I do not set any Signals inside the computed(), and computation here is pure and simple :)

If I go this way:

  public get mutatingItems(): Signal<T[]> {
    return toSignal(this.mutatingItems$, {initialValue: [] as T[], injector: this.injector});
  }

then by the signature, I'm providing a Signal, and everything is fine... until the user will try to use this Signal inside the computed() or effect() (and maybe even in a template).

If I go this way:

public readonly updatingItems = toSignal(this.updatingItems$, {initialValue: [] as T[], injector: this.injector});

then I'll have 17 subscriptions "just in case".

---

My main idea is: calling signal.set() inside the toSignal() is limiting its usage.

[pkozlowski-opensource]

As you can see, I do not set any Signals inside the computed(), and computation here is pure and simple :)

The toSignal() operation is not pure - as you've noted it setups subscription etc.

[pkozlowski-opensource]

If I go this way, then I'll have 17 subscriptions "just in case".

This is one of the fundamental differences between signals and Observables - a signal always needs a value so in this sense it can not be "lazy". I see 3 options:

• if you want to expose signals, you would have to expose them and subscribe eagerly;
• continue exposing observables and let people convert to signal as needed
• expose signals only and let people convert to observables if needed.

I'm not familiar with the library in question to say which approach is the best

[e-oz]

This is one of the fundamental differences between signals and Observables

I'm pretty much aware, it's one of the tradeoffs.

a signal always needs a value so in this sense it can not be "lazy"

I don't want them to be "lazy", I just want to create them only when it's needed.

I'm not familiar with the library in question to say which approach is the best

None of them, unfortunately, otherwise I wouldn't have created this issue. But still, thank you for your time and ideas. I'll keep looking for a workaround.

[e-oz]

I found a workaround for this use case! I still hope you'll reconsider toSignal() implementation one day, but for now, I'll use this trick:

export class CollectionService<T> {
  // ...

  private updatingItemsSignal?: Signal<T[]>;

  public get updatingItems(): Signal<T[]> {
    if (this.updatingItemsSignal) {
      return this.updatingItemsSignal;
    }
    return this.updatingItemsSignal = untracked( // 🙈
      () => toSignal(this.updatingItems$, {initialValue: [] as T[], injector: this.injector})
    );
  }

  private mutatingItemsSignal?: Signal<T[]>;

  public get mutatingItems(): Signal<T[]> {
    if (this.mutatingItemsSignal) {
      return this.mutatingItemsSignal;
    }

    // 👉 here we are getting Signals outside of the `computed()` call
    const updatingItems = this.updatingItems;
    const deletingItems = this.deletingItems;

    return this.mutatingItemsSignal = computed(() => ([
      ...updatingItems(),
      ...deletingItems()
    ]));
  }

  // ...
}

Please close the issue if you think that toSignal() doesn't need to be modified.

[e-oz]

Just in case, all you would need to change is this:

    next: value => () => state.set({kind: StateKind.Value, value}),
    error: error => () => state.set({kind: StateKind.Error, error}),

to this:

    next: value => untracked(() => state.set({kind: StateKind.Value, value})),
    error: error => untracked(() => state.set({kind: StateKind.Error, error})),

😉

[pkozlowski-opensource]

You can certainly cache signals returned from the toSignal invocations. At this point we don't want to change the toSignal implementation - signal instances need to have a value and should be initialized with one - even if converting from observables. Reading signals should be side-effects free, otherwise it becomes less predictable and harder to reason about.

Thnx for sharing the use-case though!

[angular-automatic-lock-bot]

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.}