peopleware / angular-sdk

3 stars 1 forks source link

mixinDetectFormChanges forces the usage of allowSignalWrites flag in effects #8

Closed glenstaes closed 8 months ago

glenstaes commented 8 months ago

With the new input signals, it is now possible to have a reactive signal-way of resetting a FormGroup when the input binding of the component changes:

@Component({ ...omitted... })
export class MyComponent extends mixinDetectFormChanges() {
    public person: InputSignal<Person> = input.required()
    public form!: FormGroup<PersonForm>

    public ngOnInit(): void {
        this.form = this.generateForm(this.person())
    }

    public constructor() {
        super()
        effect(() => {
            this.resetForm(this.person())
        })
    }

    public generateForm(person: Person): FormGroup<PersonForm> {
        const form = new FormGroup<PersonForm>({
            name: createNullableControl(person.name ?? null, [])
        })
        this.detectFormChanges(this.form)
        return form
    }

    public resetForm(person: Person): void {
        this.form.reset(person)
        this.detectFormChanges(this.form)
    }
}

This is a scenario that makes sense. The input binding changes and the value of the form should reset. The end-result should be that the hasFormChangesSig changes its value from true to false.

This approach however raises an exception: image

It looks like the Angular team didn't intend us to follow this approach. It is however possible to enable the allowSignalWrites flag, but we're hesitant to do so since this isn't the default behavior.

glenstaes commented 8 months ago

From the docs: Using effects to synchronize data by writing to signals can lead to confusing and potentially incorrect behavior, and should be enabled only when necessary.

@MarkCuypersPpw Looks like we should go with the allowSignalWrites flag anyway. This seems to be an opt-in feature, preventing us from doing crazy things we didn't intend to do. Since the above scenario is an expected signal write, I don't see anything wrong with enabling the flag.