Skip to content

Commit

Permalink
fix(core): properly execute content queries for root components (#54457)
Browse files Browse the repository at this point in the history
Prior to this fix an incorrect view instance (a dynamically created component
one instead of the root view) was passed to the content query function. Having
incorrect view instance meant that a component instance could not be found.

This is a pre-existing bug, introduction of signal-based queries just surfaced it.

Fixes #54450

PR Close #54457
  • Loading branch information
pkozlowski-opensource authored and AndrewKushnir committed Feb 15, 2024
1 parent b975ff1 commit 3bd5860
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 6 deletions.
3 changes: 1 addition & 2 deletions packages/core/src/render3/component_ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ import {CONTEXT, HEADER_OFFSET, INJECTOR, LView, LViewEnvironment, LViewFlags, T
import {MATH_ML_NAMESPACE, SVG_NAMESPACE} from './namespaces';
import {createElementNode, setupStaticAttributes, writeDirectClass} from './node_manipulation';
import {extractAttrsAndClassesFromSelector, stringifyCSSSelectorList} from './node_selector_matcher';
import {EffectScheduler} from './reactivity/effect';
import {enterView, getCurrentTNode, getLView, leaveView} from './state';
import {computeStaticStyling} from './styling/static_styling';
import {mergeHostAttrs, setUpAttributes} from './util/attrs_utils';
Expand Down Expand Up @@ -504,7 +503,7 @@ function createRootComponent<T>(

// We want to generate an empty QueryList for root content queries for backwards
// compatibility with ViewEngine.
executeContentQueries(tView, rootTNode, componentView);
executeContentQueries(tView, rootTNode, rootLView);

return component;
}
Expand Down
10 changes: 7 additions & 3 deletions packages/core/src/render3/instructions/shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,11 @@ import {attachPatchData} from '../context_discovery';
import {getFactoryDef} from '../definition_factory';
import {diPublicInInjector, getNodeInjectable, getOrCreateNodeInjectorForNode} from '../di';
import {throwMultipleComponentError} from '../errors';
import {AttributeMarker} from '../interfaces/attribute_marker';
import {CONTAINER_HEADER_OFFSET, LContainer} from '../interfaces/container';
import {ComponentDef, ComponentTemplate, DirectiveDef, DirectiveDefListOrFactory, HostBindingsFunction, HostDirectiveBindingMap, HostDirectiveDefs, PipeDefListOrFactory, RenderFlags, ViewQueriesFunction} from '../interfaces/definition';
import {NodeInjectorFactory} from '../interfaces/injector';
import {InputFlags} from '../interfaces/input_flags';
import {getUniqueLViewId} from '../interfaces/lview_tracking';
import {InitialInputData, InitialInputs, LocalRefExtractor, NodeInputBindings, NodeOutputBindings, TAttributes, TConstantsOrFactory, TContainerNode, TDirectiveHostNode, TElementContainerNode, TElementNode, TIcuContainerNode, TNode, TNodeFlags, TNodeType, TProjectionNode} from '../interfaces/node';
import {Renderer} from '../interfaces/renderer';
Expand All @@ -56,8 +58,6 @@ import {selectIndexInternal} from './advance';
import {ɵɵdirectiveInject} from './di';
import {handleUnknownPropertyError, isPropertyValid, matchingSchemas} from './element_validation';
import {writeToDirectiveInput} from './write_to_directive_input';
import { AttributeMarker } from '../interfaces/attribute_marker';
import { InputFlags } from '../interfaces/input_flags';

/**
* Invoke `HostBindingsFunction`s for view.
Expand Down Expand Up @@ -287,7 +287,11 @@ export function executeContentQueries(tView: TView, tNode: TNode, lView: LView)
for (let directiveIndex = start; directiveIndex < end; directiveIndex++) {
const def = tView.data[directiveIndex] as DirectiveDef<any>;
if (def.contentQueries) {
def.contentQueries(RenderFlags.Create, lView[directiveIndex], directiveIndex);
const directiveInstance = lView[directiveIndex];
ngDevMode &&
assertDefined(
directiveIndex, 'Incorrect reference to a directive defining a content query');
def.contentQueries(RenderFlags.Create, directiveInstance, directiveIndex);
}
}
} finally {
Expand Down
36 changes: 35 additions & 1 deletion packages/core/test/acceptance/authoring/signal_queries_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {Component, computed, contentChild, contentChildren, Directive, ElementRef, viewChild, viewChildren} from '@angular/core';
import {Component, computed, contentChild, contentChildren, createComponent, Directive, ElementRef, EnvironmentInjector, viewChild, viewChildren} from '@angular/core';
import {TestBed} from '@angular/core/testing';
import {By} from '@angular/platform-browser';

Expand Down Expand Up @@ -441,4 +441,38 @@ describe('queries as signals', () => {
expect(recomputeCount).toBe(1);
});
});

describe('dynamic component creation', () => {
it('should return empty results for content queries of dynamically created components', () => {
// https://github.com/angular/angular/issues/54450
@Component({
selector: 'query-cmp',
standalone: true,
template: ``,
})
class QueryComponent {
elements = contentChildren('el');
element = contentChild('el');
}

@Component({
standalone: true,
template: ``,
})
class TestComponent {
constructor(private _envInjector: EnvironmentInjector) {}

createDynamic() {
return createComponent(QueryComponent, {environmentInjector: this._envInjector});
}
}

const fixture = TestBed.createComponent(TestComponent);
const cmpRef = fixture.componentInstance.createDynamic();
cmpRef.changeDetectorRef.detectChanges();

expect(cmpRef.instance.elements()).toEqual([]);
expect(cmpRef.instance.element()).toBeUndefined();
});
});
});

0 comments on commit 3bd5860

Please sign in to comment.