diff --git a/packages/core/src/render3/context_discovery.ts b/packages/core/src/render3/context_discovery.ts index 812b3c21d1..6ce8791ba4 100644 --- a/packages/core/src/render3/context_discovery.ts +++ b/packages/core/src/render3/context_discovery.ts @@ -6,15 +6,13 @@ * found in the LICENSE file at https://angular.io/license */ import './ng_dev_mode'; - import {assertDomNode} from './assert'; import {EMPTY_ARRAY} from './definition'; import {LContext, MONKEY_PATCH_KEY_NAME} from './interfaces/context'; import {TNode, TNodeFlags} from './interfaces/node'; import {RElement} from './interfaces/renderer'; import {CONTEXT, HEADER_OFFSET, HOST, LView, TVIEW} from './interfaces/view'; -import {getComponentViewByIndex, getNativeByTNode, readElementValue, readPatchedData} from './util'; - +import {getComponentViewByIndex, getNativeByTNode, getTNode, readElementValue, readPatchedData} from './util'; /** Returns the matching `LContext` data for a given DOM node, directive or component instance. @@ -67,8 +65,8 @@ export function getLContext(target: any): LContext|null { } // the goal is not to fill the entire context full of data because the lookups - // are expensive. Instead, only the target data (the element, compontent or - // directive details) are filled into the context. If called multiple times + // are expensive. Instead, only the target data (the element, component, container, ICU + // expression or directive details) are filled into the context. If called multiple times // with different target values then the missing target data will be filled in. const native = readElementValue(lView[nodeIndex]); const existingCtx = readPatchedData(native); @@ -208,10 +206,15 @@ function traverseNextElement(tNode: TNode): TNode|null { return tNode.child; } else if (tNode.next) { return tNode.next; - } else if (tNode.parent) { - return tNode.parent.next || null; + } else { + // Let's take the following template:
text
+ // After checking the text node, we need to find the next parent that has a "next" TNode, + // in this case the parent `div`, so that we can find the component. + while (tNode.parent && !tNode.parent.next) { + tNode = tNode.parent; + } + return tNode.parent && tNode.parent.next; } - return null; } /** diff --git a/packages/core/src/render3/i18n.ts b/packages/core/src/render3/i18n.ts index c2e5941694..754586036b 100644 --- a/packages/core/src/render3/i18n.ts +++ b/packages/core/src/render3/i18n.ts @@ -9,8 +9,8 @@ import {SRCSET_ATTRS, URI_ATTRS, VALID_ATTRS, VALID_ELEMENTS, getTemplateContent} from '../sanitization/html_sanitizer'; import {InertBodyHelper} from '../sanitization/inert_body'; import {_sanitizeUrl, sanitizeSrcset} from '../sanitization/url_sanitizer'; - import {assertDefined, assertEqual, assertGreaterThan} from './assert'; +import {attachPatchData} from './context_discovery'; import {allocExpando, createNodeAtIndex, elementAttribute, load, textBinding} from './instructions'; import {LContainer, NATIVE, RENDER_PARENT} from './interfaces/container'; import {COMMENT_MARKER, ELEMENT_MARKER, I18nMutateOpCode, I18nMutateOpCodes, I18nUpdateOpCode, I18nUpdateOpCodes, IcuType, TI18n, TIcu} from './interfaces/i18n'; @@ -337,9 +337,7 @@ const parentIndexStack: number[] = []; export function i18nStart(index: number, message: string, subTemplateIndex?: number): void { const tView = getLView()[TVIEW]; ngDevMode && assertDefined(tView, `tView should be defined`); - ngDevMode && - assertEqual( - tView.firstTemplatePass, true, `You should only call i18nEnd on first template pass`); + i18nIndexStack[++i18nIndexStackPointer] = index; if (tView.firstTemplatePass && tView.data[index + HEADER_OFFSET] === null) { i18nStartFirstPass(tView, index, message, subTemplateIndex); } @@ -350,7 +348,6 @@ export function i18nStart(index: number, message: string, subTemplateIndex?: num */ function i18nStartFirstPass( tView: TView, index: number, message: string, subTemplateIndex?: number) { - i18nIndexStack[++i18nIndexStackPointer] = index; const viewData = getLView(); const expandoStartIndex = tView.blueprint.length - HEADER_OFFSET; const previousOrParentTNode = getPreviousOrParentTNode(); @@ -484,7 +481,6 @@ function appendI18nNode(tNode: TNode, parentTNode: TNode, previousTNode: TNode | // Nodes that inject ViewContainerRef also have a comment node that should be moved appendChild(slotValue[NATIVE], tNode, viewData); } - return tNode; } @@ -566,12 +562,7 @@ export function i18nPostprocess( export function i18nEnd(): void { const tView = getLView()[TVIEW]; ngDevMode && assertDefined(tView, `tView should be defined`); - ngDevMode && - assertEqual( - tView.firstTemplatePass, true, `You should only call i18nEnd on first template pass`); - if (tView.firstTemplatePass) { - i18nEndFirstPass(tView); - } + i18nEndFirstPass(tView); } /** @@ -675,6 +666,7 @@ function readCreateOpCodes( previousTNode = currentTNode; currentTNode = createNodeAtIndex( expandoStartIndex++, TNodeType.IcuContainer, commentRNode, null, null); + attachPatchData(commentRNode, viewData); (currentTNode as TIcuContainerNode).activeCaseIndex = null; // We will add the case nodes later, during the update phase setIsParent(false); @@ -1510,4 +1502,4 @@ function parseNodes( nestedIcuNodeIndex << I18nMutateOpCode.SHIFT_REF | I18nMutateOpCode.Remove); } } -} \ No newline at end of file +} diff --git a/packages/core/src/render3/instructions.ts b/packages/core/src/render3/instructions.ts index 2c35e2226e..30522ebc1f 100644 --- a/packages/core/src/render3/instructions.ts +++ b/packages/core/src/render3/instructions.ts @@ -6,7 +6,6 @@ * found in the LICENSE file at https://angular.io/license */ - import {resolveForwardRef} from '../di/forward_ref'; import {InjectionToken} from '../di/injection_token'; import {Injector} from '../di/injector'; @@ -44,7 +43,6 @@ import {NO_CHANGE} from './tokens'; import {getComponentViewByIndex, getNativeByIndex, getNativeByTNode, getRootContext, getRootView, getTNode, isComponent, isComponentDef, loadInternal, readElementValue, readPatchedLView, stringify} from './util'; - /** * A permanent marker promise which signifies that the current CD tree is * clean. @@ -212,24 +210,26 @@ export function createNodeAtIndex( let tNode = tView.data[adjustedIndex] as TNode; if (tNode == null) { - const previousOrParentTNode = getPreviousOrParentTNode(); - const isParent = getIsParent(); // TODO(misko): Refactor createTNode so that it does not depend on LView. tNode = tView.data[adjustedIndex] = createTNode(lView, type, adjustedIndex, name, attrs, null); + } - // Now link ourselves into the tree. - if (previousOrParentTNode) { - if (isParent && previousOrParentTNode.child == null && - (tNode.parent !== null || previousOrParentTNode.type === TNodeType.View)) { - // We are in the same view, which means we are adding content node to the parent view. - previousOrParentTNode.child = tNode; - } else if (!isParent) { - previousOrParentTNode.next = tNode; - } + // Now link ourselves into the tree. + // We need this even if tNode exists, otherwise we might end up pointing to unexisting tNodes when + // we use i18n (especially with ICU expressions that update the DOM during the update phase). + const previousOrParentTNode = getPreviousOrParentTNode(); + const isParent = getIsParent(); + if (previousOrParentTNode) { + if (isParent && previousOrParentTNode.child == null && + (tNode.parent !== null || previousOrParentTNode.type === TNodeType.View)) { + // We are in the same view, which means we are adding content node to the parent view. + previousOrParentTNode.child = tNode; + } else if (!isParent) { + previousOrParentTNode.next = tNode; } } - if (tView.firstChild == null && type === TNodeType.Element) { + if (tView.firstChild == null) { tView.firstChild = tNode; } @@ -504,6 +504,7 @@ export function elementContainerStart( appendChild(native, tNode, lView); createDirectivesAndLocals(tView, lView, localRefs); + attachPatchData(native, lView); } /** Mark the end of the . */ @@ -1921,6 +1922,8 @@ export function template( createDirectivesAndLocals(tView, lView, localRefs, localRefExtractor); const currentQueries = lView[QUERIES]; const previousOrParentTNode = getPreviousOrParentTNode(); + const native = getNativeByTNode(previousOrParentTNode, lView); + attachPatchData(native, lView); if (currentQueries) { lView[QUERIES] = currentQueries.addNode(previousOrParentTNode as TContainerNode); } diff --git a/packages/core/src/render3/interfaces/view.ts b/packages/core/src/render3/interfaces/view.ts index 9ef2e94d14..1b28ee54df 100644 --- a/packages/core/src/render3/interfaces/view.ts +++ b/packages/core/src/render3/interfaces/view.ts @@ -284,7 +284,7 @@ export interface TView { /** * Pointer to the `TNode` that represents the root of the view. * - * If this is a `TNode` for an `LViewNode`, this is an embedded view of a container. + * If this is a `TViewNode` for an `LViewNode`, this is an embedded view of a container. * We need this pointer to be able to efficiently find this node when inserting the view * into an anchor. * diff --git a/packages/core/test/linker/integration_spec.ts b/packages/core/test/linker/integration_spec.ts index 6d0e1a5e74..9ac8ba617d 100644 --- a/packages/core/test/linker/integration_spec.ts +++ b/packages/core/test/linker/integration_spec.ts @@ -399,29 +399,30 @@ function declareTests(config?: {useJit: boolean}) { expect(fixture.nativeElement).toHaveText('baz'); }); - fixmeIvy( - 'FW-665: Discovery util fails with "Unable to find the given context data for the given target"') - .it('should not detach views in ViewContainers when the parent view is destroyed.', () => { - TestBed.configureTestingModule({declarations: [MyComp, SomeViewport]}); - const template = - '
{{greeting}}
'; - TestBed.overrideComponent(MyComp, {set: {template}}); - const fixture = TestBed.createComponent(MyComp); + it('should not detach views in ViewContainers when the parent view is destroyed.', () => { + TestBed.configureTestingModule({declarations: [MyComp, SomeViewport]}); + const template = + '
{{greeting}}
'; + TestBed.overrideComponent(MyComp, {set: {template}}); + const fixture = TestBed.createComponent(MyComp); - fixture.componentInstance.ctxBoolProp = true; - fixture.detectChanges(); + fixture.componentInstance.ctxBoolProp = true; + fixture.detectChanges(); - const ngIfEl = fixture.debugElement.children[0]; - const someViewport: SomeViewport = ngIfEl.childNodes[0].injector.get(SomeViewport); - expect(someViewport.container.length).toBe(2); - expect(ngIfEl.children.length).toBe(2); + const ngIfEl = fixture.debugElement.children[0]; + const someViewport: SomeViewport = + ngIfEl.childNodes + .find(debugElement => debugElement.nativeNode.nodeType === Node.COMMENT_NODE) + .injector.get(SomeViewport); + expect(someViewport.container.length).toBe(2); + expect(ngIfEl.children.length).toBe(2); - fixture.componentInstance.ctxBoolProp = false; - fixture.detectChanges(); + fixture.componentInstance.ctxBoolProp = false; + fixture.detectChanges(); - expect(someViewport.container.length).toBe(2); - expect(fixture.debugElement.children.length).toBe(0); - }); + expect(someViewport.container.length).toBe(2); + expect(fixture.debugElement.children.length).toBe(0); + }); it('should use a comment while stamping out `` elements.', () => { const fixture = @@ -798,40 +799,37 @@ function declareTests(config?: {useJit: boolean}) { emitter.fireEvent('fired !'); })); - fixmeIvy( - 'FW-665: Discovery util fails with Unable to find the given context data for the given target') - .it('should support events via EventEmitter on template elements', async(() => { - const fixture = - TestBed - .configureTestingModule({ - declarations: [MyComp, DirectiveEmittingEvent, DirectiveListeningEvent] - }) - .overrideComponent(MyComp, { - set: { - template: - '' - } - }) - .createComponent(MyComp); + it('should support events via EventEmitter on template elements', async(() => { + const fixture = + TestBed + .configureTestingModule( + {declarations: [MyComp, DirectiveEmittingEvent, DirectiveListeningEvent]}) + .overrideComponent(MyComp, { + set: { + template: + '' + } + }) + .createComponent(MyComp); + const tc = fixture.debugElement.childNodes.find( + debugElement => debugElement.nativeNode.nodeType === Node.COMMENT_NODE); - const tc = fixture.debugElement.childNodes[0]; + const emitter = tc.injector.get(DirectiveEmittingEvent); + const myComp = fixture.debugElement.injector.get(MyComp); + const listener = tc.injector.get(DirectiveListeningEvent); - const emitter = tc.injector.get(DirectiveEmittingEvent); - const myComp = fixture.debugElement.injector.get(MyComp); - const listener = tc.injector.get(DirectiveListeningEvent); + myComp.ctxProp = ''; + expect(listener.msg).toEqual(''); - myComp.ctxProp = ''; - expect(listener.msg).toEqual(''); + emitter.event.subscribe({ + next: () => { + expect(listener.msg).toEqual('fired !'); + expect(myComp.ctxProp).toEqual('fired !'); + } + }); - emitter.event.subscribe({ - next: () => { - expect(listener.msg).toEqual('fired !'); - expect(myComp.ctxProp).toEqual('fired !'); - } - }); - - emitter.fireEvent('fired !'); - })); + emitter.fireEvent('fired !'); + })); it('should support [()] syntax', async(() => { TestBed.configureTestingModule({declarations: [MyComp, DirectiveWithTwoWayBinding]}); diff --git a/packages/core/test/render3/discovery_utils_spec.ts b/packages/core/test/render3/discovery_utils_spec.ts index 085ff607f1..d0e79dc790 100644 --- a/packages/core/test/render3/discovery_utils_spec.ts +++ b/packages/core/test/render3/discovery_utils_spec.ts @@ -8,17 +8,15 @@ import {createInjector} from '@angular/core'; import {StaticInjector} from '../../src/di/injector'; -import {getComponent, getContext, getDirectives, getInjectionTokens, getInjector, getListeners, getLocalRefs, getRootComponents, getViewComponent} from '../../src/render3/discovery_utils'; -import {ProvidersFeature, RenderFlags, defineComponent, defineDirective, getHostElement} from '../../src/render3/index'; - +import {getComponent, getContext, getDirectives, getInjectionTokens, getInjector, getListeners, getLocalRefs, getRootComponents, getViewComponent, loadLContext} from '../../src/render3/discovery_utils'; +import {ProvidersFeature, RenderFlags, defineComponent, defineDirective, elementContainerEnd, elementContainerStart, getHostElement, i18n, i18nApply, i18nExp} from '../../src/render3/index'; import {element, elementEnd, elementStart, elementStyling, elementStylingApply, template, bind, elementProperty, text, textBinding, markDirty, listener} from '../../src/render3/instructions'; - import {ComponentFixture} from './render_util'; import {NgIf} from './common_with_def'; describe('discovery utils', () => { let fixture: ComponentFixture; - let myApp: MyApp[]; + let myApp: MyApp; let dirA: DirectiveA[]; let childComponent: DirectiveA[]; let child: NodeListOf; @@ -29,7 +27,6 @@ describe('discovery utils', () => { beforeEach(() => { log = []; - myApp = []; dirA = []; childComponent = []; fixture = new ComponentFixture( @@ -63,6 +60,7 @@ describe('discovery utils', () => { *

* * + * ICU expression * * * ``` @@ -96,15 +94,19 @@ describe('discovery utils', () => { }); } + const MSG_DIV = `{�0�, select, + other {ICU expression} + }`; + class MyApp { text: string = 'INIT'; - constructor() { myApp.push(this); } + constructor() { myApp = this; } static ngComponentDef = defineComponent({ type: MyApp, selectors: [['my-app']], factory: () => new MyApp(), - consts: 9, + consts: 13, vars: 1, directives: [Child, DirectiveA, NgIf], template: (rf: RenderFlags, ctx: MyApp) => { @@ -121,10 +123,18 @@ describe('discovery utils', () => { element(0, 'child'); } }, 1, 0, 'child', ['ngIf', '']); + elementStart(9, 'i18n'); + i18n(10, MSG_DIV); + elementEnd(); + elementContainerStart(11); + { text(12, 'content'); } + elementContainerEnd(); } if (rf & RenderFlags.Update) { textBinding(1, bind(ctx.text)); elementProperty(8, 'ngIf', bind(true)); + i18nExp(bind(ctx.text)); + i18nApply(10); } } }); @@ -141,7 +151,7 @@ describe('discovery utils', () => { expect(() => getComponent(dirA[1] as any)).toThrowError(/Expecting instance of DOM Node/); }); it('should return component from element', () => { - expect(getComponent(fixture.hostElement)).toEqual(myApp[0]); + expect(getComponent(fixture.hostElement)).toEqual(myApp); expect(getComponent(child[0])).toEqual(childComponent[0]); expect(getComponent(child[1])).toEqual(childComponent[1]); }); @@ -153,7 +163,7 @@ describe('discovery utils', () => { expect(() => getContext(dirA[1] as any)).toThrowError(/Expecting instance of DOM Node/); }); it('should return context from element', () => { - expect(getContext(child[0])).toEqual(myApp[0]); + expect(getContext(child[0])).toEqual(myApp); expect(getContext<{$implicit: boolean}>(child[2]) !.$implicit).toEqual(true); expect(getContext(p[0])).toEqual(childComponent[0]); }); @@ -161,7 +171,7 @@ describe('discovery utils', () => { describe('getHostElement', () => { it('should return element on component', () => { - expect(getHostElement(myApp[0])).toEqual(fixture.hostElement); + expect(getHostElement(myApp)).toEqual(fixture.hostElement); expect(getHostElement(childComponent[0])).toEqual(child[0]); expect(getHostElement(childComponent[1])).toEqual(child[1]); }); @@ -181,7 +191,7 @@ describe('discovery utils', () => { expect(getInjector(p[0]).get(String)).toEqual('Child'); }); it('should return node-injector from component with providers', () => { - expect(getInjector(myApp[0]).get(String)).toEqual('Module'); + expect(getInjector(myApp).get(String)).toEqual('Module'); expect(getInjector(childComponent[0]).get(String)).toEqual('Child'); expect(getInjector(childComponent[1]).get(String)).toEqual('Child'); }); @@ -206,34 +216,34 @@ describe('discovery utils', () => { describe('getViewComponent', () => { it('should return null when called on root component', () => { expect(getViewComponent(fixture.hostElement)).toEqual(null); - expect(getViewComponent(myApp[0])).toEqual(null); + expect(getViewComponent(myApp)).toEqual(null); }); it('should return containing component of child component', () => { - expect(getViewComponent(child[0])).toEqual(myApp[0]); - expect(getViewComponent(child[1])).toEqual(myApp[0]); - expect(getViewComponent(child[2])).toEqual(myApp[0]); + expect(getViewComponent(child[0])).toEqual(myApp); + expect(getViewComponent(child[1])).toEqual(myApp); + expect(getViewComponent(child[2])).toEqual(myApp); - expect(getViewComponent(childComponent[0])).toEqual(myApp[0]); - expect(getViewComponent(childComponent[1])).toEqual(myApp[0]); - expect(getViewComponent(childComponent[2])).toEqual(myApp[0]); + expect(getViewComponent(childComponent[0])).toEqual(myApp); + expect(getViewComponent(childComponent[1])).toEqual(myApp); + expect(getViewComponent(childComponent[2])).toEqual(myApp); }); it('should return containing component of any view element', () => { - expect(getViewComponent(span[0])).toEqual(myApp[0]); - expect(getViewComponent(div[0])).toEqual(myApp[0]); + expect(getViewComponent(span[0])).toEqual(myApp); + expect(getViewComponent(div[0])).toEqual(myApp); expect(getViewComponent(p[0])).toEqual(childComponent[0]); expect(getViewComponent(p[1])).toEqual(childComponent[1]); expect(getViewComponent(p[2])).toEqual(childComponent[2]); }); it('should return containing component of child directive', () => { - expect(getViewComponent(dirA[0])).toEqual(myApp[0]); - expect(getViewComponent(dirA[1])).toEqual(myApp[0]); + expect(getViewComponent(dirA[0])).toEqual(myApp); + expect(getViewComponent(dirA[1])).toEqual(myApp); }); }); describe('getLocalRefs', () => { it('should retrieve empty map', () => { expect(getLocalRefs(fixture.hostElement)).toEqual({}); - expect(getLocalRefs(myApp[0])).toEqual({}); + expect(getLocalRefs(myApp)).toEqual({}); expect(getLocalRefs(span[0])).toEqual({}); expect(getLocalRefs(child[0])).toEqual({}); }); @@ -249,8 +259,8 @@ describe('discovery utils', () => { describe('getRootComponents', () => { it('should return root components from component', () => { - const rootComponents = [myApp[0]]; - expect(getRootComponents(myApp[0])).toEqual(rootComponents); + const rootComponents = [myApp]; + expect(getRootComponents(myApp)).toEqual(rootComponents); expect(getRootComponents(childComponent[0])).toEqual(rootComponents); expect(getRootComponents(childComponent[1])).toEqual(rootComponents); expect(getRootComponents(dirA[0])).toEqual(rootComponents); @@ -289,12 +299,46 @@ describe('discovery utils', () => { describe('markDirty', () => { it('should re-render component', () => { expect(span[0].textContent).toEqual('INIT'); - myApp[0].text = 'WORKS'; - markDirty(myApp[0]); + myApp.text = 'WORKS'; + markDirty(myApp); fixture.requestAnimationFrame.flush(); expect(span[0].textContent).toEqual('WORKS'); }); }); + + describe('loadLContext', () => { + it('should work on components', () => { + const lContext = loadLContext(child[0]); + expect(lContext).toBeDefined(); + expect(lContext.native as any).toBe(child[0]); + }); + + it('should work on templates', () => { + const templateComment = Array.from(fixture.hostElement.childNodes) + .find((node: ChildNode) => node.nodeType === Node.COMMENT_NODE); + const lContext = loadLContext(templateComment); + expect(lContext).toBeDefined(); + expect(lContext.native as any).toBe(templateComment); + }); + + it('should work on ICU expressions', () => { + const icuComment = Array.from(fixture.hostElement.querySelector('i18n') !.childNodes) + .find((node: ChildNode) => node.nodeType === Node.COMMENT_NODE); + const lContext = loadLContext(icuComment); + expect(lContext).toBeDefined(); + expect(lContext.native as any).toBe(icuComment); + }); + + it('should work on ng-container', () => { + const ngContainerComment = Array.from(fixture.hostElement.childNodes) + .find( + (node: ChildNode) => node.nodeType === Node.COMMENT_NODE && + node.textContent === `ng-container`); + const lContext = loadLContext(ngContainerComment); + expect(lContext).toBeDefined(); + expect(lContext.native as any).toBe(ngContainerComment); + }); + }); }); describe('discovery utils deprecated', () => {