fix(ivy): constant object literals shared across element and component instances (#33705)
Currently if a consumer does something like the following, the object literal will be shared across the two elements and any instances of the component template. The same applies to array literals: ``` <div [someDirective]="{}"></div> <div [someDirective]="{}"></div> ``` These changes make it so that we generate a pure function even if an object is constant so that each instance gets its own object. Note that the original design for this fix included moving the pure function factories into the `consts` array. In the process of doing so I realized that pure function are also used inside of directive host bindings which means that we don't have access to the `consts`. These changes also: * Fix an issue that meant that the `pureFunction0` instruction could only be run during creation mode. * Make the `getConstant` utility slightly more convenient to use. This isn't strictly required for these changes to work, but I had made it as a part of a larger refactor that I ended up reverting. PR Close #33705
This commit is contained in:
@ -73,8 +73,8 @@ export function ɵɵtemplate(
|
||||
|
||||
// TODO: consider a separate node type for templates
|
||||
const tContainerNode = containerInternal(
|
||||
lView, index, tagName || null, getConstant(tViewConsts, attrsIndex) as TAttributes);
|
||||
const localRefs = getConstant(tViewConsts, localRefsIndex) as string[];
|
||||
lView, index, tagName || null, getConstant<TAttributes>(tViewConsts, attrsIndex));
|
||||
const localRefs = getConstant<string[]>(tViewConsts, localRefsIndex);
|
||||
if (tView.firstCreatePass) {
|
||||
ngDevMode && ngDevMode.firstCreatePass++;
|
||||
resolveDirectives(tView, lView, tContainerNode, localRefs);
|
||||
|
@ -46,8 +46,8 @@ export function ɵɵelementStart(
|
||||
const lView = getLView();
|
||||
const tView = lView[TVIEW];
|
||||
const tViewConsts = tView.consts;
|
||||
const attrs = getConstant(tViewConsts, attrsIndex) as TAttributes;
|
||||
const localRefs = getConstant(tViewConsts, localRefsIndex) as string[];
|
||||
const attrs = getConstant<TAttributes>(tViewConsts, attrsIndex);
|
||||
const localRefs = getConstant<string[]>(tViewConsts, localRefsIndex);
|
||||
ngDevMode && assertEqual(
|
||||
getBindingIndex(), tView.bindingStartIndex,
|
||||
'elements should be created before any bindings');
|
||||
|
@ -43,8 +43,8 @@ export function ɵɵelementContainerStart(
|
||||
const renderer = lView[RENDERER];
|
||||
const tagName = 'ng-container';
|
||||
const tViewConsts = tView.consts;
|
||||
const attrs = getConstant(tViewConsts, attrsIndex) as TAttributes;
|
||||
const localRefs = getConstant(tViewConsts, localRefsIndex) as string[];
|
||||
const attrs = getConstant<TAttributes>(tViewConsts, attrsIndex);
|
||||
const localRefs = getConstant<string[]>(tViewConsts, localRefsIndex);
|
||||
ngDevMode && assertEqual(
|
||||
getBindingIndex(), tView.bindingStartIndex,
|
||||
'element containers should be created before any bindings');
|
||||
|
@ -8,7 +8,7 @@
|
||||
|
||||
import {bindingUpdated, bindingUpdated2, bindingUpdated3, bindingUpdated4, getBinding, updateBinding} from './bindings';
|
||||
import {getBindingRoot, getLView} from './state';
|
||||
import {isCreationMode} from './util/view_utils';
|
||||
import {NO_CHANGE} from './tokens';
|
||||
|
||||
|
||||
/**
|
||||
@ -44,7 +44,7 @@ export function ɵɵpureFunction0<T>(slotOffset: number, pureFn: () => T, thisAr
|
||||
// TODO(kara): use bindingRoot instead of bindingStartIndex when implementing host bindings
|
||||
const bindingIndex = getBindingRoot() + slotOffset;
|
||||
const lView = getLView();
|
||||
return isCreationMode(lView) ?
|
||||
return lView[bindingIndex] === NO_CHANGE ?
|
||||
updateBinding(lView, bindingIndex, thisArg ? pureFn.call(thisArg) : pureFn()) :
|
||||
getBinding(lView, bindingIndex);
|
||||
}
|
||||
|
@ -176,8 +176,9 @@ export function viewAttachedToContainer(view: LView): boolean {
|
||||
}
|
||||
|
||||
/** Returns a constant from `TConstants` instance. */
|
||||
export function getConstant(consts: TConstants | null, index: number | null | undefined) {
|
||||
return consts === null || index == null ? null : consts[index];
|
||||
export function getConstant<T>(consts: TConstants | null, index: number | null | undefined): T|
|
||||
null {
|
||||
return consts === null || index == null ? null : consts[index] as unknown as T;
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -6,9 +6,10 @@
|
||||
* found in the LICENSE file at https://angular.io/license
|
||||
*/
|
||||
import {CommonModule} from '@angular/common';
|
||||
import {Component, Input} from '@angular/core';
|
||||
import {Component, Directive, Input, QueryList, ViewChild, ViewChildren} from '@angular/core';
|
||||
import {TestBed} from '@angular/core/testing';
|
||||
import {By} from '@angular/platform-browser';
|
||||
import {onlyInIvy} from '@angular/private/testing';
|
||||
|
||||
describe('components using pure function instructions internally', () => {
|
||||
describe('with array literals', () => {
|
||||
@ -479,4 +480,84 @@ describe('components using pure function instructions internally', () => {
|
||||
expect(objectComps[1].config).toEqual({opacity: 1, duration: 600});
|
||||
});
|
||||
});
|
||||
|
||||
onlyInIvy('issue has only been fixed for Ivy').describe('identical literals', () => {
|
||||
@Directive({selector: '[dir]'})
|
||||
class Dir {
|
||||
@Input('dir') value: any;
|
||||
}
|
||||
|
||||
it('should not share object literals across elements', () => {
|
||||
@Component({
|
||||
template: `
|
||||
<div [dir]="{}"></div>
|
||||
<div [dir]="{}"></div>
|
||||
`
|
||||
})
|
||||
class App {
|
||||
@ViewChildren(Dir) directives !: QueryList<Dir>;
|
||||
}
|
||||
|
||||
TestBed.configureTestingModule({declarations: [Dir, App]});
|
||||
const fixture = TestBed.createComponent(App);
|
||||
fixture.detectChanges();
|
||||
|
||||
const directives = fixture.componentInstance.directives.toArray();
|
||||
expect(directives[0].value).not.toBe(directives[1].value);
|
||||
});
|
||||
|
||||
it('should not share array literals across elements', () => {
|
||||
@Component({
|
||||
template: `
|
||||
<div [dir]="[]"></div>
|
||||
<div [dir]="[]"></div>
|
||||
`
|
||||
})
|
||||
class App {
|
||||
@ViewChildren(Dir) directives !: QueryList<Dir>;
|
||||
}
|
||||
|
||||
TestBed.configureTestingModule({declarations: [Dir, App]});
|
||||
const fixture = TestBed.createComponent(App);
|
||||
fixture.detectChanges();
|
||||
|
||||
const directives = fixture.componentInstance.directives.toArray();
|
||||
expect(directives[0].value).not.toBe(directives[1].value);
|
||||
});
|
||||
|
||||
it('should not share object literals across component instances', () => {
|
||||
@Component({template: `<div [dir]="{}"></div>`})
|
||||
class App {
|
||||
@ViewChild(Dir) directive !: Dir;
|
||||
}
|
||||
|
||||
TestBed.configureTestingModule({declarations: [Dir, App]});
|
||||
const firstFixture = TestBed.createComponent(App);
|
||||
firstFixture.detectChanges();
|
||||
|
||||
const secondFixture = TestBed.createComponent(App);
|
||||
secondFixture.detectChanges();
|
||||
|
||||
expect(firstFixture.componentInstance.directive.value)
|
||||
.not.toBe(secondFixture.componentInstance.directive.value);
|
||||
});
|
||||
|
||||
it('should not share array literals across component instances', () => {
|
||||
@Component({template: `<div [dir]="[]"></div>`})
|
||||
class App {
|
||||
@ViewChild(Dir) directive !: Dir;
|
||||
}
|
||||
|
||||
TestBed.configureTestingModule({declarations: [Dir, App]});
|
||||
const firstFixture = TestBed.createComponent(App);
|
||||
firstFixture.detectChanges();
|
||||
|
||||
const secondFixture = TestBed.createComponent(App);
|
||||
secondFixture.detectChanges();
|
||||
|
||||
expect(firstFixture.componentInstance.directive.value)
|
||||
.not.toBe(secondFixture.componentInstance.directive.value);
|
||||
});
|
||||
|
||||
});
|
||||
});
|
||||
|
Reference in New Issue
Block a user