Skip to content

Commit c685cc2

Browse files
chuckjazjasonaden
authored andcommitted
feat(compiler-cli): lower metadata useValue and data literal fields (#18905)
With this commit the compiler will "lower" expressions into exported variables for values the compiler does not need to know statically in order to be able to generate a factory. For example: ``` providers: [{provider: 'token', useValue: calculated()}] ``` produced an error as the expression `calculated()` is not supported by the compiler because `calculated` is not a [known function](https://angular.io/guide/metadata#annotationsdecorators) With this commit this is rewritten, during emit of the .js file, into something like: ``` export var ɵ0 = calculated(); ... provdiers: [{provider: 'token', useValue: ɵ0}] ``` The compiler then will now generate a reference to the exported `ɵ0` instead of failing to evaluate `calculated()`. PR Close #18905
1 parent 2f2d5f3 commit c685cc2

File tree

8 files changed

+324
-28
lines changed

8 files changed

+324
-28
lines changed

packages/compiler-cli/src/diagnostics/check_types.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,9 +166,13 @@ function diagnosticMessageToString(message: ts.DiagnosticMessageChain | string):
166166
return ts.flattenDiagnosticMessageText(message, '\n');
167167
}
168168

169+
const REWRITE_PREFIX = /^\u0275[0-9]+$/;
170+
169171
function createFactoryInfo(emitter: TypeScriptEmitter, file: GeneratedFile): FactoryInfo {
170-
const {sourceText, context} =
171-
emitter.emitStatementsAndContext(file.srcFileUrl, file.genFileUrl, file.stmts !);
172+
const {sourceText, context} = emitter.emitStatementsAndContext(
173+
file.srcFileUrl, file.genFileUrl, file.stmts !,
174+
/* preamble */ undefined, /* emitSourceMaps */ undefined,
175+
/* referenceFilter */ reference => !!(reference.name && REWRITE_PREFIX.test(reference.name)));
172176
const source = ts.createSourceFile(
173177
file.genFileUrl, sourceText, ts.ScriptTarget.Latest, /* setParentNodes */ true);
174178
return {source, context};

packages/compiler-cli/src/transformers/lower_expressions.ts

Lines changed: 93 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {CollectorOptions, MetadataCollector, MetadataValue, ModuleMetadata} from '@angular/tsc-wrapped';
9+
import {CollectorOptions, MetadataCollector, MetadataValue, ModuleMetadata, isMetadataGlobalReferenceExpression} from '@angular/tsc-wrapped';
1010
import * as ts from 'typescript';
1111

1212
export interface LoweringRequest {
@@ -181,6 +181,30 @@ function shouldLower(node: ts.Node | undefined): boolean {
181181
return true;
182182
}
183183

184+
const REWRITE_PREFIX = '\u0275';
185+
186+
function isPrimitive(value: any): boolean {
187+
return Object(value) !== value;
188+
}
189+
190+
function isRewritten(value: any): boolean {
191+
return isMetadataGlobalReferenceExpression(value) && value.name.startsWith(REWRITE_PREFIX);
192+
}
193+
194+
function isLiteralFieldNamed(node: ts.Node, names: Set<string>): boolean {
195+
if (node.parent && node.parent.kind == ts.SyntaxKind.PropertyAssignment) {
196+
const property = node.parent as ts.PropertyAssignment;
197+
if (property.parent && property.parent.kind == ts.SyntaxKind.ObjectLiteralExpression &&
198+
property.name && property.name.kind == ts.SyntaxKind.Identifier) {
199+
const propertyName = property.name as ts.Identifier;
200+
return names.has(propertyName.text);
201+
}
202+
}
203+
return false;
204+
}
205+
206+
const LOWERABLE_FIELD_NAMES = new Set(['useValue', 'useFactory', 'data']);
207+
184208
export class LowerMetadataCache implements RequestsMap {
185209
private collector: MetadataCollector;
186210
private metadataCache = new Map<string, MetadataAndLoweringRequests>();
@@ -208,19 +232,41 @@ export class LowerMetadataCache implements RequestsMap {
208232

209233
private getMetadataAndRequests(sourceFile: ts.SourceFile): MetadataAndLoweringRequests {
210234
let identNumber = 0;
211-
const freshIdent = () => '\u0275' + identNumber++;
235+
const freshIdent = () => REWRITE_PREFIX + identNumber++;
212236
const requests = new Map<number, LoweringRequest>();
237+
238+
const isExportedSymbol = (() => {
239+
let exportTable: Set<string>;
240+
return (node: ts.Node) => {
241+
if (node.kind == ts.SyntaxKind.Identifier) {
242+
const ident = node as ts.Identifier;
243+
244+
if (!exportTable) {
245+
exportTable = createExportTableFor(sourceFile);
246+
}
247+
return exportTable.has(ident.text);
248+
}
249+
return false;
250+
};
251+
})();
252+
213253
const replaceNode = (node: ts.Node) => {
214254
const name = freshIdent();
215255
requests.set(node.pos, {name, kind: node.kind, location: node.pos, end: node.end});
216256
return {__symbolic: 'reference', name};
217257
};
218258

219259
const substituteExpression = (value: MetadataValue, node: ts.Node): MetadataValue => {
220-
if ((node.kind === ts.SyntaxKind.ArrowFunction ||
221-
node.kind === ts.SyntaxKind.FunctionExpression) &&
222-
shouldLower(node)) {
223-
return replaceNode(node);
260+
if (!isPrimitive(value) && !isRewritten(value)) {
261+
if ((node.kind === ts.SyntaxKind.ArrowFunction ||
262+
node.kind === ts.SyntaxKind.FunctionExpression) &&
263+
shouldLower(node)) {
264+
return replaceNode(node);
265+
}
266+
if (isLiteralFieldNamed(node, LOWERABLE_FIELD_NAMES) && shouldLower(node) &&
267+
!isExportedSymbol(node)) {
268+
return replaceNode(node);
269+
}
224270
}
225271
return value;
226272
};
@@ -229,4 +275,44 @@ export class LowerMetadataCache implements RequestsMap {
229275

230276
return {metadata, requests};
231277
}
232-
}
278+
}
279+
280+
function createExportTableFor(sourceFile: ts.SourceFile): Set<string> {
281+
const exportTable = new Set<string>();
282+
// Lazily collect all the exports from the source file
283+
ts.forEachChild(sourceFile, function scan(node) {
284+
switch (node.kind) {
285+
case ts.SyntaxKind.ClassDeclaration:
286+
case ts.SyntaxKind.FunctionDeclaration:
287+
case ts.SyntaxKind.InterfaceDeclaration:
288+
if ((ts.getCombinedModifierFlags(node) & ts.ModifierFlags.Export) != 0) {
289+
const classDeclaration =
290+
node as(ts.ClassDeclaration | ts.FunctionDeclaration | ts.InterfaceDeclaration);
291+
const name = classDeclaration.name;
292+
if (name) exportTable.add(name.text);
293+
}
294+
break;
295+
case ts.SyntaxKind.VariableStatement:
296+
const variableStatement = node as ts.VariableStatement;
297+
for (const declaration of variableStatement.declarationList.declarations) {
298+
scan(declaration);
299+
}
300+
break;
301+
case ts.SyntaxKind.VariableDeclaration:
302+
const variableDeclaration = node as ts.VariableDeclaration;
303+
if ((ts.getCombinedModifierFlags(node) & ts.ModifierFlags.Export) != 0 &&
304+
variableDeclaration.name.kind == ts.SyntaxKind.Identifier) {
305+
const name = variableDeclaration.name as ts.Identifier;
306+
exportTable.add(name.text);
307+
}
308+
break;
309+
case ts.SyntaxKind.ExportDeclaration:
310+
const exportDeclaration = node as ts.ExportDeclaration;
311+
const {moduleSpecifier, exportClause} = exportDeclaration;
312+
if (!moduleSpecifier && exportClause) {
313+
exportClause.elements.forEach(spec => { exportTable.add(spec.name.text); });
314+
}
315+
}
316+
});
317+
return exportTable;
318+
}

packages/compiler-cli/test/diagnostics/check_types_spec.ts

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,15 @@ import * as ts from 'typescript';
1212

1313
import {TypeChecker} from '../../src/diagnostics/check_types';
1414
import {Diagnostic} from '../../src/transformers/api';
15+
import {LowerMetadataCache} from '../../src/transformers/lower_expressions';
1516

1617
function compile(
1718
rootDirs: MockData, options: AotCompilerOptions = {},
1819
tsOptions: ts.CompilerOptions = {}): Diagnostic[] {
1920
const rootDirArr = toMockFileArray(rootDirs);
2021
const scriptNames = rootDirArr.map(entry => entry.fileName).filter(isSource);
2122
const host = new MockCompilerHost(scriptNames, arrayToMockDir(rootDirArr));
22-
const aotHost = new MockAotCompilerHost(host);
23+
const aotHost = new MockAotCompilerHost(host, new LowerMetadataCache({}));
2324
const tsSettings = {...settings, ...tsOptions};
2425
const program = ts.createProgram(host.scriptNames.slice(0), tsSettings, host);
2526
const ngChecker = new TypeChecker(program, tsSettings, host, aotHost, options);
@@ -80,6 +81,12 @@ describe('ng type checker', () => {
8081
it('should accept a safe property access of a nullable field reference of a method result',
8182
() => { a('{{getMaybePerson()?.name}}'); });
8283
});
84+
85+
describe('with lowered expressions', () => {
86+
it('should not report lowered expressions as errors', () => {
87+
expectNoDiagnostics(compile([angularFiles, LOWERING_QUICKSTART]));
88+
});
89+
});
8390
});
8491

8592
function appComponentSource(template: string): string {
@@ -134,8 +141,38 @@ const QUICKSTART: MockDirectory = {
134141
}
135142
};
136143

144+
const LOWERING_QUICKSTART: MockDirectory = {
145+
quickstart: {
146+
app: {
147+
'app.component.ts': appComponentSource('<h1>Hello {{name}}</h1>'),
148+
'app.module.ts': `
149+
import { NgModule, Component } from '@angular/core';
150+
import { toString } from './utils';
151+
152+
import { AppComponent } from './app.component';
153+
154+
class Foo {}
155+
156+
@Component({
157+
template: '',
158+
providers: [
159+
{provide: 'someToken', useFactory: () => new Foo()}
160+
]
161+
})
162+
export class Bar {}
163+
164+
@NgModule({
165+
declarations: [ AppComponent, Bar ],
166+
bootstrap: [ AppComponent ]
167+
})
168+
export class AppModule { }
169+
`
170+
}
171+
}
172+
};
173+
137174
function expectNoDiagnostics(diagnostics: Diagnostic[]) {
138175
if (diagnostics && diagnostics.length) {
139176
throw new Error(diagnostics.map(d => `${d.span}: ${d.message}`).join('\n'));
140177
}
141-
}
178+
}

packages/compiler-cli/test/ngc_spec.ts

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -848,4 +848,78 @@ describe('ngc transformer command-line', () => {
848848
shouldExist('app/main.js');
849849
});
850850
});
851+
852+
describe('expression lowering', () => {
853+
const shouldExist = (fileName: string) => {
854+
if (!fs.existsSync(path.resolve(basePath, fileName))) {
855+
throw new Error(`Expected ${fileName} to be emitted (basePath: ${basePath})`);
856+
}
857+
};
858+
859+
it('should be able to lower supported expressions', () => {
860+
writeConfig(`{
861+
"extends": "./tsconfig-base.json",
862+
"files": ["module.ts"]
863+
}`);
864+
write('module.ts', `
865+
import {NgModule, InjectionToken} from '@angular/core';
866+
import {AppComponent} from './app';
867+
868+
export interface Info {
869+
route: string;
870+
data: string;
871+
}
872+
873+
export const T1 = new InjectionToken<string>('t1');
874+
export const T2 = new InjectionToken<string>('t2');
875+
export const T3 = new InjectionToken<number>('t3');
876+
export const T4 = new InjectionToken<Info[]>('t4');
877+
878+
enum SomeEnum {
879+
OK,
880+
Cancel
881+
}
882+
883+
function calculateString() {
884+
return 'someValue';
885+
}
886+
887+
const routeLikeData = [{
888+
route: '/home',
889+
data: calculateString()
890+
}];
891+
892+
@NgModule({
893+
declarations: [AppComponent],
894+
providers: [
895+
{ provide: T1, useValue: calculateString() },
896+
{ provide: T2, useFactory: () => 'someValue' },
897+
{ provide: T3, useValue: SomeEnum.OK },
898+
{ provide: T4, useValue: routeLikeData }
899+
]
900+
})
901+
export class MyModule {}
902+
`);
903+
write('app.ts', `
904+
import {Component, Inject} from '@angular/core';
905+
import * as m from './module';
906+
907+
@Component({
908+
selector: 'my-app',
909+
template: ''
910+
})
911+
export class AppComponent {
912+
constructor(
913+
@Inject(m.T1) private t1: string,
914+
@Inject(m.T2) private t2: string,
915+
@Inject(m.T3) private t3: number,
916+
@Inject(m.T4) private t4: m.Info[],
917+
) {}
918+
}
919+
`);
920+
921+
expect(mainSync(['-p', basePath], s => {})).toBe(0);
922+
shouldExist('built/module.js');
923+
});
924+
});
851925
});

0 commit comments

Comments
 (0)