Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

ngFor + ngModel: How can I unshift values to the array I am iterating?

I have an array of elements which the user can not only edit, but also add and delete complete array elements. This works nicely, unless I attempt to add a value to the beginning of the array (e.g. using unshift).

Here is a test demonstrating my problem:

import { Component } from '@angular/core';
import { ComponentFixture, TestBed } from '@angular/core/testing';
import { FormsModule } from '@angular/forms';


@Component({
    template: `
        <form>
            <div *ngFor="let item of values; let index = index">
                <input [name]="'elem' + index" [(ngModel)]="item.value">
            </div>
        </form>`
})
class TestComponent {
    values: {value: string}[] = [{value: 'a'}, {value: 'b'}];
}

fdescribe('ngFor/Model', () => {
    let component: TestComponent;
    let fixture: ComponentFixture<TestComponent>;
    let element: HTMLDivElement;

    beforeEach(async () => {
        TestBed.configureTestingModule({
            imports: [FormsModule],
            declarations: [TestComponent]
        });

        fixture = TestBed.createComponent(TestComponent);
        component = fixture.componentInstance;
        element = fixture.nativeElement;

        fixture.detectChanges();
        await fixture.whenStable();
    });

    function getAllValues() {
        return Array.from(element.querySelectorAll('input')).map(elem => elem.value);
    }

    it('should display all values', async () => {
        // evaluation
        expect(getAllValues()).toEqual(['a', 'b']);
    });

    it('should display all values after push', async () => {
        // execution
        component.values.push({value: 'c'});
        fixture.detectChanges();
        await fixture.whenStable();

        // evaluation
        expect(getAllValues()).toEqual(['a', 'b', 'c']);
    });

    it('should display all values after unshift', async () => {
        // execution
        component.values.unshift({value: 'z'});
        fixture.detectChanges();
        await fixture.whenStable();

        // evaluation
        console.log(JSON.stringify(getAllValues())); // Logs '["z","z","b"]'
        expect(getAllValues()).toEqual(['z', 'a', 'b']);
    });
});

The first two tests pass just fine. However the third test fails. In the third test I attempt to prepend "z" to my inputs, which is successful, however the second input also shows "z", which it should not.

(Note that hundreds of similar questions exist on the web, but in the other cases people were just not having unique name-attributes and they are also just appending, not prepending).

Why is this happening and what can I do about it?

Notes on trackBy

So far the answers were just "use trackBy". However the documentation for trackBy states:

By default, the change detector assumes that the object instance identifies the node in the iterable

Since I don't supply an explicit trackBy-Function, that means that angular is supposed to track by identity, which (in the case above) absolutely correctly identifies each object and is inline with what the documentation expects.

The answer by Morphyish basically states that the feature to track by identity is broken and proposes to use an id-Property. At first it seemed to be a solution, but then it turned out to be just an error. Using an id-Property exhibits the exact same behavior as my test above.

The answer by penleychan tracks by index, which causes angular to think, that after I unshifted a value angular thinks that actually I pushed a value and all values in the array just happened to have updated. It kind of works around the issue, but it is in violation to the track-By contract and it defeats the purpose of the track-by-function (to reduce churn in the DOM).

like image 450
yankee Avatar asked Jul 23 '19 11:07

yankee


2 Answers

Edit

After further investigation, the issue wasn't actually coming from the ngFor. It was the ngModel using the name attribute of the input.

In the loop, the name attribute is generated using the array index. However, when placing a new element at the start of the array, we suddenly have a new element with the same name.

This is probably creating a conflict with multiple ngModel observing the same input internally.

This behavior can be furthered observed when adding multiple inputs at the start of the array. All inputs that were initially created with the same name attribute, will take the value of the new input being created. Regardless if their respective values were changed or not.

To fix this issue you simply need to give each input a unique name. Either by using a unique id, as from my example below

<input [name]="'elem' + item.id" [(ngModel)]="item.value">

Or by using a unique name/id generator (similar to what's Angular Material does).


Original answer

The issue, as stated by penleychan, is the missing trackBy on your ngFor directive.

You can find a working example of what you are looking for here

With the updated code from your example

import { Component } from '@angular/core';
import { ComponentFixture, TestBed } from '@angular/core/testing';
import { FormsModule } from '@angular/forms';


@Component({
    template: `
        <form>
            <div *ngFor="let item of values; let index = index; trackBy: trackByFn">
                <input [name]="'elem' + index" [(ngModel)]="item.value">
            </div>
        </form>`
})
class TestComponent {
    values: {id: number, value: string}[] = [{id: 0, value: 'a'}, {id: 1, value: 'b'}];
    trackByFn = (index, item) => item.id;
}

fdescribe('ngFor/Model', () => {
    let component: TestComponent;
    let fixture: ComponentFixture<TestComponent>;
    let element: HTMLDivElement;

    beforeEach(async () => {
        TestBed.configureTestingModule({
            imports: [FormsModule],
            declarations: [TestComponent]
        });

        fixture = TestBed.createComponent(TestComponent);
        component = fixture.componentInstance;
        element = fixture.nativeElement;

        fixture.detectChanges();
        await fixture.whenStable();
    });

    function getAllValues() {
        return Array.from(element.querySelectorAll('input')).map(elem => elem.value);
    }

    it('should display all values', async () => {
        // evaluation
        expect(getAllValues()).toEqual(['a', 'b']);
    });

    it('should display all values after push', async () => {
        // execution
        component.values.push({id: 2, value: 'c'});
        fixture.detectChanges();
        await fixture.whenStable();

        // evaluation
        expect(getAllValues()).toEqual(['a', 'b', 'c']);
    });

    it('should display all values after unshift', async () => {
        // execution
        component.values.unshift({id: 2, value: 'z'});
        fixture.detectChanges();
        await fixture.whenStable();

        // evaluation
        console.log(JSON.stringify(getAllValues())); // Logs '["z","z","b"]'
        expect(getAllValues()).toEqual(['z', 'a', 'b']);
    });
});

Despite your comment, it not a workaround. trackBy was made for the type of use (as well as performances but both are linked).

You can find ngForOf directive code here if you want to take a look for yourself, but here is how it works.

The ngForOf directive is diffing the array to determine modifications made, however without a specific trackBy function passed it is left with making soft comparisons. Which is fine for simple data structure such as strings, or numbers. But when you are using Objects, it can get wacky really fast.

On top of lowering performances, the lack of clear identification for the items inside the array can force the array to re-render the entirety of the elements.

However, if the ngForOf directive is able to clearly determine which item has changed, which item was deleted and which one was added. It can leave every other items untouched, add or remove templates from the DOM as required, and update only the ones that needs to be.

If you add the trackBy function and add an item at the start of the array, the diffing can realize that this is exactly what happened, and prepend a new template a the beginning of the loop while binding the corresponding item to it.

like image 136
Morphyish Avatar answered Oct 10 '22 18:10

Morphyish


Figured out the issue. Use trackBy

The issue is if the value changes, the differ reports a change. So if the default function returns object references, it will not match the current item if the object reference has changed.

Explaination on trackBy https://stackoverflow.com/a/45391247/6736888

https://stackblitz.com/edit/angular-testing-gvpdhr

like image 45
penleychan Avatar answered Oct 10 '22 16:10

penleychan