Skip to content

Commit 98ed5b6

Browse files
devversionpkozlowski-opensource
authored andcommitted
feat(compiler-cli): run JIT transform on classes with jit: true opt-out (angular#56892)
Currently when compiling code with the Angular compiler, all classes with Angular decorators are compiled with AOT. This includes type checking, scope collection etc. This may not be desirable for all components, e.g. dynamic components, or test components w/ `TestBed.configureTestingModule` (if compiled with ngtsc). Those components can opt out of AOT on a per component-basis via `jit: true`. This is helpful as it allows incremental migrations/refactorings to AOT. Whether we want to keep this capability long-term is something to be discussed separately. For now though, we should fix that components compiled with `jit: true` actually work as expected. Currently this **not the case** as soon as the new initializer APIs are used— as those do no longer declare class metadata with decorators. This commit runs the JIT transform on JIT-opted classes. Related: https://docs.google.com/document/d/1ox4atCJldWWDXlaYgwM-hU8BNsTpKNW7gx8OfZ0HtRY/edit?resourcekey=0-G1haTNYtD-dN0vNRkQ8_OQ&tab=t.0 PR Close angular#56892
1 parent c0ef7de commit 98ed5b6

File tree

14 files changed

+159
-16
lines changed

14 files changed

+159
-16
lines changed

packages/compiler-cli/src/ngtsc/annotations/common/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,3 +18,4 @@ export * from './src/references_registry';
1818
export * from './src/schema';
1919
export * from './src/util';
2020
export * from './src/input_transforms';
21+
export * from './src/jit_declaration_registry';
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
import {ClassDeclaration} from '../../../reflection';
10+
11+
/**
12+
* Registry that keeps track of Angular declarations that are explicitly
13+
* marked for JIT compilation and are skipping compilation by trait handlers.
14+
*/
15+
export class JitDeclarationRegistry {
16+
jitDeclarations = new Set<ClassDeclaration>();
17+
}

packages/compiler-cli/src/ngtsc/annotations/component/src/handler.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,7 @@ import {
177177
validateAndFlattenComponentImports,
178178
} from './util';
179179
import {getTemplateDiagnostics} from '../../../typecheck';
180+
import {JitDeclarationRegistry} from '../../common/src/jit_declaration_registry';
180181

181182
const EMPTY_ARRAY: any[] = [];
182183

@@ -249,6 +250,7 @@ export class ComponentDecoratorHandler
249250
private readonly enableBlockSyntax: boolean,
250251
private readonly enableLetSyntax: boolean,
251252
private readonly localCompilationExtraImportsTracker: LocalCompilationExtraImportsTracker | null,
253+
private readonly jitDeclarationRegistry: JitDeclarationRegistry,
252254
) {
253255
this.extractTemplateOptions = {
254256
enableI18nLegacyMessageIdFormat: this.enableI18nLegacyMessageIdFormat,
@@ -410,10 +412,11 @@ export class ComponentDecoratorHandler
410412
this.compilationMode,
411413
this.elementSchemaRegistry.getDefaultComponentElementName(),
412414
);
413-
if (directiveResult === undefined) {
414-
// `extractDirectiveMetadata` returns undefined when the @Directive has `jit: true`. In this
415-
// case, compilation of the decorator is skipped. Returning an empty object signifies
416-
// that no analysis was produced.
415+
// `extractDirectiveMetadata` returns `jitForced = true` when the `@Component` has
416+
// set `jit: true`. In this case, compilation of the decorator is skipped. Returning
417+
// an empty object signifies that no analysis was produced.
418+
if (directiveResult.jitForced) {
419+
this.jitDeclarationRegistry.jitDeclarations.add(node);
417420
return {};
418421
}
419422

packages/compiler-cli/src/ngtsc/annotations/component/test/component_spec.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import {getDeclaration, makeProgram} from '../../../testing';
3939
import {CompilationMode} from '../../../transform';
4040
import {
4141
InjectableClassRegistry,
42+
JitDeclarationRegistry,
4243
NoopReferencesRegistry,
4344
ResourceLoader,
4445
ResourceLoaderContext,
@@ -105,6 +106,7 @@ function setup(
105106
);
106107
const resourceLoader = new StubResourceLoader();
107108
const importTracker = new ImportedSymbolsTracker();
109+
const jitDeclarationRegistry = new JitDeclarationRegistry();
108110

109111
const handler = new ComponentDecoratorHandler(
110112
reflectionHost,
@@ -144,6 +146,7 @@ function setup(
144146
/* enableBlockSyntax */ true,
145147
/* enableLetSyntax */ true,
146148
/* localCompilationExtraImportsTracker */ null,
149+
jitDeclarationRegistry,
147150
);
148151
return {reflectionHost, handler, resourceLoader, metaRegistry};
149152
}

packages/compiler-cli/src/ngtsc/annotations/directive/src/handler.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ import {
7676

7777
import {extractDirectiveMetadata} from './shared';
7878
import {DirectiveSymbol} from './symbol';
79+
import {JitDeclarationRegistry} from '../../common/src/jit_declaration_registry';
7980

8081
const FIELD_DECORATORS = [
8182
'Input',
@@ -133,7 +134,7 @@ export class DirectiveDecoratorHandler
133134
private importTracker: ImportedSymbolsTracker,
134135
private includeClassMetadata: boolean,
135136
private readonly compilationMode: CompilationMode,
136-
private readonly generateExtraImportsInLocalMode: boolean,
137+
private readonly jitDeclarationRegistry: JitDeclarationRegistry,
137138
) {}
138139

139140
readonly precedence = HandlerPrecedence.PRIMARY;
@@ -189,9 +190,14 @@ export class DirectiveDecoratorHandler
189190
this.compilationMode,
190191
/* defaultSelector */ null,
191192
);
192-
if (directiveResult === undefined) {
193+
// `extractDirectiveMetadata` returns `jitForced = true` when the `@Directive` has
194+
// set `jit: true`. In this case, compilation of the decorator is skipped. Returning
195+
// an empty object signifies that no analysis was produced.
196+
if (directiveResult.jitForced) {
197+
this.jitDeclarationRegistry.jitDeclarations.add(node);
193198
return {};
194199
}
200+
195201
const analysis = directiveResult.metadata;
196202

197203
let providersRequiringFactory: Set<Reference<ClassDeclaration>> | null = null;

packages/compiler-cli/src/ngtsc/annotations/directive/src/shared.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ export function extractDirectiveMetadata(
117117
defaultSelector: string | null,
118118
):
119119
| {
120+
jitForced: false;
120121
decorator: Map<string, ts.Expression>;
121122
metadata: R3DirectiveMetadata;
122123
inputs: ClassPropertyMapping<InputMapping>;
@@ -125,7 +126,7 @@ export function extractDirectiveMetadata(
125126
hostDirectives: HostDirectiveMeta[] | null;
126127
rawHostDirectives: ts.Expression | null;
127128
}
128-
| undefined {
129+
| {jitForced: true} {
129130
let directive: Map<string, ts.Expression>;
130131
if (decorator.args === null || decorator.args.length === 0) {
131132
directive = new Map<string, ts.Expression>();
@@ -149,7 +150,7 @@ export function extractDirectiveMetadata(
149150

150151
if (directive.has('jit')) {
151152
// The only allowed value is true, so there's no need to expand further.
152-
return undefined;
153+
return {jitForced: true};
153154
}
154155

155156
const members = reflector.getMembersOfClass(clazz);
@@ -407,6 +408,7 @@ export function extractDirectiveMetadata(
407408
null,
408409
};
409410
return {
411+
jitForced: false,
410412
decorator: directive,
411413
metadata,
412414
inputs,

packages/compiler-cli/src/ngtsc/annotations/directive/test/directive_spec.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,11 @@ import {
2929
import {LocalModuleScopeRegistry, MetadataDtsModuleScopeResolver} from '../../../scope';
3030
import {getDeclaration, makeProgram} from '../../../testing';
3131
import {CompilationMode} from '../../../transform';
32-
import {InjectableClassRegistry, NoopReferencesRegistry} from '../../common';
32+
import {
33+
InjectableClassRegistry,
34+
JitDeclarationRegistry,
35+
NoopReferencesRegistry,
36+
} from '../../common';
3337
import {DirectiveDecoratorHandler} from '../index';
3438

3539
runInEachFileSystem(() => {
@@ -190,6 +194,8 @@ runInEachFileSystem(() => {
190194
);
191195
const injectableRegistry = new InjectableClassRegistry(reflectionHost, /* isCore */ false);
192196
const importTracker = new ImportedSymbolsTracker();
197+
const jitDeclarationRegistry = new JitDeclarationRegistry();
198+
193199
const handler = new DirectiveDecoratorHandler(
194200
reflectionHost,
195201
evaluator,
@@ -207,7 +213,7 @@ runInEachFileSystem(() => {
207213
importTracker,
208214
/*includeClassMetadata*/ true,
209215
/*compilationMode */ CompilationMode.FULL,
210-
/*generateExtraImportsInLocalMode*/ false,
216+
jitDeclarationRegistry,
211217
);
212218

213219
const DirNode = getDeclaration(program, _('/entry.ts'), dirName, isNamedClassDeclaration);

packages/compiler-cli/src/ngtsc/annotations/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ export {
1616
ReferencesRegistry,
1717
ResourceLoader,
1818
ResourceLoaderContext,
19+
JitDeclarationRegistry,
1920
} from './common';
2021
export {ComponentDecoratorHandler} from './component';
2122
export {

packages/compiler-cli/src/ngtsc/core/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ ts_library(
4444
"//packages/compiler-cli/src/ngtsc/util",
4545
"//packages/compiler-cli/src/ngtsc/validation",
4646
"//packages/compiler-cli/src/ngtsc/xi18n",
47+
"//packages/compiler-cli/src/transformers/jit_transforms",
4748
"@npm//@types/semver",
4849
"@npm//semver",
4950
"@npm//typescript",

packages/compiler-cli/src/ngtsc/core/src/compiler.ts

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import {
1818
PipeDecoratorHandler,
1919
ReferencesRegistry,
2020
} from '../../annotations';
21-
import {InjectableClassRegistry} from '../../annotations/common';
21+
import {InjectableClassRegistry, JitDeclarationRegistry} from '../../annotations/common';
2222
import {CycleAnalyzer, CycleHandlingStrategy, ImportGraph} from '../../cycles';
2323
import {
2424
COMPILER_ERRORS_WITH_GUIDES,
@@ -124,6 +124,7 @@ import {DiagnosticCategoryLabel, NgCompilerAdapter, NgCompilerOptions} from '../
124124

125125
import {coreHasSymbol} from './core_version';
126126
import {coreVersionSupportsFeature} from './feature_detection';
127+
import {angularJitApplicationTransform} from '@angular/compiler-cli/src/transformers/jit_transforms';
127128

128129
/**
129130
* State information about a compilation which is only generated once some data is requested from
@@ -145,6 +146,8 @@ interface LazyCompilationState {
145146
extendedTemplateChecker: ExtendedTemplateChecker | null;
146147
templateSemanticsChecker: TemplateSemanticsChecker | null;
147148
sourceFileValidator: SourceFileValidator | null;
149+
jitDeclarationRegistry: JitDeclarationRegistry;
150+
supportJitMode: boolean;
148151

149152
/**
150153
* Only available in local compilation mode when option `generateExtraImportsInLocalMode` is set.
@@ -814,6 +817,39 @@ export class NgCompiler {
814817
defaultImportTracker.importPreservingTransformer(),
815818
];
816819

820+
// If there are JIT declarations, wire up the JIT transform and efficiently
821+
// run it against the target declarations.
822+
if (compilation.supportJitMode && compilation.jitDeclarationRegistry.jitDeclarations.size > 0) {
823+
const {jitDeclarations} = compilation.jitDeclarationRegistry;
824+
const jitDeclarationsArray = Array.from(jitDeclarations);
825+
const jitDeclarationOriginalNodes = new Set(
826+
jitDeclarationsArray.map((d) => ts.getOriginalNode(d)),
827+
);
828+
const sourceFilesWithJit = new Set(
829+
jitDeclarationsArray.map((d) => d.getSourceFile().fileName),
830+
);
831+
832+
before.push((ctx) => {
833+
const reflectionHost = new TypeScriptReflectionHost(this.inputProgram.getTypeChecker());
834+
const jitTransform = angularJitApplicationTransform(
835+
this.inputProgram,
836+
compilation.isCore,
837+
(node) => {
838+
// Class may be synthetic at this point due to Ivy transform.
839+
node = ts.getOriginalNode(node, ts.isClassDeclaration);
840+
return reflectionHost.isClass(node) && jitDeclarationOriginalNodes.has(node);
841+
},
842+
)(ctx);
843+
844+
return (sourceFile) => {
845+
if (!sourceFilesWithJit.has(sourceFile.fileName)) {
846+
return sourceFile;
847+
}
848+
return jitTransform(sourceFile);
849+
};
850+
});
851+
}
852+
817853
const afterDeclarations: ts.TransformerFactory<ts.SourceFile>[] = [];
818854

819855
// In local compilation mode we don't make use of .d.ts files for Angular compilation, so their
@@ -1361,6 +1397,8 @@ export class NgCompiler {
13611397
);
13621398
}
13631399

1400+
const jitDeclarationRegistry = new JitDeclarationRegistry();
1401+
13641402
// Set up the IvyCompilation, which manages state for the Ivy transformer.
13651403
const handlers: DecoratorHandler<unknown, unknown, SemanticSymbol | null, unknown>[] = [
13661404
new ComponentDecoratorHandler(
@@ -1401,6 +1439,7 @@ export class NgCompiler {
14011439
this.enableBlockSyntax,
14021440
this.enableLetSyntax,
14031441
localCompilationExtraImportsTracker,
1442+
jitDeclarationRegistry,
14041443
),
14051444

14061445
// TODO(alxhub): understand why the cast here is necessary (something to do with `null`
@@ -1422,7 +1461,7 @@ export class NgCompiler {
14221461
importTracker,
14231462
supportTestBed,
14241463
compilationMode,
1425-
!!this.options.generateExtraImportsInLocalMode,
1464+
jitDeclarationRegistry,
14261465
) as Readonly<DecoratorHandler<unknown, unknown, SemanticSymbol | null, unknown>>,
14271466
// Pipe handler must be before injectable handler in list so pipe factories are printed
14281467
// before injectable factories (so injectable factories can delegate to them)
@@ -1545,8 +1584,10 @@ export class NgCompiler {
15451584
resourceRegistry,
15461585
extendedTemplateChecker,
15471586
localCompilationExtraImportsTracker,
1587+
jitDeclarationRegistry,
15481588
templateSemanticsChecker,
15491589
sourceFileValidator,
1590+
supportJitMode,
15501591
};
15511592
}
15521593
}

0 commit comments

Comments
 (0)