angular: Angular 17 - Mutable signals are not supported

I noticed that a computed signal does not always fire when one of the dependencies has updated. Let me give a quick example:

@Component({
  selector: 'app-root',
  standalone: true,
  imports: [CommonModule, RouterOutlet],
  template: '
  <ul>
    @for(person of filteredPersons(); track person.name) {
      <li>
        {{ person.name }} has {{ person.money }} dollars
      </li>
    }
  </ul>
  <button (click)="previousPage()">Previous Page</button>
  <button (click)="nextPage()">Next Page</button>
  <small>Page {{ pagingState().page }} of {{ totalPages() }}</small>
  ',
  styles: [],
})
export class AppComponent {

  private persons = this.personService.persons;

  filteredPersons = computed(() => {
    console.log('One of the dependents has changed :D')
    const persons = this.persons();
    const pagingState = this.pagingState();
    const amountToSkip = (pagingState.page - 1) * pagingState.pageSize;
    return persons?.slice(amountToSkip, amountToSkip + pagingState.pageSize);;
  })

  pagingState = signal({
    page: 1,
    pageSize: 5
  })

  totalPages = computed(() => Math.ceil(this.persons()?.length / this.pagingState().pageSize));

  constructor(private personService: PersonService) {}

  nextPage(): void {
    this.pagingState.update((state) => {
      if (state.page < this.totalPages()) state.page += 1
      return { ...state}; // Cant return state directly since it wont pickup the change whyy????
    })
  }

  previousPage(): void {
    this.pagingState.update((state) => {
      if (state.page > 1) state.page -= 1
      return { ...state}; // Cant return state directly since it wont pickup the change whyy????
    })
  }
}`

Service
`  private _persons = signal<Person[]>(undefined);
  get persons() {
    return this._persons.asReadonly();
  }

  hasLoaded = computed(() => this.persons() !== undefined);

  constructor() {
    this.refresh();
  }

  getAll(): Observable<Person[]> {
    return of([
      {
        name: 'Alice',
        money: 150,
      },
      {
        name: 'Bob',
        money: 280,
      },
      {
        name: 'Carol',
        money: 210,
      },
      {
        name: 'David',
        money: 320,
      },
      {
        name: 'Eva',
        money: 180,
      },
      {
        name: 'Frank',
        money: 270,
      },
      {
        name: 'Grace',
        money: 190,
      },
      {
        name: 'Helen',
        money: 230,
      }
    ]).pipe(delay(500))
  }

  refresh(): void {
    this._persons.set(undefined);
    this.getAll().subscribe((persons) => {
      this._persons.set(persons);
    });
  }```

Whenever I press one of the page change buttons the pagingState gets updated with the new page value. The correct state is displayed in html but for some reason the computed signal filteredPersons doesnt get triggered. Apparantly it does not see the change which I would have expected. Returning a shallow copy of the state in nextPage and previousPage solves this. If this is expected behavior it would seem really counterintuitive.

About this issue

  • Original URL
  • State: closed
  • Created 8 months ago
  • Reactions: 3
  • Comments: 50 (16 by maintainers)

Commits related to this issue

Most upvoted comments

@e-oz we absolutely do read through the issues and are painfully aware of the trade-offs of enforcing immutable data. Working with mutable data structures has lots of advantages but also sharp edges. Similar statement could be made about immutable data. Those are not trivial choices.

We are working on a longer blog post explaining in details our reasoning so far and potential improvements for people that would continue to work with the mutable data structures. Will communicate more as soon as we’ve got the details ironed out.

Again, we are absolutely listening, hearing and understanding the pain-points. Just carefully weighting pros and cons of different approach. Stay tuned and thnx for taking time to write down your thoughts.

#53446 should address this issue - check the PR for more details.

This is working as expected. Signals do not support mutable data. If Object.is returns true, the signal will not notify dependents (see https://github.com/angular/angular/pull/51821 and https://github.com/angular/angular/pull/52465).

@alxhub @pkozlowski-opensource Should this be included in the breaking changes notes even though signals were in developer preview and is there sufficient documentation about this?

I’ve played a bit with a “possible” solution for MutableSignal in user-land based on the wrapper idea + custom EqualityFn that try to optimize O(1) evaluation of equality for mutable objects using a counter of changes (similar to initial implementation of signal/computed notification tree) You can find my playground here 👀 WDYT ?

I’ve tryed my POC only with @e-oz sample, but I think that this can handle even @rainerhahnekamp Form sample, and the same approach can help even with mutation of Set/Map/DOMelements ... if you use the mutate method.

PS: I know that writing $sig().value.xxxx is quite annoying (no-one like boilerplate) but this tricks is my 1st (or 2nd) iteration to the solution, maybe if you find it valueable and validated by someone of the Team, we can experiment with Proxy or other way to lower complexity 🤔

🙏 @alxhub and @pkozlowski-opensource take a look on it, just to check & validate my approach if you see something totally crazy / or problem with the Signal propagation or maybe in other iteration with CD & internal FW mechanism that honestly I don’t know in deep!

@txone

s.update((oldSet) => {
  const newSet = new Set(oldSet);
  newSet.add('some item');
  return newSet;
});

Similar code for Map and Date, but for many classes there is no such easy way to recreate them using an existing instance.

This is some extreme memory thrashing. Not an option in a real world scenario.

Angular guys, please consider an option to bring back v16 signal behaviour (through any means, e.g. CreateSignalOptions).

@rainerhahnekamp provided a good example that aligns with my opinion about signals and reactivity:

But signals are not only for change detection. There might be layers and layers of computed signals before the result is consumed by the change detection logic.

That code example works fine with v17.0.0-rc.1, but doesn’t work after https://github.com/angular/angular/pull/52465.

The example code might look too simple and easy to fix, but it’s just because it’s an example, and the code there should be small enough to read and understand this code quickly.

Still, we can see there that reactivity is lost not in the template - it is lost in the computed() signal because the custom equality function is ignored.

In the template, {{message()}} should generate a new string every time, so CD will get a new value and will, in fact, detect the change. So it is not a similar situation to my code example. Here we see the exact reason why short-circuiting custom equality functions causes damage to the signals-based reactivity in general.

Let’s take a look at the whole picture, an example of the application layers structure:

API COMMUNICATION SERVICES
ABSTRACTION LAYER 
STATE MANAGEMENT
ABSTRACTION LAYER
TEMPLATE

In this structure, in every layer except the TEMPLATE, ignoring custom equality functions is not justified. There is only one layer where we have a reason to do this.

in @rainerhahnekamp’s example message = computed() is the layer above the TEMPLATE.

Angular Team, please consider creating some workaround that would not damage the signals-based reactivity in general, just for the CD layer.

Maybe you could create some LView flag - you know these things much better than me.

signal.mutate will be back?

@txone no - although you can partly simulate it, for the signals you create, with a custom equality function.

@for (and NgFor) doesn’t care about the identity of the array. It diffs the array on every change detection cycle.

That’s new info for me, thanks, I thought that ngFor receives its input similarly to other directives/components.

Is it possible to make something similar for signals in the template?

It does. NgForOf isn’t using ngOnChanges to decide when to diff, but instead uses ngDoCheck to diff every time the host view is checked. @for behaves similarly.

@dmorosinotto

With this approach you are creating a new Mutable object reference on each mutate() call. Therefore, it also works w/o custom equal fn.

I guess the underlying concerns about working w/ mutable state is still the same. But I am very much looking forward reading the mentioned blog post by the Angular team, explaining their thoughts in detail.

Well @mikezks your statement is partially correct, but with the custom Mutable.equal you can “skip” some signal notification if it return false (maybe using _alter_ to change value - must be discussed if wanted/needed 🤔) but this approach has another advantage: it keeps the same mutable object reference inside #value without needing to use “typical immutable” with spread {...} or other recreate of object and it is performant O(1) and totally agnostic of object nature instead of a real deep equal compare for mutable object”.

I’d love to use signals in all those layers as well 😢

I’m sorry for spamming everyone who subscribed to this issue.

After some thinking and experimenting, I found that I, actually, understood Alex correctly, and before that change (first appeared in 17.0.0-rc.2) mutable signals will NOT work as expected. I could not notice it because in the (really) rare cases when I was using mutable structures, I was never using them in the template as is (as a whole object), I was always using some of their fields. And, because of that, CD had different values on every mutation (CD was comparing values of the fields of the mutated object). So if you use some field of a mutable structure, then it doesn’t matter if you use a custom equality function or not - CD doesn’t care.

The only case where you could use a signal with a mutable value a value itself, not as fields of that value, is [input].

Here is my experimenting playground that illustrates @alxhub words: https://stackblitz.com/edit/stackblitz-starters-2hge8w?file=src%2Fmain.ts

The only workaround I see here is to add some new state of LView: not “dirty”, but “re-render is definitely needed” (CD will not compare old and new values, but just trigger re-rendering). This would not only help with mutable signals, but also would save unneeded comparison - if a signal itself compared them already, then CD doesn’t need to do it again.

Signals do not support mutable data.

Angular Signals can use mutable data, @atscott , if a custom equality function is provided.

Like this one:

export function equalPrimitives<T>(a: T, b: T): boolean {
  return (a === null || typeof a !== 'object') && Object.is(a, b);
}

Any non-primitive value will always be considered non-equal.

You can provide an equality check function in the second argument of signal() or computed().

[!IMPORTANT] Functions like I provided might be useful in computed() and other layers of reactivity, although for the signals that you use in the template, it’s better to use either the default equality check function or a function that will emit notifications even less frequently than the default one. And if you provide a custom function - it should work really fast.

It happened to me too, I do an update and nothing is notified.

Ex:

this.orderQuery.update((value) => {
    value.orderFilter = 'all';
    return value;
});

Primitive types are notified but objects are not

My point of view is that as a developer I’d like to have the possibility to store any objects inside signals and mutate them without necessarily using the “immutable approach” and at the same time have the responsibility & control of when to “trigger propagation notifications” or NOT! 😉 Possibly without breaking the automatic update/ recalculation of computed/effects that signals promise as a reactive primitive 🤓 - More or less what I could do before #51821 with the mutate method and writing custom equal logic to decide how to compare my objects.

With the implementation that I proposed, it is true that Mutable<T> actually always emits new references in response to mutate, but this is done only to overcome the Object.is “short-circuit of equal logic” introduced in #52465 - So I decided to re-create new Mutable wrapper, but maintaining control of the equal function (which only controls internal #changes) and having new instance wrapper will helps even with CD problem for @Input sink (at least from what it seems to me, but must be validated by @alxhub). In either case with this approach we have the possibility to decide whether to change/mutate the ref #value using mutate an have the notification propagate through the signal-tree, or “don’t let them fire” using _alter_ (🤔 We need to find a better name for that method! or a better “api/dx-ergonomics” to mutate or change without propagation) … and BONUS: all of this without needing to write a custom logic for complex compare of two mutable objects (it’s just a number compare that take O(1) - credits for original idea by @pkozlowski-opensource and @alxhub 🤯)

This is my 2cents and my motivations WDYT? Hope to see this discussion going on with the community and the Team to find a better way to accomodate all ours need!

@dmorosinotto

With this approach you are creating a new Mutable object reference on each mutate() call. Therefore, it also works w/o custom equal fn.

I guess the underlying concerns about working w/ mutable state are still the same. But I am very much looking forward reading the mentioned blog post by the Angular team, explaining their thoughts in detail.

@mikezks I’m aware that CD will be executed, and DOM event will mark LView (and ancestors) as dirty, but when CD “executes” the view and reads the value from a signal, that signal will return the same array (just mutated). Because arrays are equal by reference, CD should not detect a change here, as it does in the case of [input]. But with @for it still detects the change.

As a wild assumption, I think that @for here creates a shallow copy and returns it to the template, and that’s why CD can see the change. Again, I didn’t read the code and might be wrong.

@for (and NgFor) doesn’t care about the identity of the array. It diffs the array on every change detection cycle.

@mikezks I’m aware that CD will be executed, and DOM event will mark LView (and ancestors) as dirty, but when CD “executes” the view and reads the value from a signal, that signal will return the same array (just mutated). Because arrays are equal by reference, CD should not detect a change here, as it does in the case of [input]. But with @for it still detects the change.

As a wild assumption, I think that @for here creates a shallow copy and returns it to the template, and that’s why CD can see the change. Again, I didn’t read the code and might be wrong.

@txone

s.update((oldSet) => {
  const newSet = new Set(oldSet);
  newSet.add('some item');
  return newSet;
});

Similar code for Map and Date, but for many classes there is no such easy way to recreate them using an existing instance.

Thanks @e-oz I experimented with your playground and now even I understand the motivation around input don’t be updated from CD if mutable value is readed from signal (even if we have custom equal “signaling” it)…

But I have another question for @alxhub in a near future where we’ll have Signal Component and the @Input become an input() readonly, as Alex commented in the RFC that we may “think about binding to readonly input as computed” in that case an outer signal with custom equal, will notify the input prop inside the SignalComponent or not? 🤔

@dmorosinotto I meant this:

class SomeService {
  private readonly _canvas = signal<{ item: HTMLCanvasElement } | undefined>(undefined);

  public readonly canvas = computed(() => this._canvas()?.item);

  public setCanvas(canvas: HTMLCanvasElement) {
    this._canvas.set({item: canvas});
  }
}

@e-oz when you say

edit: nevermind, we can just wrap it.

Are you referring to something like a Proxy or wrapper = { value: mutable, lastChange: number} a wrapper object with a value prop pointing to your “native mutable” ref data and handle the “immutability” of the wrapper tracing the inside change of the mutable reflecting it -> to an easy new wrapper instance pointing to the same ref data (mutable changed) and increment the wrapper.lastChange+1 so we can use a custom equal Fn that simply compare lastChange? Obviusly it’s not so easy to track the changes, but you as the developer know your code and when you need to mutate the wrapper inside signal.update - WDYT about a this idea?

And I can provide here a very simplified example that also works on stackblitz

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

As the #53139 is closed I add the comment here as well: https://github.com/angular/angular/issues/53139#issuecomment-1825524593

The current behavior does not make sense.

Either one is not allowed to create specific equal functions (in this case for same object references with result false), then we need to throw an error and ignore the equal function.

Or the warning is logged to the console, but then the equal function needs to be used even if not recommended.

Showing a warning only, but ignoring the equal function is not an expectable behavior.

Help wanted would only be on the docs clarifications outlines above.