From 393efa54e669c4ae1e479fd59fc4f9ea69ab77e9 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Wed, 3 Nov 2021 19:42:02 +0000 Subject: [PATCH] fix(compiler): ensure that partially compiled queries can handle forward references (#44113) When a partially compiled component or directive is "linked" in JIT mode, the body of its declaration is evaluated by the JavaScript runtime. If a class is referenced in a query (e.g. `ViewQuery` or `ContentQuery`) but its definition is later in the file, then the reference must be wrapped in a `forwardRef()` call. Previously, query predicates were not wrapped correctly in partial declarations causing the code to crash at runtime. In AOT mode, this code is never evaluated but instead transformed as part of the build, so this bug did not become apparent until Angular Material started running JIT mode tests on its distributable output. This change fixes this problem by noting when queries are wrapped in `forwardRef()` calls and ensuring that this gets passed through to partial compilation declarations and then suitably stripped during linking. See https://github.com/angular/components/pull/23882 and https://github.com/angular/components/issues/23907 PR Close #44113 --- .../partial_component_linker_1.ts | 10 +- .../partial_directive_linker_1.ts | 4 +- .../partial_injectable_linker_1.ts | 7 +- .../src/file_linker/partial_linkers/util.ts | 7 +- .../src/ngtsc/annotations/src/directive.ts | 12 +- .../src/ngtsc/annotations/src/injectable.ts | 9 +- .../queries/GOLDEN_PARTIAL.js | 176 ++++++++++++++++++ .../queries/TEST_CASES.json | 28 +++ .../queries/content_query_forward_ref.js | 27 +++ .../queries/content_query_forward_ref.ts | 34 ++++ .../queries/view_query_forward_ref.js | 16 ++ .../queries/view_query_forward_ref.ts | 32 ++++ packages/compiler/src/compiler.ts | 2 +- .../compiler/src/injectable_compiler_2.ts | 7 +- packages/compiler/src/jit_compiler_facade.ts | 26 ++- packages/compiler/src/render3/partial/api.ts | 4 +- .../compiler/src/render3/partial/directive.ts | 6 +- packages/compiler/src/render3/util.ts | 64 ++++--- packages/compiler/src/render3/view/api.ts | 4 +- packages/compiler/src/render3/view/util.ts | 11 +- .../render3/jit/declare_directive_spec.ts | 44 ++++- 21 files changed, 462 insertions(+), 68 deletions(-) create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/queries/content_query_forward_ref.js create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/queries/content_query_forward_ref.ts create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/queries/view_query_forward_ref.js create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/queries/view_query_forward_ref.ts diff --git a/packages/compiler-cli/linker/src/file_linker/partial_linkers/partial_component_linker_1.ts b/packages/compiler-cli/linker/src/file_linker/partial_linkers/partial_component_linker_1.ts index 778bc2e507f..cc278cb6efc 100644 --- a/packages/compiler-cli/linker/src/file_linker/partial_linkers/partial_component_linker_1.ts +++ b/packages/compiler-cli/linker/src/file_linker/partial_linkers/partial_component_linker_1.ts @@ -5,7 +5,7 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ -import {ChangeDetectionStrategy, compileComponentFromMetadata, ConstantPool, DeclarationListEmitMode, DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig, makeBindingParser, outputAst as o, parseTemplate, R3ComponentMetadata, R3DeclareComponentMetadata, R3DeclareUsedDirectiveMetadata, R3PartialDeclaration, R3UsedDirectiveMetadata, ViewEncapsulation} from '@angular/compiler'; +import {ChangeDetectionStrategy, compileComponentFromMetadata, ConstantPool, DeclarationListEmitMode, DEFAULT_INTERPOLATION_CONFIG, ForwardRefHandling, InterpolationConfig, makeBindingParser, outputAst as o, parseTemplate, R3ComponentMetadata, R3DeclareComponentMetadata, R3DeclareUsedDirectiveMetadata, R3PartialDeclaration, R3UsedDirectiveMetadata, ViewEncapsulation} from '@angular/compiler'; import {AbsoluteFsPath} from '../../../../src/ngtsc/file_system'; import {Range} from '../../ast/ast_host'; @@ -69,8 +69,8 @@ export class PartialComponentLinkerVersion1 implements const type = directiveExpr.getValue('type'); const selector = directiveExpr.getString('selector'); - const {expression: typeExpr, isForwardRef} = extractForwardRef(type); - if (isForwardRef) { + const {expression: typeExpr, forwardRef} = extractForwardRef(type); + if (forwardRef === ForwardRefHandling.Unwrapped) { declarationListEmitMode = DeclarationListEmitMode.Closure; } @@ -101,8 +101,8 @@ export class PartialComponentLinkerVersion1 implements let pipes = new Map(); if (metaObj.has('pipes')) { pipes = metaObj.getObject('pipes').toMap(pipe => { - const {expression: pipeType, isForwardRef} = extractForwardRef(pipe); - if (isForwardRef) { + const {expression: pipeType, forwardRef} = extractForwardRef(pipe); + if (forwardRef === ForwardRefHandling.Unwrapped) { declarationListEmitMode = DeclarationListEmitMode.Closure; } return pipeType; diff --git a/packages/compiler-cli/linker/src/file_linker/partial_linkers/partial_directive_linker_1.ts b/packages/compiler-cli/linker/src/file_linker/partial_linkers/partial_directive_linker_1.ts index 9985841f6f8..41bbe07199c 100644 --- a/packages/compiler-cli/linker/src/file_linker/partial_linkers/partial_directive_linker_1.ts +++ b/packages/compiler-cli/linker/src/file_linker/partial_linkers/partial_directive_linker_1.ts @@ -13,7 +13,7 @@ import {AstObject, AstValue} from '../../ast/ast_value'; import {FatalLinkerError} from '../../fatal_linker_error'; import {PartialLinker} from './partial_linker'; -import {wrapReference} from './util'; +import {extractForwardRef, wrapReference} from './util'; /** * A `PartialLinker` that is designed to process `ɵɵngDeclareDirective()` call expressions. @@ -140,7 +140,7 @@ function toQueryMetadata(obj: AstObject entry.getString()); } else { - predicate = predicateExpr.getOpaque(); + predicate = extractForwardRef(predicateExpr); } return { propertyName: obj.getString('propertyName'), diff --git a/packages/compiler-cli/linker/src/file_linker/partial_linkers/partial_injectable_linker_1.ts b/packages/compiler-cli/linker/src/file_linker/partial_linkers/partial_injectable_linker_1.ts index f4839a77996..d1c11ccab06 100644 --- a/packages/compiler-cli/linker/src/file_linker/partial_linkers/partial_injectable_linker_1.ts +++ b/packages/compiler-cli/linker/src/file_linker/partial_linkers/partial_injectable_linker_1.ts @@ -5,7 +5,7 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ -import {compileInjectable, ConstantPool, createMayBeForwardRefExpression, outputAst as o, R3DeclareInjectableMetadata, R3InjectableMetadata, R3PartialDeclaration} from '@angular/compiler'; +import {compileInjectable, ConstantPool, createMayBeForwardRefExpression, ForwardRefHandling, outputAst as o, R3DeclareInjectableMetadata, R3InjectableMetadata, R3PartialDeclaration} from '@angular/compiler'; import {AstObject} from '../../ast/ast_value'; import {FatalLinkerError} from '../../fatal_linker_error'; @@ -43,8 +43,9 @@ export function toR3InjectableMeta( type: wrapReference(typeExpr.getOpaque()), internalType: typeExpr.getOpaque(), typeArgumentCount: 0, - providedIn: metaObj.has('providedIn') ? extractForwardRef(metaObj.getValue('providedIn')) : - createMayBeForwardRefExpression(o.literal(null), false), + providedIn: metaObj.has('providedIn') ? + extractForwardRef(metaObj.getValue('providedIn')) : + createMayBeForwardRefExpression(o.literal(null), ForwardRefHandling.None), }; if (metaObj.has('useClass')) { diff --git a/packages/compiler-cli/linker/src/file_linker/partial_linkers/util.ts b/packages/compiler-cli/linker/src/file_linker/partial_linkers/util.ts index 48986e807e2..c2e4e30c1b9 100644 --- a/packages/compiler-cli/linker/src/file_linker/partial_linkers/util.ts +++ b/packages/compiler-cli/linker/src/file_linker/partial_linkers/util.ts @@ -5,7 +5,7 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ -import {createMayBeForwardRefExpression, MaybeForwardRefExpression, outputAst as o, R3DeclareDependencyMetadata, R3DependencyMetadata, R3Reference} from '@angular/compiler'; +import {createMayBeForwardRefExpression, ForwardRefHandling, MaybeForwardRefExpression, outputAst as o, R3DeclareDependencyMetadata, R3DependencyMetadata, R3Reference} from '@angular/compiler'; import {AstObject, AstValue} from '../../ast/ast_value'; import {FatalLinkerError} from '../../fatal_linker_error'; @@ -68,7 +68,7 @@ export function getDependency( export function extractForwardRef(expr: AstValue): MaybeForwardRefExpression> { if (!expr.isCallExpression()) { - return createMayBeForwardRefExpression(expr.getOpaque(), /* isForwardRef */ false); + return createMayBeForwardRefExpression(expr.getOpaque(), ForwardRefHandling.None); } const callee = expr.getCallee(); @@ -90,5 +90,6 @@ export function extractForwardRef(expr: AstValue; } +/**************************************************************************************************** + * PARTIAL FILE: view_query_forward_ref.js + ****************************************************************************************************/ +import { Component, Directive, forwardRef, NgModule, ViewChild, ViewChildren } from '@angular/core'; +import * as i0 from "@angular/core"; +export class ViewQueryComponent { +} +ViewQueryComponent.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: ViewQueryComponent, deps: [], target: i0.ɵɵFactoryTarget.Component }); +ViewQueryComponent.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", type: ViewQueryComponent, selector: "view-query-component", viewQueries: [{ propertyName: "someDir", first: true, predicate: i0.forwardRef(function () { return SomeDirective; }), descendants: true }, { propertyName: "someDirList", predicate: i0.forwardRef(function () { return SomeDirective; }), descendants: true }], ngImport: i0, template: ` +
+ `, isInline: true, directives: [{ type: i0.forwardRef(function () { return SomeDirective; }), selector: "[someDir]" }] }); +i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: ViewQueryComponent, decorators: [{ + type: Component, + args: [{ + selector: 'view-query-component', + template: ` +
+ ` + }] + }], propDecorators: { someDir: [{ + type: ViewChild, + args: [forwardRef(() => SomeDirective)] + }], someDirList: [{ + type: ViewChildren, + args: [forwardRef(() => SomeDirective)] + }] } }); +export class MyApp { +} +MyApp.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, deps: [], target: i0.ɵɵFactoryTarget.Component }); +MyApp.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", type: MyApp, selector: "my-app", ngImport: i0, template: ` + + `, isInline: true, components: [{ type: ViewQueryComponent, selector: "view-query-component" }] }); +i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, decorators: [{ + type: Component, + args: [{ + selector: 'my-app', + template: ` + + ` + }] + }] }); +export class SomeDirective { +} +SomeDirective.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: SomeDirective, deps: [], target: i0.ɵɵFactoryTarget.Directive }); +SomeDirective.ɵdir = i0.ɵɵngDeclareDirective({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", type: SomeDirective, selector: "[someDir]", ngImport: i0 }); +i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: SomeDirective, decorators: [{ + type: Directive, + args: [{ + selector: '[someDir]', + }] + }] }); +export class MyModule { +} +MyModule.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyModule, deps: [], target: i0.ɵɵFactoryTarget.NgModule }); +MyModule.ɵmod = i0.ɵɵngDeclareNgModule({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyModule, declarations: [SomeDirective, ViewQueryComponent, MyApp] }); +MyModule.ɵinj = i0.ɵɵngDeclareInjector({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyModule }); +i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyModule, decorators: [{ + type: NgModule, + args: [{ declarations: [SomeDirective, ViewQueryComponent, MyApp] }] + }] }); + +/**************************************************************************************************** + * PARTIAL FILE: view_query_forward_ref.d.ts + ****************************************************************************************************/ +import { QueryList } from '@angular/core'; +import * as i0 from "@angular/core"; +export declare class ViewQueryComponent { + someDir: SomeDirective; + someDirList: QueryList; + static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵcmp: i0.ɵɵComponentDeclaration; +} +export declare class MyApp { + static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵcmp: i0.ɵɵComponentDeclaration; +} +export declare class SomeDirective { + static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵdir: i0.ɵɵDirectiveDeclaration; +} +export declare class MyModule { + static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵmod: i0.ɵɵNgModuleDeclaration; + static ɵinj: i0.ɵɵInjectorDeclaration; +} + /**************************************************************************************************** * PARTIAL FILE: view_query_for_local_ref.js ****************************************************************************************************/ @@ -410,6 +496,96 @@ export declare class MyModule { static ɵinj: i0.ɵɵInjectorDeclaration; } +/**************************************************************************************************** + * PARTIAL FILE: content_query_forward_ref.js + ****************************************************************************************************/ +import { Component, ContentChild, ContentChildren, Directive, forwardRef, NgModule } from '@angular/core'; +import * as i0 from "@angular/core"; +export class ContentQueryComponent { +} +ContentQueryComponent.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: ContentQueryComponent, deps: [], target: i0.ɵɵFactoryTarget.Component }); +ContentQueryComponent.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", type: ContentQueryComponent, selector: "content-query-component", queries: [{ propertyName: "someDir", first: true, predicate: i0.forwardRef(function () { return SomeDirective; }), descendants: true }, { propertyName: "someDirList", predicate: i0.forwardRef(function () { return SomeDirective; }) }], ngImport: i0, template: ` +
+ `, isInline: true }); +i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: ContentQueryComponent, decorators: [{ + type: Component, + args: [{ + selector: 'content-query-component', + template: ` +
+ ` + }] + }], propDecorators: { someDir: [{ + type: ContentChild, + args: [forwardRef(() => SomeDirective)] + }], someDirList: [{ + type: ContentChildren, + args: [forwardRef(() => SomeDirective)] + }] } }); +export class MyApp { +} +MyApp.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, deps: [], target: i0.ɵɵFactoryTarget.Component }); +MyApp.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", type: MyApp, selector: "my-app", ngImport: i0, template: ` + +
+ + `, isInline: true, components: [{ type: i0.forwardRef(function () { return ContentQueryComponent; }), selector: "content-query-component" }], directives: [{ type: i0.forwardRef(function () { return SomeDirective; }), selector: "[someDir]" }] }); +i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, decorators: [{ + type: Component, + args: [{ + selector: 'my-app', + template: ` + +
+ + ` + }] + }] }); +export class SomeDirective { +} +SomeDirective.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: SomeDirective, deps: [], target: i0.ɵɵFactoryTarget.Directive }); +SomeDirective.ɵdir = i0.ɵɵngDeclareDirective({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", type: SomeDirective, selector: "[someDir]", ngImport: i0 }); +i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: SomeDirective, decorators: [{ + type: Directive, + args: [{ + selector: '[someDir]', + }] + }] }); +export class MyModule { +} +MyModule.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyModule, deps: [], target: i0.ɵɵFactoryTarget.NgModule }); +MyModule.ɵmod = i0.ɵɵngDeclareNgModule({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyModule, declarations: [SomeDirective, ContentQueryComponent, MyApp] }); +MyModule.ɵinj = i0.ɵɵngDeclareInjector({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyModule }); +i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyModule, decorators: [{ + type: NgModule, + args: [{ declarations: [SomeDirective, ContentQueryComponent, MyApp] }] + }] }); + +/**************************************************************************************************** + * PARTIAL FILE: content_query_forward_ref.d.ts + ****************************************************************************************************/ +import { QueryList } from '@angular/core'; +import * as i0 from "@angular/core"; +export declare class ContentQueryComponent { + someDir: SomeDirective; + someDirList: QueryList; + static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵcmp: i0.ɵɵComponentDeclaration; +} +export declare class MyApp { + static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵcmp: i0.ɵɵComponentDeclaration; +} +export declare class SomeDirective { + static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵdir: i0.ɵɵDirectiveDeclaration; +} +export declare class MyModule { + static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵmod: i0.ɵɵNgModuleDeclaration; + static ɵinj: i0.ɵɵInjectorDeclaration; +} + /**************************************************************************************************** * PARTIAL FILE: content_query_for_local_ref.js ****************************************************************************************************/ diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/queries/TEST_CASES.json b/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/queries/TEST_CASES.json index a842eacb0ac..8978b21dc51 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/queries/TEST_CASES.json +++ b/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/queries/TEST_CASES.json @@ -16,6 +16,20 @@ } ] }, + { + "description": "should support view queries with forwardRefs", + "inputFiles": [ + "view_query_forward_ref.ts" + ], + "expectations": [ + { + "failureMessage": "Invalid ViewQuery declaration", + "files": [ + "view_query_forward_ref.js" + ] + } + ] + }, { "description": "should support view queries with local refs", "inputFiles": [ @@ -75,6 +89,20 @@ } ] }, + { + "description": "should support content queries with forwardRefs", + "inputFiles": [ + "content_query_forward_ref.ts" + ], + "expectations": [ + { + "failureMessage": "Invalid ContentQuery declaration", + "files": [ + "content_query_forward_ref.js" + ] + } + ] + }, { "description": "should support content queries with local refs", "inputFiles": [ diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/queries/content_query_forward_ref.js b/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/queries/content_query_forward_ref.js new file mode 100644 index 00000000000..a5fb6aa5c19 --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/queries/content_query_forward_ref.js @@ -0,0 +1,27 @@ +ContentQueryComponent.ɵcmp = /*@__PURE__*/ $r3$.ɵɵdefineComponent({ + type: ContentQueryComponent, + selectors: [["content-query-component"]], + contentQueries: function ContentQueryComponent_ContentQueries(rf, ctx, dirIndex) { + if (rf & 1) { + $r3$.ɵɵcontentQuery(dirIndex, SomeDirective, __QueryFlags.descendants__|__QueryFlags.emitDistinctChangesOnly__); + $r3$.ɵɵcontentQuery(dirIndex, SomeDirective, __QueryFlags.emitDistinctChangesOnly__); + } + if (rf & 2) { + let $tmp$; + $r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.someDir = $tmp$.first); + $r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.someDirList = $tmp$); + } + }, + ngContentSelectors: _c0, + decls: 2, + vars: 0, + template: function ContentQueryComponent_Template(rf, ctx) { + if (rf & 1) { + $r3$.ɵɵprojectionDef(); + $r3$.ɵɵelementStart(0, "div"); + $r3$.ɵɵprojection(1); + $r3$.ɵɵelementEnd(); + } + }, + encapsulation: 2 +}); diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/queries/content_query_forward_ref.ts b/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/queries/content_query_forward_ref.ts new file mode 100644 index 00000000000..73fe0170ea8 --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/queries/content_query_forward_ref.ts @@ -0,0 +1,34 @@ +import {Component, ContentChild, ContentChildren, Directive, forwardRef, NgModule, QueryList} from '@angular/core'; + +@Component({ + selector: 'content-query-component', + template: ` +
+ ` +}) +export class ContentQueryComponent { + @ContentChild(forwardRef(() => SomeDirective)) someDir!: SomeDirective; + @ContentChildren(forwardRef(() => SomeDirective)) someDirList!: QueryList; +} + +@Component({ + selector: 'my-app', + template: ` + +
+ + ` +}) +export class MyApp { +} + + +@Directive({ + selector: '[someDir]', +}) +export class SomeDirective { +} + +@NgModule({declarations: [SomeDirective, ContentQueryComponent, MyApp]}) +export class MyModule { +} diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/queries/view_query_forward_ref.js b/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/queries/view_query_forward_ref.js new file mode 100644 index 00000000000..6f04af70e9c --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/queries/view_query_forward_ref.js @@ -0,0 +1,16 @@ +ViewQueryComponent.ɵcmp = /*@__PURE__*/ $r3$.ɵɵdefineComponent({ + type: ViewQueryComponent, + selectors: [["view-query-component"]], + viewQuery: function ViewQueryComponent_Query(rf, ctx) { + if (rf & 1) { + $r3$.ɵɵviewQuery(SomeDirective, __QueryFlags.descendants__|__QueryFlags.emitDistinctChangesOnly__); + $r3$.ɵɵviewQuery(SomeDirective, __QueryFlags.descendants__|__QueryFlags.emitDistinctChangesOnly__); + } + if (rf & 2) { + let $tmp$; + $r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.someDir = $tmp$.first); + $r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.someDirList = $tmp$); + } + }, + // ... +}); diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/queries/view_query_forward_ref.ts b/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/queries/view_query_forward_ref.ts new file mode 100644 index 00000000000..c6acd8409db --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/queries/view_query_forward_ref.ts @@ -0,0 +1,32 @@ +import {Component, Directive, forwardRef, NgModule, QueryList, ViewChild, ViewChildren} from '@angular/core'; + +@Component({ + selector: 'view-query-component', + template: ` +
+ ` +}) +export class ViewQueryComponent { + @ViewChild(forwardRef(() => SomeDirective)) someDir!: SomeDirective; + @ViewChildren(forwardRef(() => SomeDirective)) someDirList!: QueryList; +} + +@Component({ + selector: 'my-app', + template: ` + + ` +}) +export class MyApp { +} + + +@Directive({ + selector: '[someDir]', +}) +export class SomeDirective { +} + +@NgModule({declarations: [SomeDirective, ViewQueryComponent, MyApp]}) +export class MyModule { +} diff --git a/packages/compiler/src/compiler.ts b/packages/compiler/src/compiler.ts index b38082ab531..01db7147834 100644 --- a/packages/compiler/src/compiler.ts +++ b/packages/compiler/src/compiler.ts @@ -103,7 +103,7 @@ export {compileNgModule, R3NgModuleMetadata} from './render3/r3_module_compiler' export {compileInjector, R3InjectorMetadata} from './render3/r3_injector_compiler'; export {compilePipeFromMetadata, R3PipeMetadata} from './render3/r3_pipe_compiler'; export {makeBindingParser, ParsedTemplate, parseTemplate, ParseTemplateOptions} from './render3/view/template'; -export {MaybeForwardRefExpression, R3CompiledExpression, R3Reference, createMayBeForwardRefExpression, devOnlyGuardedExpression, getSafePropertyAccessString} from './render3/util'; +export {ForwardRefHandling, MaybeForwardRefExpression, R3CompiledExpression, R3Reference, createMayBeForwardRefExpression, devOnlyGuardedExpression, getSafePropertyAccessString} from './render3/util'; export {compileComponentFromMetadata, compileDirectiveFromMetadata, parseHostBindings, ParsedHostBindings, verifyHostBindings} from './render3/view/compiler'; export {compileDeclareClassMetadata} from './render3/partial/class_metadata'; export {compileDeclareComponentFromMetadata, DeclareComponentTemplateInfo} from './render3/partial/component'; diff --git a/packages/compiler/src/injectable_compiler_2.ts b/packages/compiler/src/injectable_compiler_2.ts index 1ec5eef78e0..897a2134fc4 100644 --- a/packages/compiler/src/injectable_compiler_2.ts +++ b/packages/compiler/src/injectable_compiler_2.ts @@ -9,7 +9,7 @@ import * as o from './output/output_ast'; import {compileFactoryFunction, FactoryTarget, R3DependencyMetadata, R3FactoryDelegateType, R3FactoryMetadata} from './render3/r3_factory'; import {Identifiers} from './render3/r3_identifiers'; -import {generateForwardRef, MaybeForwardRefExpression, R3CompiledExpression, R3Reference, typeWithParameters} from './render3/util'; +import {convertFromMaybeForwardRefExpression, ForwardRefHandling, generateForwardRef, MaybeForwardRefExpression, R3CompiledExpression, R3Reference, typeWithParameters} from './render3/util'; import {DefinitionMap} from './render3/view/util'; export interface R3InjectableMetadata { @@ -116,10 +116,7 @@ export function compileInjectable( // Only generate providedIn property if it has a non-null value if ((meta.providedIn.expression as o.LiteralExpr).value !== null) { - injectableProps.set( - 'providedIn', - meta.providedIn.isForwardRef ? generateForwardRef(meta.providedIn.expression) : - meta.providedIn.expression); + injectableProps.set('providedIn', convertFromMaybeForwardRefExpression(meta.providedIn)); } const expression = o.importExpr(Identifiers.ɵɵdefineInjectable) diff --git a/packages/compiler/src/jit_compiler_facade.ts b/packages/compiler/src/jit_compiler_facade.ts index 7a08d90c48e..527583e0d79 100644 --- a/packages/compiler/src/jit_compiler_facade.ts +++ b/packages/compiler/src/jit_compiler_facade.ts @@ -20,12 +20,13 @@ import {compileInjector, R3InjectorMetadata} from './render3/r3_injector_compile import {R3JitReflector} from './render3/r3_jit'; import {compileNgModule, compileNgModuleDeclarationExpression, R3NgModuleMetadata} from './render3/r3_module_compiler'; import {compilePipeFromMetadata, R3PipeMetadata} from './render3/r3_pipe_compiler'; -import {createMayBeForwardRefExpression, getSafePropertyAccessString, MaybeForwardRefExpression, wrapReference} from './render3/util'; +import {createMayBeForwardRefExpression, ForwardRefHandling, getSafePropertyAccessString, MaybeForwardRefExpression, wrapReference} from './render3/util'; import {DeclarationListEmitMode, R3ComponentMetadata, R3DirectiveMetadata, R3HostMetadata, R3QueryMetadata, R3UsedDirectiveMetadata} from './render3/view/api'; import {compileComponentFromMetadata, compileDirectiveFromMetadata, ParsedHostBindings, parseHostBindings, verifyHostBindings} from './render3/view/compiler'; import {makeBindingParser, parseTemplate} from './render3/view/template'; import {ResourceLoader} from './resource_loader'; import {DomElementSchemaRegistry} from './schema/dom_element_schema_registry'; +import {resolveForwardRef} from './util'; export class CompilerFacadeImpl implements CompilerFacade { FactoryTarget = FactoryTarget as any; @@ -293,8 +294,7 @@ const USE_EXISTING = Object.keys({useExisting: null})[0]; function convertToR3QueryMetadata(facade: R3QueryMetadataFacade): R3QueryMetadata { return { ...facade, - predicate: Array.isArray(facade.predicate) ? facade.predicate : - new WrappedNodeExpr(facade.predicate), + predicate: convertQueryPredicate(facade.predicate), read: facade.read ? new WrappedNodeExpr(facade.read) : null, static: facade.static, emitDistinctChangesOnly: facade.emitDistinctChangesOnly, @@ -306,8 +306,7 @@ function convertQueryDeclarationToMetadata(declaration: R3DeclareQueryMetadataFa return { propertyName: declaration.propertyName, first: declaration.first ?? false, - predicate: Array.isArray(declaration.predicate) ? declaration.predicate : - new WrappedNodeExpr(declaration.predicate), + predicate: convertQueryPredicate(declaration.predicate), descendants: declaration.descendants ?? false, read: declaration.read ? new WrappedNodeExpr(declaration.read) : null, static: declaration.static ?? false, @@ -315,6 +314,15 @@ function convertQueryDeclarationToMetadata(declaration: R3DeclareQueryMetadataFa }; } +function convertQueryPredicate(predicate: OpaqueValue|string[]): MaybeForwardRefExpression| + string[] { + return Array.isArray(predicate) ? + // The predicate is an array of strings so pass it through. + predicate : + // The predicate is a type - assume that we will need to unwrap any `forwardRef()` calls. + createMayBeForwardRefExpression(new WrappedNodeExpr(predicate), ForwardRefHandling.Wrapped); +} + function convertDirectiveFacadeToMetadata(facade: R3DirectiveMetadataFacade): R3DirectiveMetadata { const inputsFromMetadata = parseInputOutputs(facade.inputs || []); const outputsFromMetadata = parseInputOutputs(facade.outputs || []); @@ -475,13 +483,13 @@ type R3DirectiveMetadataFacadeNoPropAndWhitespace = * In JIT mode we do not want the compiler to wrap the expression in a `forwardRef()` call because, * if it is referencing a type that has not yet been defined, it will have already been wrapped in * a `forwardRef()` - either by the application developer or during partial-compilation. Thus we can - * set `isForwardRef` to `false`. + * use `ForwardRefHandling.None`. */ function convertToProviderExpression(obj: any, property: string): MaybeForwardRefExpression| undefined { if (obj.hasOwnProperty(property)) { return createMayBeForwardRefExpression( - new WrappedNodeExpr(obj[property]), /* isForwardRef */ false); + new WrappedNodeExpr(obj[property]), ForwardRefHandling.None); } else { return undefined; } @@ -498,8 +506,8 @@ function wrapExpression(obj: any, property: string): WrappedNodeExpr|undefi function computeProvidedIn(providedIn: Function|string|null|undefined): MaybeForwardRefExpression { const expression = typeof providedIn === 'function' ? new WrappedNodeExpr(providedIn) : new LiteralExpr(providedIn ?? null); - // See `convertToProviderExpression()` for why `isForwardRef` is false. - return createMayBeForwardRefExpression(expression, /* isForwardRef */ false); + // See `convertToProviderExpression()` for why this uses `ForwardRefHandling.None`. + return createMayBeForwardRefExpression(expression, ForwardRefHandling.None); } function convertR3DependencyMetadataArray(facades: R3DependencyMetadataFacade[]|null| diff --git a/packages/compiler/src/render3/partial/api.ts b/packages/compiler/src/render3/partial/api.ts index a6ce0dcfedd..3009c773715 100644 --- a/packages/compiler/src/render3/partial/api.ts +++ b/packages/compiler/src/render3/partial/api.ts @@ -233,8 +233,8 @@ export interface R3DeclareQueryMetadata { first?: boolean; /** - * Either an expression representing a type or `InjectionToken` for the query - * predicate, or a set of string selectors. + * Either an expression representing a type (possibly wrapped in a `forwardRef()`) or + * `InjectionToken` for the query predicate, or a set of string selectors. */ predicate: o.Expression|string[]; diff --git a/packages/compiler/src/render3/partial/directive.ts b/packages/compiler/src/render3/partial/directive.ts index 310146963b6..1ffac2e8b6a 100644 --- a/packages/compiler/src/render3/partial/directive.ts +++ b/packages/compiler/src/render3/partial/directive.ts @@ -7,7 +7,7 @@ */ import * as o from '../../output/output_ast'; import {Identifiers as R3} from '../r3_identifiers'; -import {R3CompiledExpression} from '../util'; +import {convertFromMaybeForwardRefExpression, R3CompiledExpression} from '../util'; import {R3DirectiveMetadata, R3HostMetadata, R3QueryMetadata} from '../view/api'; import {createDirectiveType} from '../view/compiler'; import {asLiteral, conditionallyCreateMapObjectLiteral, DefinitionMap} from '../view/util'; @@ -96,7 +96,9 @@ function compileQuery(query: R3QueryMetadata): o.LiteralMapExpr { meta.set('first', o.literal(true)); } meta.set( - 'predicate', Array.isArray(query.predicate) ? asLiteral(query.predicate) : query.predicate); + 'predicate', + Array.isArray(query.predicate) ? asLiteral(query.predicate) : + convertFromMaybeForwardRefExpression(query.predicate)); if (!query.emitDistinctChangesOnly) { // `emitDistinctChangesOnly` is special because we expect it to be `true`. // Therefore we explicitly emit the field, and explicitly place it only when it's `false`. diff --git a/packages/compiler/src/render3/util.ts b/packages/compiler/src/render3/util.ts index 0a2922f53db..a7c30c3fe9c 100644 --- a/packages/compiler/src/render3/util.ts +++ b/packages/compiler/src/render3/util.ts @@ -94,48 +94,52 @@ export interface MaybeForwardRefExpression( - expression: T, isForwardRef: boolean): MaybeForwardRefExpression { - return {expression, isForwardRef}; + expression: T, forwardRef: ForwardRefHandling): MaybeForwardRefExpression { + return {expression, forwardRef}; } /** * Convert a `MaybeForwardRefExpression` to an `Expression`, possibly wrapping its expression in a * `forwardRef()` call. * - * If `MaybeForwardRefExpression.isForwardRef` is true then the expression was originally wrapped in - * a `forwardRef()` call to prevent the value from being eagerly evaluated in the code. - * - * Normally, the linker will statically process the code, putting the `expression` inside a factory - * function so the `forwardRef()` wrapper is not evaluated before it has been defined. But if the - * partial declaration is evaluated by the JIT compiler the `forwardRef()` call is still needed to - * prevent eager evaluation of the `expression`. - * - * So in partial declarations, expressions that could be forward-refs are wrapped in `forwardRef()` - * calls, and this is then unwrapped in the linker as necessary. + * If `MaybeForwardRefExpression.forwardRef` is `ForwardRefHandling.Unwrapped` then the expression + * was originally wrapped in a `forwardRef()` call to prevent the value from being eagerly evaluated + * in the code. * * See `packages/compiler-cli/src/ngtsc/annotations/src/injectable.ts` and * `packages/compiler/src/jit_compiler_facade.ts` for more information. */ export function convertFromMaybeForwardRefExpression( - {expression, isForwardRef}: MaybeForwardRefExpression): o.Expression { - return isForwardRef ? generateForwardRef(expression) : expression; + {expression, forwardRef}: MaybeForwardRefExpression): o.Expression { + switch (forwardRef) { + case ForwardRefHandling.None: + case ForwardRefHandling.Wrapped: + return expression; + case ForwardRefHandling.Unwrapped: + return generateForwardRef(expression); + } } /** @@ -148,3 +152,15 @@ export function convertFromMaybeForwardRefExpression( export function generateForwardRef(expr: o.Expression): o.Expression { return o.importExpr(Identifiers.forwardRef).callFn([o.fn([], [new o.ReturnStatement(expr)])]); } + +/** + * Specifies how a forward ref has been handled in a MaybeForwardRefExpression + */ +export const enum ForwardRefHandling { + /** The expression was not wrapped in a `forwardRef()` call in the first place. */ + None, + /** The expression is still wrapped in a `forwardRef()` call. */ + Wrapped, + /** The expression was wrapped in a `forwardRef()` call but has since been unwrapped. */ + Unwrapped, +} diff --git a/packages/compiler/src/render3/view/api.ts b/packages/compiler/src/render3/view/api.ts index 2e1d67a545b..64c7cc29efe 100644 --- a/packages/compiler/src/render3/view/api.ts +++ b/packages/compiler/src/render3/view/api.ts @@ -12,7 +12,7 @@ import * as o from '../../output/output_ast'; import {ParseSourceSpan} from '../../parse_util'; import * as t from '../r3_ast'; import {R3DependencyMetadata} from '../r3_factory'; -import {R3Reference} from '../util'; +import {MaybeForwardRefExpression, R3Reference} from '../util'; /** @@ -302,7 +302,7 @@ export interface R3QueryMetadata { * Either an expression representing a type or `InjectionToken` for the query * predicate, or a set of string selectors. */ - predicate: o.Expression|string[]; + predicate: MaybeForwardRefExpression|string[]; /** * Whether to include only direct children or all descendants. diff --git a/packages/compiler/src/render3/view/util.ts b/packages/compiler/src/render3/view/util.ts index e237933a982..f9685832bfc 100644 --- a/packages/compiler/src/render3/view/util.ts +++ b/packages/compiler/src/render3/view/util.ts @@ -12,6 +12,8 @@ import * as o from '../../output/output_ast'; import {ParseSourceSpan} from '../../parse_util'; import {splitAtColon} from '../../util'; import * as t from '../r3_ast'; +import {Identifiers as R3} from '../r3_identifiers'; +import {ForwardRefHandling} from '../util'; import {R3QueryMetadata} from './api'; import {isI18nAttribute} from './i18n/util'; @@ -148,7 +150,14 @@ export function getQueryPredicate( }); return constantPool.getConstLiteral(o.literalArr(predicate), true); } else { - return query.predicate; + // The original predicate may have been wrapped in a `forwardRef()` call. + switch (query.predicate.forwardRef) { + case ForwardRefHandling.None: + case ForwardRefHandling.Unwrapped: + return query.predicate.expression; + case ForwardRefHandling.Wrapped: + return o.importExpr(R3.resolveForwardRef).callFn([query.predicate.expression]); + } } } diff --git a/packages/core/test/render3/jit/declare_directive_spec.ts b/packages/core/test/render3/jit/declare_directive_spec.ts index d9aee93ee81..1567bd58d8f 100644 --- a/packages/core/test/render3/jit/declare_directive_spec.ts +++ b/packages/core/test/render3/jit/declare_directive_spec.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {ElementRef, ɵɵngDeclareDirective} from '@angular/core'; +import {ElementRef, forwardRef, ɵɵngDeclareDirective} from '@angular/core'; import {AttributeMarker, DirectiveDef} from '../../../src/render3'; import {functionContaining} from './matcher'; @@ -116,6 +116,27 @@ describe('directive declaration jit compilation', () => { }); }); + it('should compile content queries with forwardRefs', () => { + const def = ɵɵngDeclareDirective({ + type: TestClass, + queries: [ + { + propertyName: 'byRef', + predicate: forwardRef(() => Child), + }, + ], + }) as DirectiveDef; + + class Child {} + + expectDirectiveDef(def, { + contentQueries: functionContaining([ + /contentQuery[^(]*\(dirIndex,[^,]*resolveForwardRef[^,]*forward_ref[^,]*,[\s]*4\)/, + '(ctx.byRef = _t)', + ]), + }); + }); + it('should compile view queries', () => { const def = ɵɵngDeclareDirective({ type: TestClass, @@ -152,6 +173,27 @@ describe('directive declaration jit compilation', () => { }); }); + it('should compile view queries with forwardRefs', () => { + const def = ɵɵngDeclareDirective({ + type: TestClass, + viewQueries: [ + { + propertyName: 'byRef', + predicate: forwardRef(() => Child), + }, + ], + }) as DirectiveDef; + + class Child {} + + expectDirectiveDef(def, { + viewQuery: functionContaining([ + /viewQuery[^(]*\([^,]*resolveForwardRef[^,]*forward_ref[^,]*,[\s]*4\)/, + '(ctx.byRef = _t)', + ]), + }); + }); + it('should compile host bindings', () => { const def = ɵɵngDeclareDirective({ type: TestClass,