fix(ivy): clone queries correctly for multiple component instances (#27892)

When requesting a queries instance for a node, it was previously
decided whether it needs to be cloned if the node was not already marked
as hosting a query. This check is in place to have only a single queries
instance per node.

The issue with this approach is that no clone is created for subsequent
instantiations of a component, as the TNode is already marked as hosting
a query during first template pass, whereas the cloning of queries
should be independent of first template pass.

To overcome this issue, the queries are assigned an owner TNode such
that it can reliably be determined if a clone needs to be created.

PR Close #27892
This commit is contained in:
JoostK 2019-01-02 19:29:08 +01:00 committed by Andrew Kushnir
parent ddd8cd0573
commit b5c2ef2877
4 changed files with 77 additions and 8 deletions

View File

@ -41,7 +41,7 @@ import {getInitialClassNameValue, initializeStaticContext as initializeStaticSty
import {BoundPlayerFactory} from './styling/player_factory'; import {BoundPlayerFactory} from './styling/player_factory';
import {createEmptyStylingContext, getStylingContext, hasClassInput, hasStyling, isAnimationProp} from './styling/util'; import {createEmptyStylingContext, getStylingContext, hasClassInput, hasStyling, isAnimationProp} from './styling/util';
import {NO_CHANGE} from './tokens'; import {NO_CHANGE} from './tokens';
import {findComponentView, getComponentViewByIndex, getNativeByIndex, getNativeByTNode, getRootContext, getRootView, getTNode, isComponent, isComponentDef, loadInternal, readElementValue, readPatchedLView, renderStringify} from './util'; import {findComponentView, getComponentViewByIndex, getNativeByIndex, getNativeByTNode, getRootContext, getRootView, getTNode, isComponent, isComponentDef, isContentQueryHost, loadInternal, readElementValue, readPatchedLView, renderStringify} from './util';
@ -624,7 +624,7 @@ export function elementCreate(name: string, overriddenRenderer?: Renderer3): REl
* @param localRefExtractor mapping function that extracts local ref value from TNode * @param localRefExtractor mapping function that extracts local ref value from TNode
*/ */
function createDirectivesAndLocals( function createDirectivesAndLocals(
tView: TView, viewData: LView, localRefs: string[] | null | undefined, tView: TView, lView: LView, localRefs: string[] | null | undefined,
localRefExtractor: LocalRefExtractor = getNativeByTNode) { localRefExtractor: LocalRefExtractor = getNativeByTNode) {
if (!getBindingsEnabled()) return; if (!getBindingsEnabled()) return;
const previousOrParentTNode = getPreviousOrParentTNode(); const previousOrParentTNode = getPreviousOrParentTNode();
@ -632,12 +632,19 @@ function createDirectivesAndLocals(
ngDevMode && ngDevMode.firstTemplatePass++; ngDevMode && ngDevMode.firstTemplatePass++;
resolveDirectives( resolveDirectives(
tView, viewData, findDirectiveMatches(tView, viewData, previousOrParentTNode), tView, lView, findDirectiveMatches(tView, lView, previousOrParentTNode),
previousOrParentTNode, localRefs || null); previousOrParentTNode, localRefs || null);
} else {
// During first template pass, queries are created or cloned when first requested
// using `getOrCreateCurrentQueries`. For subsequent template passes, we clone
// any current LQueries here up-front if the current node hosts a content query.
if (isContentQueryHost(getPreviousOrParentTNode()) && lView[QUERIES]) {
lView[QUERIES] = lView[QUERIES] !.clone();
}
} }
instantiateAllDirectives(tView, viewData, previousOrParentTNode); instantiateAllDirectives(tView, lView, previousOrParentTNode);
invokeDirectivesHostBindings(tView, viewData, previousOrParentTNode); invokeDirectivesHostBindings(tView, lView, previousOrParentTNode);
saveResolvedLocalsInData(viewData, previousOrParentTNode, localRefExtractor); saveResolvedLocalsInData(lView, previousOrParentTNode, localRefExtractor);
} }
/** /**

View File

@ -175,8 +175,9 @@ export function getOrCreateCurrentQueries(
QueryType: {new (parent: null, shallow: null, deep: null): LQueries}): LQueries { QueryType: {new (parent: null, shallow: null, deep: null): LQueries}): LQueries {
const lView = getLView(); const lView = getLView();
let currentQueries = lView[QUERIES]; let currentQueries = lView[QUERIES];
// if this is the first content query on a node, any existing LQueries needs to be cloned // If this is the first content query on a node, any existing LQueries needs to be cloned.
// in subsequent template passes, the cloning occurs before directive instantiation. // In subsequent template passes, the cloning occurs before directive instantiation
// in `createDirectivesAndLocals`.
if (previousOrParentTNode && previousOrParentTNode !== lView[HOST_NODE] && if (previousOrParentTNode && previousOrParentTNode !== lView[HOST_NODE] &&
!isContentQueryHost(previousOrParentTNode)) { !isContentQueryHost(previousOrParentTNode)) {
currentQueries && (currentQueries = lView[QUERIES] = currentQueries.clone()); currentQueries && (currentQueries = lView[QUERIES] = currentQueries.clone());

View File

@ -878,6 +878,9 @@
{ {
"name": "isComponentDef" "name": "isComponentDef"
}, },
{
"name": "isContentQueryHost"
},
{ {
"name": "isContextDirty" "name": "isContextDirty"
}, },

View File

@ -2604,6 +2604,64 @@ describe('query', () => {
expect(inInstance !.fooBars.length).toBe(2); expect(inInstance !.fooBars.length).toBe(2);
}); });
it('should support nested shallow content queries across multiple component instances', () => {
let outInstance: QueryDirective;
let inInstance: QueryDirective;
class QueryDirective {
fooBars: any;
static ngDirectiveDef = defineDirective({
type: QueryDirective,
selectors: [['', 'query', '']],
exportAs: ['query'],
factory: () => new QueryDirective(),
contentQueries: (dirIndex) => {
// @ContentChildren('foo', {descendants: true}) fooBars:
// QueryList<ElementRef>;
registerContentQuery(query(null, ['foo'], false), dirIndex);
},
contentQueriesRefresh: (dirIndex: number, queryStartIdx: number) => {
let tmp: any;
const instance = load<QueryDirective>(dirIndex);
queryRefresh(tmp = loadQueryList<ElementRef>(queryStartIdx)) &&
(instance.fooBars = tmp);
},
});
}
const AppComponent = createComponent(
'app-component',
/**
* <div query #out="query">
* <div query #in="query" #foo>
* <span #foo></span>
* </div>
* </div>
*/
function(rf: RenderFlags, ctx: any) {
if (rf & RenderFlags.Create) {
elementStart(0, 'div', ['query', ''], ['out', 'query']);
{ element(2, 'div', ['query', ''], ['in', 'query', 'foo', '']); }
elementEnd();
}
if (rf & RenderFlags.Update) {
outInstance = load<QueryDirective>(1);
inInstance = load<QueryDirective>(3);
}
},
5, 0, [QueryDirective]);
const fixture1 = new ComponentFixture(AppComponent);
expect(outInstance !.fooBars.length).toBe(1);
expect(inInstance !.fooBars.length).toBe(1);
outInstance = inInstance = null !;
const fixture2 = new ComponentFixture(AppComponent);
expect(outInstance !.fooBars.length).toBe(1);
expect(inInstance !.fooBars.length).toBe(1);
});
it('should respect shallow flag on content queries when mixing deep and shallow queries', it('should respect shallow flag on content queries when mixing deep and shallow queries',
() => { () => {
class ShallowQueryDirective { class ShallowQueryDirective {