Skip to content

Commit 67e0ba7

Browse files
crisbetomattrbeck
crisbeto
authored and
mattrbeck
committed
fix(compiler-cli): generic types not filled out correctly in type check block
Fixes a regression caused by the recent TCB changes where we moved the type parameter processing earlier in the pipeline and stopped properly accounting for the `TcbGenericContextBehavior`. Fixes #67704. (cherry picked from commit 9769560)
1 parent ca328ea commit 67e0ba7

File tree

8 files changed

+113
-88
lines changed

8 files changed

+113
-88
lines changed

‎packages/compiler-cli/src/ngtsc/typecheck/api/api.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ export interface TcbDirectiveMetadata {
8989
typeParameters: TcbTypeParameter[] | null;
9090
inputs: ClassPropertyMapping<TcbInputMapping>;
9191
outputs: ClassPropertyMapping;
92-
hasRequiresInlineTypeCtor: boolean;
92+
requiresInlineTypeCtor: boolean;
9393
ngTemplateGuards: TemplateGuardMeta[];
9494
hasNgTemplateContextGuard: boolean;
9595
hasNgFieldDirective: boolean;
@@ -105,6 +105,7 @@ export interface TcbDirectiveMetadata {
105105
export interface TcbComponentMetadata {
106106
ref: TcbReferenceMetadata;
107107
typeParameters: TcbTypeParameter[] | null;
108+
typeArguments: string[] | null;
108109
}
109110

110111
export interface TcbTypeCheckBlockMetadata {

‎packages/compiler-cli/src/ngtsc/typecheck/src/context.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -671,7 +671,12 @@ class InlineTcbOp implements Op {
671671
const env = new Environment(this.config, im, refEmitter, this.reflector, sf);
672672
const fnName = `_tcb_${this.ref.node.pos}`;
673673

674-
const {tcbMeta, component} = adaptTypeCheckBlockMetadata(this.ref, this.meta, env);
674+
const {tcbMeta, component} = adaptTypeCheckBlockMetadata(
675+
this.ref,
676+
this.meta,
677+
env,
678+
TcbGenericContextBehavior.CopyClassNodes,
679+
);
675680

676681
// Inline TCBs should copy any generic type parameter nodes directly, as the TCB code is
677682
// inlined into the class in a context where that will always be legal.
@@ -682,7 +687,6 @@ class InlineTcbOp implements Op {
682687
tcbMeta,
683688
this.domSchemaChecker,
684689
this.oobRecorder,
685-
TcbGenericContextBehavior.CopyClassNodes,
686690
);
687691

688692
return fn;

‎packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ export class Environment extends ReferenceEmitEnvironment {
6969
return new TcbExpr(this.typeCtors.get(key)!);
7070
}
7171

72-
if (dir.hasRequiresInlineTypeCtor) {
72+
if (dir.requiresInlineTypeCtor) {
7373
// The constructor has already been created inline, we just need to construct a reference to
7474
// it.
7575
const typeCtorExpr = `${this.referenceTcbValue(dir.ref).print()}.ngTypeCtor`;

‎packages/compiler-cli/src/ngtsc/typecheck/src/ops/scope.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -774,7 +774,7 @@ export class Scope {
774774
// The most common case is that when a directive is not generic, we use the normal
775775
// `TcbNonDirectiveTypeOp`.
776776
return new TcbNonGenericDirectiveTypeOp(this.tcb, this, node, dir);
777-
} else if (!dir.hasRequiresInlineTypeCtor || this.tcb.env.config.useInlineTypeConstructors) {
777+
} else if (!dir.requiresInlineTypeCtor || this.tcb.env.config.useInlineTypeConstructors) {
778778
// For generic directives, we use a type constructor to infer types. If a directive requires
779779
// an inline type constructor, then inlining must be available to use the
780780
// `TcbDirectiveCtorOp`. If not we, we fallback to using `any` – see below.

‎packages/compiler-cli/src/ngtsc/typecheck/src/tcb_adapter.ts‎

Lines changed: 71 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
TcbInputMapping,
1616
TcbPipeMetadata,
1717
TypeCheckableDirectiveMeta,
18+
TcbTypeParameter,
1819
} from '../api';
1920
import {Environment} from './environment';
2021
import {ImportFlags, ReferenceEmitKind, Reference} from '../../imports';
@@ -35,6 +36,7 @@ import {generateTcbTypeParameters} from './tcb_util';
3536
import {TypeParameterEmitter} from './type_parameter_emitter';
3637
import {ClassDeclaration} from '../../reflection';
3738
import ts from 'typescript';
39+
import {TcbGenericContextBehavior} from './ops/context';
3840

3941
/**
4042
* Adapts the compiler's `TypeCheckBlockMetadata` (which includes full TS AST nodes)
@@ -44,8 +46,11 @@ export function adaptTypeCheckBlockMetadata(
4446
ref: Reference<ClassDeclaration<ts.ClassDeclaration>>,
4547
meta: TypeCheckBlockMetadata,
4648
env: Environment,
49+
genericContextBehavior: TcbGenericContextBehavior,
4750
): {tcbMeta: TcbTypeCheckBlockMetadata; component: TcbComponentMetadata} {
4851
const refCache = new Map<Reference<ClassDeclaration>, TcbReferenceMetadata>();
52+
const dirCache = new Map<TypeCheckableDirectiveMeta, TcbDirectiveMetadata>();
53+
4954
const extractRef = (ref: Reference<ClassDeclaration>) => {
5055
if (refCache.has(ref)) {
5156
return refCache.get(ref)!;
@@ -55,8 +60,6 @@ export function adaptTypeCheckBlockMetadata(
5560
return result;
5661
};
5762

58-
const dirCache = new Map<TypeCheckableDirectiveMeta, TcbDirectiveMetadata>();
59-
6063
const convertDir = (dir: TypeCheckableDirectiveMeta): TcbDirectiveMetadata => {
6164
if (dirCache.has(dir)) return dirCache.get(dir)!;
6265

@@ -110,45 +113,33 @@ export function adaptTypeCheckBlockMetadata(
110113

111114
ref: extractRef(dir.ref as Reference<ClassDeclaration>),
112115
isGeneric: dir.isGeneric,
113-
114-
typeParameters: (() => {
115-
const node = dir.ref.node as ClassDeclaration<ts.ClassDeclaration>;
116-
if (!node.typeParameters) {
117-
return null;
118-
}
119-
const emitter = new TypeParameterEmitter(node.typeParameters, env.reflector);
120-
let emitted: ts.TypeParameterDeclaration[] | undefined;
121-
if (!emitter.canEmit((ref) => env.canReferenceType(ref))) {
122-
emitted = [...node.typeParameters] as ts.TypeParameterDeclaration[];
123-
} else {
124-
emitted = emitter.emit((ref) => env.referenceType(ref));
125-
}
126-
return generateTcbTypeParameters(emitted || [], env.contextFile);
127-
})(),
128-
hasRequiresInlineTypeCtor: requiresInlineTypeCtor(
116+
requiresInlineTypeCtor: requiresInlineTypeCtor(
129117
dir.ref.node as ClassDeclaration<ts.ClassDeclaration>,
130118
env.reflector,
131119
env,
132120
),
121+
...adaptGenerics(
122+
dir.ref.node as ClassDeclaration<ts.ClassDeclaration>,
123+
env,
124+
TcbGenericContextBehavior.UseEmitter,
125+
),
133126
};
134127

135128
dirCache.set(dir, tcbDir);
136129
return tcbDir;
137130
};
138131

132+
const originalBoundTarget = meta.boundTarget.target;
139133
const adaptedBoundTarget: BoundTarget<TcbDirectiveMetadata> = {
140-
target: (() => {
141-
const originalTarget = meta.boundTarget.target;
142-
return {
143-
template: originalTarget.template,
144-
host: originalTarget.host
145-
? {
146-
node: originalTarget.host.node,
147-
directives: originalTarget.host.directives.map(convertDir),
148-
}
149-
: undefined,
150-
};
151-
})(),
134+
target: {
135+
template: originalBoundTarget.template,
136+
host: originalBoundTarget.host
137+
? {
138+
node: originalBoundTarget.host.node,
139+
directives: originalBoundTarget.host.directives.map(convertDir),
140+
}
141+
: undefined,
142+
},
152143
getUsedDirectives: () => meta.boundTarget.getUsedDirectives().map(convertDir),
153144
getEagerlyUsedDirectives: () => meta.boundTarget.getEagerlyUsedDirectives().map(convertDir),
154145
getUsedPipes: () => meta.boundTarget.getUsedPipes(),
@@ -208,25 +199,59 @@ export function adaptTypeCheckBlockMetadata(
208199
isStandalone: meta.isStandalone,
209200
preserveWhitespaces: meta.preserveWhitespaces,
210201
},
211-
component: (() => {
212-
return {
213-
ref: extractRef(ref as Reference<ClassDeclaration>),
214-
typeParameters: (() => {
215-
if (!ref.node.typeParameters) return null;
216-
const emitter = new TypeParameterEmitter(ref.node.typeParameters, env.reflector);
217-
let emitted: ts.TypeParameterDeclaration[] | undefined;
218-
if (!emitter.canEmit((r) => env.canReferenceType(r))) {
219-
emitted = [...ref.node.typeParameters] as ts.TypeParameterDeclaration[];
220-
} else {
221-
emitted = emitter.emit((r) => env.referenceType(r));
222-
}
223-
return generateTcbTypeParameters(emitted || [], env.contextFile);
224-
})(),
225-
};
226-
})(),
202+
component: {
203+
ref: extractRef(ref as Reference<ClassDeclaration>),
204+
...adaptGenerics(
205+
ref.node,
206+
env,
207+
env.config.useContextGenericType
208+
? genericContextBehavior
209+
: TcbGenericContextBehavior.FallbackToAny,
210+
),
211+
},
227212
};
228213
}
229214

215+
function adaptGenerics(
216+
node: ClassDeclaration<ts.ClassDeclaration>,
217+
env: Environment,
218+
genericContextBehavior: TcbGenericContextBehavior,
219+
): {
220+
typeParameters: TcbTypeParameter[] | null;
221+
typeArguments: string[] | null;
222+
} {
223+
let typeParameters: TcbTypeParameter[] | null;
224+
let typeArguments: string[] | null;
225+
226+
if (node.typeParameters !== undefined && node.typeParameters.length > 0) {
227+
switch (genericContextBehavior) {
228+
case TcbGenericContextBehavior.UseEmitter:
229+
const emitter = new TypeParameterEmitter(node.typeParameters, env.reflector);
230+
const emittedParams = emitter.canEmit((r) => env.canReferenceType(r))
231+
? emitter.emit((typeRef) => env.referenceType(typeRef))
232+
: undefined;
233+
typeParameters = generateTcbTypeParameters(
234+
emittedParams || node.typeParameters,
235+
env.contextFile,
236+
);
237+
typeArguments = typeParameters.map((param) => param.name);
238+
break;
239+
case TcbGenericContextBehavior.CopyClassNodes:
240+
typeParameters = generateTcbTypeParameters(node.typeParameters, env.contextFile);
241+
typeArguments = typeParameters.map((param) => param.name);
242+
break;
243+
case TcbGenericContextBehavior.FallbackToAny:
244+
typeParameters = generateTcbTypeParameters(node.typeParameters, env.contextFile);
245+
typeArguments = new Array<string>(node.typeParameters.length).fill('any');
246+
break;
247+
}
248+
} else {
249+
typeParameters = typeArguments = null;
250+
}
251+
252+
return {typeParameters, typeArguments};
253+
}
254+
230255
function extractReferenceMetadata(
231256
ref: Reference<ClassDeclaration>,
232257
env: Environment,

‎packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts‎

Lines changed: 5 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,12 @@
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88

9-
import ts from 'typescript';
10-
import {TcbComponentMetadata, TcbTypeCheckBlockMetadata, TcbTypeParameter} from '../api';
9+
import {TcbComponentMetadata, TcbTypeCheckBlockMetadata} from '../api';
1110
import {DomSchemaChecker} from './dom';
1211
import {Environment} from './environment';
1312
import {OutOfBandDiagnosticRecorder} from './oob';
1413
import {createHostBindingsBlockGuard} from './host_bindings';
15-
import {Context, TcbGenericContextBehavior} from './ops/context';
14+
import {Context} from './ops/context';
1615
import {Scope} from './ops/scope';
1716
import {getStatementsBlock} from './ops/codegen';
1817

@@ -47,7 +46,6 @@ export function generateTypeCheckBlock(
4746
meta: TcbTypeCheckBlockMetadata,
4847
domSchemaChecker: DomSchemaChecker,
4948
oobRecorder: OutOfBandDiagnosticRecorder,
50-
genericContextBehavior: TcbGenericContextBehavior,
5149
): string {
5250
const tcb = new Context(
5351
env,
@@ -61,42 +59,14 @@ export function generateTypeCheckBlock(
6159
meta.preserveWhitespaces,
6260
);
6361
const ctxRawType = env.referenceTcbValue(component.ref);
64-
let typeParameters: TcbTypeParameter[] | undefined = undefined;
65-
let typeArguments: string[] | undefined = undefined;
66-
67-
if (component.typeParameters !== undefined) {
68-
if (!env.config.useContextGenericType) {
69-
genericContextBehavior = TcbGenericContextBehavior.FallbackToAny;
70-
}
71-
72-
switch (genericContextBehavior) {
73-
case TcbGenericContextBehavior.UseEmitter:
74-
// Guaranteed to emit type parameters since we checked that the class has them above.
75-
const emittedParams = component.typeParameters || [];
76-
typeParameters = emittedParams;
77-
typeArguments = typeParameters!.map((param) => param.name);
78-
break;
79-
case TcbGenericContextBehavior.CopyClassNodes:
80-
const copiedParams = component.typeParameters ? [...component.typeParameters] : [];
81-
typeParameters = copiedParams;
82-
typeArguments = typeParameters!.map((param) => param.name);
83-
break;
84-
case TcbGenericContextBehavior.FallbackToAny:
85-
typeArguments = Array.from({length: component.typeParameters?.length ?? 0}).map(
86-
() => 'any',
87-
);
88-
break;
89-
}
90-
}
62+
const {typeParameters, typeArguments} = component;
9163

9264
const typeParamsStr =
93-
typeParameters === undefined || typeParameters.length === 0
65+
!env.config.useContextGenericType || typeParameters === null || typeParameters.length === 0
9466
? ''
9567
: `<${typeParameters.map((p) => p.representation).join(', ')}>`;
9668
const typeArgsStr =
97-
typeArguments === undefined || typeArguments.length === 0
98-
? ''
99-
: `<${typeArguments.join(', ')}>`;
69+
typeArguments === null || typeArguments.length === 0 ? '' : `<${typeArguments.join(', ')}>`;
10070

10171
const thisParamStr = `this: ${ctxRawType.print()}${typeArgsStr}`;
10272
const statements: string[] = [];

‎packages/compiler-cli/src/ngtsc/typecheck/src/type_check_file.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,15 +70,19 @@ export class TypeCheckFile extends Environment {
7070
genericContextBehavior: TcbGenericContextBehavior,
7171
): void {
7272
const fnId = `_tcb${this.nextTcbId++}`;
73-
const {tcbMeta, component} = adaptTypeCheckBlockMetadata(ref, meta, this);
73+
const {tcbMeta, component} = adaptTypeCheckBlockMetadata(
74+
ref,
75+
meta,
76+
this,
77+
genericContextBehavior,
78+
);
7479
const fn = generateTypeCheckBlock(
7580
this,
7681
component,
7782
fnId,
7883
tcbMeta,
7984
domSchemaChecker,
8085
oobRecorder,
81-
genericContextBehavior,
8286
);
8387
this.tcbStatements.push(fn);
8488
}

‎packages/compiler-cli/test/ngtsc/ngtsc_spec.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10788,6 +10788,27 @@ runInEachFileSystem((os: string) => {
1078810788
expect(codes).toEqual([ngErrorCode(ErrorCode.NGMODULE_BOOTSTRAP_IS_STANDALONE)]);
1078910789
});
1079010790

10791+
it('should compile a component with a complex generic', () => {
10792+
env.write(
10793+
'test.ts',
10794+
`
10795+
import {Component} from '@angular/core';
10796+
10797+
@Component({
10798+
selector: 'app-root',
10799+
template: '',
10800+
})
10801+
export class App<
10802+
T extends object = object,
10803+
TOptions extends { [K in keyof T]?: T[K] } = object
10804+
> {}
10805+
`,
10806+
);
10807+
10808+
const diags = env.driveDiagnostics();
10809+
expect(diags.length).toBe(0);
10810+
});
10811+
1079110812
describe('InjectorDef emit optimizations for standalone', () => {
1079210813
it('should not filter components out of NgModule.imports', () => {
1079310814
env.write(

0 commit comments

Comments
 (0)