From 1b6d8a78b08f4a71631009c5ed329ecb2ef97e05 Mon Sep 17 00:00:00 2001 From: Pawel Kozlowski Date: Thu, 31 Jan 2019 16:02:19 +0100 Subject: [PATCH] fix(ivy): queries should match container node itself before matching its views (#28473) PR Close #28473 --- packages/core/src/render3/instructions.ts | 38 +++++++++++-------- packages/core/src/render3/interfaces/node.ts | 2 +- .../src/render3/view_engine_compatibility.ts | 12 +++--- .../bundling/todo/bundle.golden_symbols.json | 3 ++ .../test/linker/query_integration_spec.ts | 28 +++++++------- 5 files changed, 45 insertions(+), 38 deletions(-) diff --git a/packages/core/src/render3/instructions.ts b/packages/core/src/render3/instructions.ts index c7b5e7935c..da37de5ac0 100644 --- a/packages/core/src/render3/instructions.ts +++ b/packages/core/src/render3/instructions.ts @@ -31,11 +31,11 @@ import {CssSelectorList, NG_PROJECT_AS_ATTR_NAME} from './interfaces/projection' import {LQueries} from './interfaces/query'; import {GlobalTargetResolver, ProceduralRenderer3, RComment, RElement, RText, Renderer3, RendererFactory3, isProceduralRenderer} from './interfaces/renderer'; import {SanitizerFn} from './interfaces/sanitization'; -import {BINDING_INDEX, CLEANUP, CONTAINER_INDEX, CONTENT_QUERIES, CONTEXT, DECLARATION_VIEW, FLAGS, HEADER_OFFSET, HOST, HOST_NODE, INJECTOR, InitPhaseState, LView, LViewFlags, NEXT, OpaqueViewState, PARENT, QUERIES, RENDERER, RENDERER_FACTORY, RootContext, RootContextFlags, SANITIZER, TAIL, TData, TVIEW, TView} from './interfaces/view'; +import {BINDING_INDEX, CLEANUP, CONTAINER_INDEX, CONTEXT, DECLARATION_VIEW, FLAGS, HEADER_OFFSET, HOST, HOST_NODE, INJECTOR, InitPhaseState, LView, LViewFlags, NEXT, OpaqueViewState, PARENT, QUERIES, RENDERER, RENDERER_FACTORY, RootContext, RootContextFlags, SANITIZER, TAIL, TData, TVIEW, TView} from './interfaces/view'; import {assertNodeOfPossibleTypes, assertNodeType} from './node_assert'; import {appendChild, appendProjectedNode, createTextNode, getLViewChild, insertView, removeView} from './node_manipulation'; import {isNodeMatchingSelectorList, matchingSelectorIndex} from './node_selector_matcher'; -import {decreaseElementDepthCount, enterView, getBindingsEnabled, getCheckNoChangesMode, getContextLView, getCurrentDirectiveDef, getCurrentQueryIndex, getElementDepthCount, getIsParent, getLView, getPreviousOrParentTNode, increaseElementDepthCount, isCreationMode, leaveView, nextContextImpl, resetComponentState, setBindingRoot, setCheckNoChangesMode, setCurrentDirectiveDef, setCurrentQueryIndex, setIsParent, setPreviousOrParentTNode} from './state'; +import {decreaseElementDepthCount, enterView, getBindingsEnabled, getCheckNoChangesMode, getContextLView, getCurrentDirectiveDef, getElementDepthCount, getIsParent, getLView, getPreviousOrParentTNode, increaseElementDepthCount, isCreationMode, leaveView, nextContextImpl, resetComponentState, setBindingRoot, setCheckNoChangesMode, setCurrentDirectiveDef, setCurrentQueryIndex, setIsParent, setPreviousOrParentTNode} from './state'; import {getInitialClassNameValue, initializeStaticContext as initializeStaticStylingContext, patchContextWithStaticAttrs, renderInitialStylesAndClasses, renderStyling, updateClassProp as updateElementClassProp, updateContextWithBindings, updateStyleProp as updateElementStyleProp, updateStylingMap} from './styling/class_and_style_bindings'; import {BoundPlayerFactory} from './styling/player_factory'; import {createEmptyStylingContext, getStylingContext, hasClassInput, hasStyling, isAnimationProp} from './styling/util'; @@ -324,7 +324,7 @@ export function renderTemplate( * Such lViewNode will then be renderer with renderEmbeddedTemplate() (see below). */ export function createEmbeddedViewAndNode( - tView: TView, context: T, declarationView: LView, renderer: Renderer3, queries: LQueries | null, + tView: TView, context: T, declarationView: LView, queries: LQueries | null, injectorIndex: number): LView { const _isParent = getIsParent(); const _previousOrParentTNode = getPreviousOrParentTNode(); @@ -2100,12 +2100,7 @@ export function template( } createDirectivesAndLocals(tView, lView, localRefs, localRefExtractor); - - const currentQueries = lView[QUERIES]; - if (currentQueries) { - lView[QUERIES] = currentQueries.addNode(tContainerNode); - } - + addTContainerToQueries(lView, tContainerNode); attachPatchData(getNativeByTNode(tContainerNode, lView), lView); registerPostOrderHooks(tView, tContainerNode); setIsParent(false); @@ -2126,6 +2121,7 @@ export function container(index: number): void { if (lView[TVIEW].firstTemplatePass) { tNode.tViews = []; } + addTContainerToQueries(lView, tNode); setIsParent(false); } @@ -2148,16 +2144,28 @@ function containerInternal( // because views can be removed and re-inserted. addToViewTree(lView, index + HEADER_OFFSET, lContainer); - const currentQueries = lView[QUERIES]; - if (currentQueries) { - // prepare place for matching nodes from views inserted into a given container - lContainer[QUERIES] = currentQueries.container(); - } - ngDevMode && assertNodeType(getPreviousOrParentTNode(), TNodeType.Container); return tNode; } +/** + * Reporting a TContainer node queries is a 2-step process as we need to: + * - check if the container node itself is matching (query might match a node); + * - prepare room for nodes from views that might be created based on the TemplateRef linked to this + * container. + * + * Those 2 operations need to happen in the specific order (match the container node itself, then + * prepare space for nodes from views). + */ +function addTContainerToQueries(lView: LView, tContainerNode: TContainerNode): void { + const queries = lView[QUERIES]; + if (queries) { + lView[QUERIES] = queries.addNode(tContainerNode); + const lContainer = lView[tContainerNode.index]; + lContainer[QUERIES] = queries.container(); + } +} + /** * Sets a container up to receive views. * diff --git a/packages/core/src/render3/interfaces/node.ts b/packages/core/src/render3/interfaces/node.ts index 1c8775668f..e04e4dafd8 100644 --- a/packages/core/src/render3/interfaces/node.ts +++ b/packages/core/src/render3/interfaces/node.ts @@ -35,7 +35,7 @@ export const enum TNodeFlags { /** This bit is set if the node has been projected */ isProjected = 0b0010, - /** This bit is set if the node has any content queries */ + /** This bit is set if any directive on this node has content queries */ hasContentQuery = 0b0100, /** This bit is set if the node has any directives that contain [class properties */ diff --git a/packages/core/src/render3/view_engine_compatibility.ts b/packages/core/src/render3/view_engine_compatibility.ts index 5baa501305..c30e2a7bef 100644 --- a/packages/core/src/render3/view_engine_compatibility.ts +++ b/packages/core/src/render3/view_engine_compatibility.ts @@ -7,7 +7,7 @@ */ import {ChangeDetectorRef as ViewEngine_ChangeDetectorRef} from '../change_detection/change_detector_ref'; -import {Injector, NullInjector} from '../di/injector'; +import {Injector} from '../di/injector'; import {ComponentFactory as viewEngine_ComponentFactory, ComponentRef as viewEngine_ComponentRef} from '../linker/component_factory'; import {ElementRef as ViewEngine_ElementRef} from '../linker/element_ref'; import {NgModuleRef as viewEngine_NgModuleRef} from '../linker/ng_module_factory'; @@ -19,9 +19,7 @@ import {assertDefined, assertGreaterThan, assertLessThan} from '../util/assert'; import {NodeInjector, getParentInjectorLocation} from './di'; import {addToViewTree, createEmbeddedViewAndNode, createLContainer, renderEmbeddedTemplate} from './instructions'; import {ACTIVE_INDEX, LContainer, NATIVE, VIEWS} from './interfaces/container'; -import {RenderFlags} from './interfaces/definition'; import {TContainerNode, TElementContainerNode, TElementNode, TNode, TNodeType, TViewNode} from './interfaces/node'; -import {LQueries} from './interfaces/query'; import {RComment, RElement, Renderer3, isProceduralRenderer} from './interfaces/renderer'; import {CONTAINER_INDEX, CONTEXT, HOST_NODE, LView, QUERIES, RENDERER, TView} from './interfaces/view'; import {assertNodeOfPossibleTypes} from './node_assert'; @@ -64,7 +62,7 @@ export function createElementRef( let R3TemplateRef: { new ( _declarationParentView: LView, elementRef: ViewEngine_ElementRef, _tView: TView, - _renderer: Renderer3, _queries: LQueries | null, _injectorIndex: number): + _renderer: Renderer3, _hostLContainer: LContainer, _injectorIndex: number): ViewEngine_TemplateRef }; @@ -97,7 +95,7 @@ export function createTemplateRef( R3TemplateRef = class TemplateRef_ extends TemplateRefToken { constructor( private _declarationParentView: LView, readonly elementRef: ViewEngine_ElementRef, - private _tView: TView, private _renderer: Renderer3, private _queries: LQueries|null, + private _tView: TView, private _renderer: Renderer3, private _hostLContainer: LContainer, private _injectorIndex: number) { super(); } @@ -107,7 +105,7 @@ export function createTemplateRef( hostTNode?: TElementNode|TContainerNode|TElementContainerNode, hostView?: LView, index?: number): viewEngine_EmbeddedViewRef { const lView = createEmbeddedViewAndNode( - this._tView, context, this._declarationParentView, this._renderer, this._queries, + this._tView, context, this._declarationParentView, this._hostLContainer[QUERIES], this._injectorIndex); if (container) { insertView(lView, container, hostView !, index !, hostTNode !.index); @@ -125,7 +123,7 @@ export function createTemplateRef( ngDevMode && assertDefined(hostTNode.tViews, 'TView must be allocated'); return new R3TemplateRef( hostView, createElementRef(ElementRefToken, hostTNode, hostView), hostTNode.tViews as TView, - getLView()[RENDERER], hostContainer[QUERIES], hostTNode.injectorIndex); + getLView()[RENDERER], hostContainer, hostTNode.injectorIndex); } else { return null; } diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index 628c3189d7..04f07b1937 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -362,6 +362,9 @@ { "name": "addRemoveViewFromContainer" }, + { + "name": "addTContainerToQueries" + }, { "name": "addToViewTree" }, diff --git a/packages/core/test/linker/query_integration_spec.ts b/packages/core/test/linker/query_integration_spec.ts index 8ef710d2ec..fcaf05e908 100644 --- a/packages/core/test/linker/query_integration_spec.ts +++ b/packages/core/test/linker/query_integration_spec.ts @@ -94,21 +94,19 @@ describe('Query API', () => { ]); }); - fixmeIvy( - 'FW-982: For queries, ng-template own directives should be registered before the ones from inside the template') - .it('should contain the first content child when target is on with embedded view (issue #16568)', - () => { - const template = - '
' + - '
'; - const view = createTestCmp(MyComp0, template); - view.detectChanges(); - const q: NeedsContentChild = view.debugElement.children[1].references !['q']; - expect(q.child.text).toEqual('foo'); - const directive: DirectiveNeedsContentChild = - view.debugElement.children[0].injector.get(DirectiveNeedsContentChild); - expect(directive.child.text).toEqual('foo'); - }); + it('should contain the first content child when target is on with embedded view (issue #16568)', + () => { + const template = + '
' + + '
'; + const view = createTestCmp(MyComp0, template); + view.detectChanges(); + const q: NeedsContentChild = view.debugElement.children[1].references !['q']; + expect(q.child.text).toEqual('foo'); + const directive: DirectiveNeedsContentChild = + view.debugElement.children[0].injector.get(DirectiveNeedsContentChild); + expect(directive.child.text).toEqual('foo'); + }); it('should contain the first view child', () => { const template = '';