Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reactive Form - Formgroup with a nested formgroup throw exception on reset call with null values #20509

Closed
szanellato93 opened this issue Nov 17, 2017 · 7 comments
Assignees
Labels
area: forms P2 The issue is important to a large percentage of users, with a workaround
Milestone

Comments

@szanellato93
Copy link

szanellato93 commented Nov 17, 2017


[ ] Regression (a behavior that used to work and stopped working in a new release)
[ ] Bug report  
[X] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question

Current behavior

When i call form reset on a formGroup with nested formGroup passing it with a null value i receive a error "Cannot read property ... of null"

Expected behavior

Expected result is that on form reset with a null value the nested form group properties become all null.

Minimal reproduction of the problem with instructions

When i have a formGroup with a nested formGroup like this:

let form = this.fb.group({
Id: [null],
Nested: fb.group({
    Id: [null]
})
})

If i call a form reset or a patch value with a object like this:

form.reset({
Id: 1,
Nested: null
})

i receive the error "Cannot read property 'Id' of null" because he try to read the nested property from a null object

Expected result is that on form reset with a null value the nested form group properties become all null. This behavior happen when i call the reset without the nested properties:
form.reset({Id: 3})

What is the motivation / use case for changing the behavior?

Usually service return models with all properties and nested entities null if they don't exist instead return the nested entities initialized

@szanellato93 szanellato93 changed the title Formgroup with nested formgroup throw exception on null values Formgroup with nested formgroup throw exception on reset with null values Nov 17, 2017
@szanellato93 szanellato93 changed the title Formgroup with nested formgroup throw exception on reset with null values Reactive Form - Formgroup with a nested formgroup throw exception on reset call with null values Nov 17, 2017
@kara kara added freq2: medium feature Issue that requests a new feature workaround1: obvious labels Dec 5, 2017
@ngbot ngbot bot added this to the Backlog milestone Jan 23, 2018
@artem-galas
Copy link
Contributor

I would like to take this issue.
But I'm not sure how to implement it.

The error occurred on .reset method for FormGroup. https://github.com/angular/angular/blob/master/packages/forms/src/model.ts#L1478

We can check if (control instanceof FormGroup) into this method, but I'm not sure if am I right.

Here is my idea:
As I understand we have to iterate through all controls from nested formGroup and reset value to null.
So basically we can call control._forEachChild.

Could you please advise me a little here?

Thanks.

@timrasche
Copy link

Any update on this?

@w5l
Copy link

w5l commented Apr 14, 2021

This still happens, currently caused by the reset code in FormGroup here, specifically on line 1679:

reset(value: any = {}, options: {onlySelf?: boolean, emitEvent?: boolean} = {}): void {
this._forEachChild((control: AbstractControl, name: string) => {
control.reset(value[name], {onlySelf: true, emitEvent: options.emitEvent});
});
this._updatePristine(options);
this._updateTouched(options);
this.updateValueAndValidity(options);
}

It blindly assumes value is set to a non null value.

As a workaround, we're using the following reset code. Note that this modifies the input object which might be undesired, but in our use case it doesn't matter. YMMV.

export const resetForm = (group: FormGroup, value?: any, options?: {
    onlySelf?: boolean;
    emitEvent?: boolean;
}) => {
    const ensureValues = (g: FormGroup, v: any) => Object.keys(g.controls).forEach(key => {
        // The list of controls can change (for ex. controls might be removed) while the loop
        // is running (as a result of invoking Forms API in `valueChanges` subscription), so we
        // have to null check before invoking the callback.
        const control = g.controls[key];
        if (control instanceof FormGroup) {
            // Ensure the value to be bound to this formgroup is not null.
            v[key] = v[key] || {};
            // Recursively check child controls for nested formgroups.
            ensureValues(control, v[key]);
        }
    });
    ensureValues(group, value || {});
    group.reset(value, options);
};

@angular-robot angular-robot bot added the feature: votes required Feature request which is currently still in the voting phase label Jun 4, 2021
@angular-robot
Copy link
Contributor

angular-robot bot commented Jun 4, 2021

Just a heads up that we kicked off a community voting process for your feature request. There are 20 days until the voting process ends.

Find more details about Angular's feature request process in our documentation.

@angular-robot
Copy link
Contributor

angular-robot bot commented Jun 26, 2021

Thank you for submitting your feature request! Looks like during the polling process it didn't collect a sufficient number of votes to move to the next stage.

We want to keep Angular rich and ergonomic and at the same time be mindful about its scope and learning journey. If you think your request could live outside Angular's scope, we'd encourage you to collaborate with the community on publishing it as an open source package.

You can find more details about the feature request process in our documentation.

@angular-robot angular-robot bot added the feature: insufficient votes Label to add when the not a sufficient number of votes or comments from unique authors label Jun 26, 2021
@jessicajaniuk jessicajaniuk added P2 The issue is important to a large percentage of users, with a workaround and removed feature Issue that requests a new feature feature: insufficient votes Label to add when the not a sufficient number of votes or comments from unique authors feature: votes required Feature request which is currently still in the voting phase labels Jul 18, 2022
@dylhunn
Copy link
Contributor

dylhunn commented Jul 18, 2022

We'll need to see how breaking this fix is. I really hope it's not too breaking in g3 -- that would mean people are relying on app crashes 🤦

JeanMeche added a commit to JeanMeche/angular that referenced this issue Jan 24, 2023
Non typed forms allow to pass null to nested groups when calling `formGroup.reset()`, this commit prevent an undefined access.

fixes angular#20509
JeanMeche added a commit to JeanMeche/angular that referenced this issue Jan 24, 2023
Non typed forms allow to pass null to nested groups when calling `formGroup.reset()`, this commit prevent an undefined access.

fixes angular#20509
atscott pushed a commit that referenced this issue Oct 10, 2023
Non typed forms allow to pass null to nested groups when calling `formGroup.reset()`, this commit prevent an undefined access.

fixes #20509

PR Close #48830
@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Nov 11, 2023
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this issue Jan 23, 2024
)

Non typed forms allow to pass null to nested groups when calling `formGroup.reset()`, this commit prevent an undefined access.

fixes angular#20509

PR Close angular#48830
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: forms P2 The issue is important to a large percentage of users, with a workaround
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants