platform icon indicating copy to clipboard operation
platform copied to clipboard

RFC: `deepWritableSignal`

Open rainerhahnekamp opened this issue 1 year ago • 19 comments

Update 10.12.2024: We’ve decided to pause this RFC for now, as the primary use case seems limited to forms. With the anticipated introduction of SignalForm in the future, we aim to avoid introducing a feature that might need deprecation in just 2–3 major releases.

However, if you can provide compelling examples of use cases beyond forms, we would be open to revisiting this proposal.

Which @ngrx/* package(s) are relevant/related to the feature request?

signals

Information

Problem

As Signal adoption grows, we anticipate that APIs will increasingly require Signals. Type Signal will not cause any issues for the SignalStore, but WritableSignal will.

A good example we have already today, are Template-driven forms.

@Component({
    selector: "app-user",
    template: `<h1>User Detail</h1>
    <form (ngSubmit)="handleSubmit()">
      <input [(ngModel)]="firstname" />
      <input [(ngModel)]="city" />
    </form>`,
    standalone: true,
})
export class UserDetailComponent {
    #userStore = inject(UserStore);

    firstname = signal(this.#userStore.user.firstname());
    city = signal(this.#userStore.user.address.city());

    formUser = computed(() => ({firstname: this.firstname(), city: this.city()}))

    handleSubmit() {
        this.#userStore.save(this.formUser());
    }
}

We have to manually create a WritableSignal for each field, which doesn’t scale well.

Proposed Solution

deepWritableSignal makes all nested properties in an object literal within a WritableSignal into WritableSignals themselves. This approach matches the behavior of deepComputed, which can also be used as standalone.


deepWritableSignal can also be applied to an unprotected SignalStore and to the state returned by signalState. That would be the types WritableSignalSource and SignalState.

Example

Default Use Case

With the combination of linkedSignal, we provide a significant DX improvement without compromising the state protection:

@Component({
    selector: "app-user",
    template: `<h1>User Detail</h1>
    <form (ngSubmit)="handleSubmit()">
      <input [(ngModel)]="user.firstname" />
      <input [(ngModel)]="user.address.city" />
    </form>`,
    standalone: true,
})
export class UserDetailComponent {
    #userStore = inject(UserStore);

    user = deepWritableSignal(linkedSignal(this.#userStore));

    handleSubmit() {
        this.#userStore.save(this.user());
    }
}

Unprotected SignalStore

For an unprotected Signal Store


const UserStore = signalStore(
  {protectedState: false}, 
  withState({user: {
    id: 1,
    name: 'Konrad'
  }})
);

const userStore = new UserStore();

const user = deepWritableSignal(userStore.user);
user.name.set('Max');

For signalState

const userState = signalState({
  user: {
    id: 1,
    name: 'Konrad'
  }
});

const user = deepWritableSignal(userState.user);
user.name.set('Max');

Potential Extensions

Due to the protected state, every Signal from the SignalStore needs to be mapped to the type WritableSignal.

To address this, we could extend deepWritableSignal with an additional parameter that internally applies linkedSignal. Alternatively, we could implement an implicit linkedSignal if deepWritableSignal doesn’t receive a WritableSignal.

Version 1: Explicit linkedSignal

declare function deepWritableSignal<T>(signal: WritableSignal<T>, wrapWithLinkedSignal = false): DeepWritableSignal<T>;
declare function deepWritableSignal<T>(signal: Signal<T>, wrapWithLinkedSignal: true): DeepWritableSignal<T>;

export class UserDetailComponent {
    #userStore = inject(UserStore);

    user = deepWritableSignal(this.#userStore, true);

    handleSubmit() {
        this.#userStore.save(this.formUser());
    }
}

Version 2: Implicit linkedSignal

declare function deepWritableSignal<T>(signal: WritableSignal<T>, wrapWithLinkedSignal = false): DeepWritableSignal<T>;
declare function deepWritableSignal<T>(signal: Signal<T>): DeepWritableSignal<T>;

export class UserDetailComponent {
    #userStore = inject(UserStore);

    user = deepWritableSignal(this.#userStore);

    handleSubmit() {
        this.#userStore.save(this.formUser());
    }
}

Describe any alternatives/workarounds you're currently using

Alternatively, we could wait for Angular to introduce a DeepWritable. Until then, developers have to do more manual work.

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

  • [X] Yes
  • [ ] No

rainerhahnekamp avatar Nov 10 '24 11:11 rainerhahnekamp

Hello! Great idea! Would be awesome to get in ngxtension too ;) It will really help with forms. Thanks a lot!

1. Just a clarification, in case I miss or don't understand some parts.

In this example ("Default Use Case") I made some edits (marked with ⬅️), please let me know where I'm wrong and why:

@Component({
    selector: "app-user",
    template: `<h1>User Detail</h1>
    <form (ngSubmit)="handleSubmit()">
      <input [(ngModel)]="user().firstname" /> ⬅️
      <input [(ngModel)]="user().address().city" /> ⬅️
    </form>`,
    standalone: true,
})
export class UserDetailComponent {
    #userStore = inject(UserStore);

    user = deepWritableSignal(linkedSignal(this.#userStore.user)); ⬅️

    handleSubmit() {
        this.#userStore.save(this.user()); ⬅️
    }
}
Edits in the template:

I think that we should update the binding when user changes, not only user.firstname. It's not a nitpicking or an attempt to "catch", I'm curious if there are hidden things that I miss or don't know that would make that user() non-needed.

Edits in the component:

Here I'm almost certain I missed some details, but maybe not :)

2. Linked signals in a protected store:

I think it should be implicit, because with an explicit approach, when that flag is set to false, things just will not work.

e-oz avatar Nov 12 '24 06:11 e-oz

A few questions came to my mind when I saw this proposal:

  • How to handle cases when set or update is the property name?

In the following example, form.user.set/update nested signals will conflict with set/update signal methods:

const form = deepWritable({
  user: { set: 0, update: 0 },
});
  • Do we need another API or we can upgrade signalState:
const form = signalState(
  { firstName: 'John', lastName: 'Lennon' },
  { writable: true },
);

form.lastName.set('Mayer');

markostanimirovic avatar Nov 12 '24 11:11 markostanimirovic

@markostanimirovic, we can't read the value of a signal without calling a function, and getters here are not a solution, because then we will not be able to bind a signal to [(ngModel)].

So it will be

form().user().set.set(1)

or

form().user().set.update(v => !v)

Then, to read this value:

{{ form().user().set() }}

To bind:

[(ngModel)]="form().user().set"

e-oz avatar Nov 12 '24 11:11 e-oz

Hi guyes, from my point of view an absolutely necessary thing we need! Since the beginning of signalStore I always wondered what is the easiest way of writing data back to signalStore. Having to create a method to update a single state property, even if I have no business logic connected with this update is too much effort, especially in a reactive programming style. Thus, already one year ago I came up with an own solution I have implemented in multiple projects already. The need came mainly from the work with template driven froms like @rainerhahnekamp mentioned.

Instead of just having a writeable or updateable signal I would like to extend the discussion to a "DeepPatchableSignal". Instead of just writing back single properties, we could also update full or parts of an object.

Using the example we have so far in this thread, according to my proposal we should have the following options to work with the state:

const user = toDeepPatchableSignal(store.user);
// patch parts of an object
user.patch({name: 'Max'});
// patch/update the full object
user.patch({id: 1, name: 'Max'});
// navigate down to deeper levels and be able to do the same
user.name.patch('John');

The goal is, that the patch function always take an Partial<> of the type of the object in the current hierarchy. Each property which is not part in the patch call remains unchanged. "Leaf signals" can of course also have the proposed set and update functions to bind those properties directly to ngModel.

From my point of view linkedSignal is not necessary to realize something like this. A clever concatenation of lambdas with the standard patchState function should also do the job if we find a way that toDeepPatchableSignal can somehow get access to the internal store object.

My current implementation can be found here: https://github.com/angular-architects/ngrx-hateoas/blob/main/libs/ngrx-hateoas/src/lib/util/deep-patchable-signal.ts

Currently I am calling my toDeepPatchableSignal inside a method inside the store and returning the result with the help of a method. This looks currently like this and should be improved so that there is no need to create a method:

withMethods((store: any) => {
  const patchableUser = toDeepPatchableSignal<User>(newVal => patchState(store, { user: newVal }), store.user);
  return {
      getUserAsPatchable: (): DeepPatchableSignal<User> => {
          return patchableUser ;
      }
})

To be able to reuse this logic as easy as possible is put all this into a generic signal store feature you can find here: https://github.com/angular-architects/ngrx-hateoas/blob/main/libs/ngrx-hateoas/src/lib/store-features/with-patchable-resource.ts

Would be really happy to see support of something similar build directly into signalStore. I would also offer to support during the implementation or the transfer of the ideas of my current solution.

danielmurrmann avatar Nov 12 '24 14:11 danielmurrmann

👋 @e-oz, thanks for your comments. Here are my answers.

<input [(ngModel)]="user().firstname" />

If we call the value of the user signal, we just get the user. With just user.firstname we could activate that DeepWritableSignal.

handleSubmit() { this.#userStore.save(this.user()); ⬅️ }

Thanks for pointing that out. I've updated the RFC, this was typo of mine

I think that we should update the binding when user changes, not only user.firstname.

Yes, when user's changes the firstname will be updated automatically.

I think it should be implicit,

I am also lending towards it.

rainerhahnekamp avatar Nov 12 '24 16:11 rainerhahnekamp

@markostanimirovic

How to handle cases when set or update is the property name?

If a nested object has a property like set or update, you would get a compilation error.

Do we need another API or we can upgrade signalState:

I don't see the need but I saw that my RFC used the signalStore instead the signalState. Fixed it now.

rainerhahnekamp avatar Nov 12 '24 17:11 rainerhahnekamp

@fancyDevelopment

like to extend the discussion to a "DeepPatchableSignal".

I like the idea, but I think adding a patch method doesn't follow the current style of the SignalStore, which works with standalone functions (tree shaking).

What do you think about integrating that into patchState?

const UserStore = signalStore(
  {protectedState: false}, 
  withState({user: {
    id: 1,
    name: 'Konrad'
  }})
);

and then within the store:

patchState(store => store.user, {id: 2})

That would be another PR though.

rainerhahnekamp avatar Nov 12 '24 17:11 rainerhahnekamp

@rainerhahnekamp this is a great 🚀

gabrielguerrero avatar Nov 12 '24 22:11 gabrielguerrero

Hi @rainerhahnekamp, firstly thank you, this is a really cool idea. Secondly, here are a few thoughts of mine:

  1. Is this really deep? Can I go arbitrarily deep in the tree, or is the magic only applied one level deep?

  2. What about an alternative solution to the problem of set and update: keeping them in separate objects? The temporary solution I built myself looks more like this:

    readonly user = model<User>()
    protected readonly userLens = lens(this.user);
    

    userLens is a proxy object, and every property is a WritableSignal containing the value from user, in a two-way binding.

    .set() and .update() can still be called on user (and the appropriate signals will still be updated in userLens), but there’s no chance of a mix between the two. Also, I’m not sure if it was covered in your example, but using a proxy also allows signals to be created for elements that don’t yet exist in the base model, rather than all being initialised at start time. More specifically, a signal is only created when it is accessed.

    The catch with this solution is that two different properties are needed in the class, but the separation is clearer, and the lens will likely only be used in the template anyway. I implemented lens with Angular 17 effects performing two-way binding (though I’m looking forward to Angular 19 making this effects run sooner), I’m curious to see your implementation.

c-harding avatar Nov 13 '24 07:11 c-harding

Perhaps some initial implementation would show us what is possible and what is not 👀

e-oz avatar Nov 13 '24 08:11 e-oz

Perhaps some initial implementation would show us what is possible and what is not 👀

@e-oz I agree. I haven't seen a complete objection so far. We can process with a prototype and gather feedback on that.

rainerhahnekamp avatar Nov 13 '24 09:11 rainerhahnekamp

@rainerhahnekamp

and then within the store:

patchState(store => store.user, {id: 2})

I think this is not of help to the ones who need this in template dirven forms approach. Here you want to mutate the state easily from the outside. Also in other cases i see this need again and again. So I believe the 'deep patchable signal' should be availalbe from outside the store. Otherwise we need to write wrapper methods again and again.

danielmurrmann avatar Dec 10 '24 14:12 danielmurrmann

@fancyDevelopment

I think this is not of help to the ones who need this in template driven forms approach.

No, it really isn't. It was meant to be a good feature for patchState in general.

That being said, we pause this RFC because we see the use case only for forms at the moment. Given that we will get a SignalForm in the future, we don't want to introduce a feature that has to be deprecated in 2 or 3 majors.

If you can come up with examples other than forms, we could start re-considering it.

rainerhahnekamp avatar Dec 10 '24 17:12 rainerhahnekamp

First of all: Thank you very much for the great work. This solution already simplifies state management in many ways and we are currently considering whether to use the store.

However, we cannot refute the following argument: A lot of boilerplate code is required to update the SignalStore, which certainly has valid reasons. However, in our view, updating a property as a WritableSignal via direct setters and getters is much more intuitive and would greatly streamline both forms and side effects that make changes to the state.

Since changes do not only occur within the application via template, but can also come from outside, e.g. through real-time updates via websockets or polling, this use case should also be considered.

We are therefore currently still looking for a practical solution that comes close to Svelte 5 Runes (see "So what could the solution look like?").

Why is the current SignalStore solution associated with a lot of boilerplate?

Currently, an API wrapper function must be used in both the store and the component to patch an input value. In addition, bidirectional binding cannot be used. This applies to every property and can get out of hand relatively quickly if large, complex objects are to be mutated. Even such a small change requires considerable effort:

template:

[ngModel]="user.password()"
(ngModelChange)="updatePassword($event)"

Note: Angular does not support spread operators in the template, which is why the store function cannot be used directly in the template. (see https://github.com/angular/angular/issues/11850)

class:

updatePassword(pw: string): void {
    this.store.setPassword({
        ...this.user(),
        password: pw,
    });
}

store:

setUser(user: User): void {
    patchState(store, { user });
}

Note: Could the getters/setters be generated automatically internally via the patchState function, e.g. via a flag, in order to avoid boilerplate?

instead of just

[(ngModel)]="user.password"

or even

very.deep.object.tree.that.needs.to.be.mutated.set(true)

More complex effects or computations that make changes to the state and react to events from inside and outside the application also benefit from this.

So what could the solution look like?

With runes and particularly the $state rune, Svelte 5 offers deep bidirectional reactivity with dynamic objects out of the box, which seems extremely powerful and could serve as a model for the SignalStore, see example.

A configurable WritableSignalStore with direct mutating via getter/setter would be a wonderful solution, or a WritableDeepSignal function that also supports lazy loading would be extremely practical. Otherwise, the proxy solution presented above is also a good idea, as it is located between the store and the logic or component and could therefore probably be a good addition.

In our view, waiting for Angular's SignalForms only makes limited sense, since mutating in effects, computations or other side effects has exactly the same problem and there is probably no solution in sight even after the introduction of AngularSignals. In our view, a writable solution already makes sense in many respects - a later integration with SignalForms will probably entail a similar amount of effort either way, right?

What is the main argument against DeepWritableSignals, even though those offer so many advantages?

sreimchen avatar Mar 06 '25 12:03 sreimchen

What is the main argument against DeepWritableSignals, even though those offer so many advantages?

The main argument is that the Angular team is looking into solutions that fix that issue as well. Maybe we'll get a native solution which goes beyond forms.

Since the SignalStore should be aligned with the Angular Signal, we don't want to provide duplicated features.

rainerhahnekamp avatar Mar 06 '25 14:03 rainerhahnekamp

What is the main argument against DeepWritableSignals, even though those offer so many advantages?

The main argument is that the Angular team is looking into solutions that fix that issue as well. Maybe we'll get a native solution which goes beyond forms.

Since the SignalStore should be aligned with the Angular Signal, we don't want to provide duplicated features.

Thanks for the answer, are there any sources for this? I couldn't find anything specific on this.

sreimchen avatar Mar 06 '25 15:03 sreimchen

Thanks for the answer, are there any sources for this? I couldn't find anything specific on this.

There aren’t any official sources on this, but I can assure you that a lot of work is going into improving Signals and the SignalStore.

rainerhahnekamp avatar Mar 06 '25 15:03 rainerhahnekamp

All I could find on the subject was more or less this statement from the Angular team, which unfortunately clearly contradicts your statement, although I believe you: Sub-RFC 2: Signal APIs #49683

It is a great pity that there are no clear statements on such an important topic. Until then, I can refer to a library that I came across that tries to address this problem: https://github.com/AKABANAKK/deep-model

Well, then we'll just have to wait and see.

sreimchen avatar Mar 06 '25 16:03 sreimchen

@sreimchen ping me on social media please

rainerhahnekamp avatar Mar 06 '25 16:03 rainerhahnekamp