Skip to content

Commit

Permalink
refactor(compiler-cli): update ImportGenerator abstraction for new …
Browse files Browse the repository at this point in the history
…manager (#54983)

`ImportGenerator` is the abstraction used by the translator functions to
insert imports for `ExternalExpr` in an AST-agnostic way.

This was built specifically for the linker which does not use any of the
complex import managers- but rather re-uses `ngImport` or uses
`ngImport.Bla`.

This commit also switches the linker AST-agnostic generator to follow
the new signatures. This was rather trivial.

PR Close #54983
  • Loading branch information
devversion authored and dylhunn committed Mar 27, 2024
1 parent 3253576 commit 4d8d324
Show file tree
Hide file tree
Showing 13 changed files with 125 additions and 98 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,15 @@ export class EmitScope<TStatement, TExpression> {
*/
translateDefinition(definition: LinkedDefinition): TExpression {
const expression = this.translator.translateExpression(
definition.expression, new LinkerImportGenerator(this.ngImport));
definition.expression, new LinkerImportGenerator(this.factory, this.ngImport));

if (definition.statements.length > 0) {
// Definition statements must be emitted "after" the declaration for which the definition is
// being emitted. However, the linker only transforms individual declaration calls, and can't
// insert statements after definitions. To work around this, the linker transforms the
// definition into an IIFE which executes the definition statements before returning the
// definition expression.
const importGenerator = new LinkerImportGenerator(this.ngImport);
const importGenerator = new LinkerImportGenerator(this.factory, this.ngImport);
return this.wrapInIifeWithStatements(
expression,
definition.statements.map(
Expand All @@ -59,7 +59,7 @@ export class EmitScope<TStatement, TExpression> {
* Return any constant statements that are shared between all uses of this `EmitScope`.
*/
getConstantStatements(): TStatement[] {
const importGenerator = new LinkerImportGenerator(this.ngImport);
const importGenerator = new LinkerImportGenerator(this.factory, this.ngImport);
return this.constantPool.statements.map(
statement => this.translator.translateStatement(statement, importGenerator));
}
Expand Down
10 changes: 6 additions & 4 deletions packages/compiler-cli/linker/src/file_linker/translator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,23 @@ export class Translator<TStatement, TExpression> {
* Translate the given output AST in the context of an expression.
*/
translateExpression(
expression: o.Expression, imports: ImportGenerator<TExpression>,
expression: o.Expression, imports: ImportGenerator<null, TExpression>,
options: TranslatorOptions<TExpression> = {}): TExpression {
return expression.visitExpression(
new ExpressionTranslatorVisitor<TStatement, TExpression>(this.factory, imports, options),
new ExpressionTranslatorVisitor<null, TStatement, TExpression>(
this.factory, imports, null, options),
new Context(false));
}

/**
* Translate the given output AST in the context of a statement.
*/
translateStatement(
statement: o.Statement, imports: ImportGenerator<TExpression>,
statement: o.Statement, imports: ImportGenerator<null, TExpression>,
options: TranslatorOptions<TExpression> = {}): TStatement {
return statement.visitStatement(
new ExpressionTranslatorVisitor<TStatement, TExpression>(this.factory, imports, options),
new ExpressionTranslatorVisitor<null, TStatement, TExpression>(
this.factory, imports, null, options),
new Context(true));
}
}
22 changes: 12 additions & 10 deletions packages/compiler-cli/linker/src/linker_import_generator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {ImportGenerator, NamedImport} from '../../src/ngtsc/translator';
import {AstFactory, ImportGenerator, ImportRequest} from '../../src/ngtsc/translator';

import {FatalLinkerError} from './fatal_linker_error';

Expand All @@ -17,17 +17,19 @@ import {FatalLinkerError} from './fatal_linker_error';
* must be achieved by property access on an `ng` namespace identifier, which is passed in via the
* constructor.
*/
export class LinkerImportGenerator<TExpression> implements ImportGenerator<TExpression> {
constructor(private ngImport: TExpression) {}

generateNamespaceImport(moduleName: string): TExpression {
this.assertModuleName(moduleName);
return this.ngImport;
export class LinkerImportGenerator<TStatement, TExpression> implements
ImportGenerator<null, TExpression> {
constructor(private factory: AstFactory<TStatement, TExpression>, private ngImport: TExpression) {
}

generateNamedImport(moduleName: string, originalSymbol: string): NamedImport<TExpression> {
this.assertModuleName(moduleName);
return {moduleImport: this.ngImport, symbol: originalSymbol};
addImport(request: ImportRequest<null>): TExpression {
this.assertModuleName(request.exportModuleSpecifier);

if (request.exportSymbolName === null) {
return this.ngImport;
}

return this.factory.createPropertyAccess(this.ngImport, request.exportSymbolName);
}

private assertModuleName(moduleName: string): void {
Expand Down
28 changes: 13 additions & 15 deletions packages/compiler-cli/linker/test/file_linker/translator_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,30 @@
* found in the LICENSE file at https://angular.io/license
*/
import * as o from '@angular/compiler';
import {ImportGenerator, NamedImport, TypeScriptAstFactory} from '@angular/compiler-cli/src/ngtsc/translator';
import {TypeScriptAstFactory} from '@angular/compiler-cli/src/ngtsc/translator';
import ts from 'typescript';

import {Translator} from '../../src/file_linker/translator';
import {LinkerImportGenerator} from '../../src/linker_import_generator';

import {generate} from './helpers';

const ngImport = ts.factory.createIdentifier('ngImport');

describe('Translator', () => {
let factory: TypeScriptAstFactory;
beforeEach(() => factory = new TypeScriptAstFactory(/* annotateForClosureCompiler */ false));
let importGenerator: LinkerImportGenerator<ts.Statement, ts.Expression>;

beforeEach(() => {
factory = new TypeScriptAstFactory(/* annotateForClosureCompiler */ false);
importGenerator = new LinkerImportGenerator<ts.Statement, ts.Expression>(factory, ngImport);
});

describe('translateExpression()', () => {
it('should generate expression specific output', () => {
const translator = new Translator<ts.Statement, ts.Expression>(factory);
const outputAst = new o.WriteVarExpr('foo', new o.LiteralExpr(42));
const translated = translator.translateExpression(outputAst, new MockImportGenerator());
const translated = translator.translateExpression(outputAst, importGenerator);
expect(generate(translated)).toEqual('(foo = 42)');
});
});
Expand All @@ -29,19 +38,8 @@ describe('Translator', () => {
it('should generate statement specific output', () => {
const translator = new Translator<ts.Statement, ts.Expression>(factory);
const outputAst = new o.ExpressionStatement(new o.WriteVarExpr('foo', new o.LiteralExpr(42)));
const translated = translator.translateStatement(outputAst, new MockImportGenerator());
const translated = translator.translateStatement(outputAst, importGenerator);
expect(generate(translated)).toEqual('foo = 42;');
});
});
class MockImportGenerator implements ImportGenerator<ts.Expression> {
generateNamespaceImport(moduleName: string): ts.Expression {
return factory.createLiteral(moduleName);
}
generateNamedImport(moduleName: string, originalSymbol: string): NamedImport<ts.Expression> {
return {
moduleImport: factory.createLiteral(moduleName),
symbol: originalSymbol,
};
}
}
});
51 changes: 38 additions & 13 deletions packages/compiler-cli/linker/test/linker_import_generator_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,37 +5,62 @@
* 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 ts from 'typescript';

import {TypeScriptAstFactory} from '../../src/ngtsc/translator';
import {LinkerImportGenerator} from '../src/linker_import_generator';

const ngImport = {
ngImport: true
};
const ngImport = ts.factory.createIdentifier('ngImport');

describe('LinkerImportGenerator<TExpression>', () => {
describe('generateNamespaceImport()', () => {
it('should error if the import is not `@angular/core`', () => {
const generator = new LinkerImportGenerator(ngImport);
expect(() => generator.generateNamespaceImport('other/import'))
const generator = new LinkerImportGenerator<ts.Statement, ts.Expression>(
new TypeScriptAstFactory(false), ngImport);

expect(
() => generator.addImport(
{exportModuleSpecifier: 'other/import', exportSymbolName: null, requestedFile: null}))
.toThrowError(`Unable to import from anything other than '@angular/core'`);
});

it('should return the ngImport expression for `@angular/core`', () => {
const generator = new LinkerImportGenerator(ngImport);
expect(generator.generateNamespaceImport('@angular/core')).toBe(ngImport);
const generator = new LinkerImportGenerator<ts.Statement, ts.Expression>(
new TypeScriptAstFactory(false), ngImport);

expect(generator.addImport({
exportModuleSpecifier: '@angular/core',
exportSymbolName: null,
requestedFile: null
})).toBe(ngImport);
});
});

describe('generateNamedImport()', () => {
it('should error if the import is not `@angular/core`', () => {
const generator = new LinkerImportGenerator(ngImport);
expect(() => generator.generateNamedImport('other/import', 'someSymbol'))
.toThrowError(`Unable to import from anything other than '@angular/core'`);
const generator = new LinkerImportGenerator<ts.Statement, ts.Expression>(
new TypeScriptAstFactory(false), ngImport);

expect(() => generator.addImport({
exportModuleSpecifier: 'other/import',
exportSymbolName: 'someSymbol',
requestedFile: null,
})).toThrowError(`Unable to import from anything other than '@angular/core'`);
});

it('should return a `NamedImport` object containing the ngImport expression', () => {
const generator = new LinkerImportGenerator(ngImport);
expect(generator.generateNamedImport('@angular/core', 'someSymbol'))
.toEqual({moduleImport: ngImport, symbol: 'someSymbol'});
const generator = new LinkerImportGenerator<ts.Statement, ts.Expression>(
new TypeScriptAstFactory(false), ngImport);

const result = generator.addImport({
exportModuleSpecifier: '@angular/core',
exportSymbolName: 'someSymbol',
requestedFile: null,
});

expect(ts.isPropertyAccessExpression(result)).toBe(true);
expect((result as ts.PropertyAccessExpression).name.text).toBe('someSymbol');
expect((result as ts.PropertyAccessExpression).expression).toBe(ngImport);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ runInEachFileSystem(() => {
const sf = getSourceFileOrError(program, _('/index.ts'));
const im = new ImportManager(new NoopImportRewriter(), 'i');
const stmt = compileClassMetadata(call).toStmt();
const tsStatement = translateStatement(stmt, im);
const tsStatement = translateStatement(sf, stmt, im);
const res = ts.createPrinter().printNode(ts.EmitHint.Unspecified, tsStatement, sf);
return res.replace(/\s+/g, ' ');
}
Expand Down
12 changes: 9 additions & 3 deletions packages/compiler-cli/src/ngtsc/transform/src/transform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,14 +118,19 @@ class IvyTransformationVisitor extends Visitor {
const statements: ts.Statement[] = [];
const members = [...node.members];

// Note: Class may be already transformed by e.g. Tsickle and
// not have a direct reference to the source file.
const sourceFile = ts.getOriginalNode(node).getSourceFile();

for (const field of this.classCompilationMap.get(node)!) {
// Type-only member.
if (field.initializer === null) {
continue;
}

// Translate the initializer for the field into TS nodes.
const exprNode = translateExpression(field.initializer, this.importManager, translateOptions);
const exprNode =
translateExpression(sourceFile, field.initializer, this.importManager, translateOptions);

// Create a static property declaration for the new field.
const property = ts.factory.createPropertyDeclaration(
Expand All @@ -142,7 +147,8 @@ class IvyTransformationVisitor extends Visitor {
/* hasTrailingNewLine */ false);
}

field.statements.map(stmt => translateStatement(stmt, this.importManager, translateOptions))
field.statements
.map(stmt => translateStatement(sourceFile, stmt, this.importManager, translateOptions))
.forEach(stmt => statements.push(stmt));

members.push(property);
Expand Down Expand Up @@ -313,7 +319,7 @@ function transformIvySourceFile(
// to the ImportManager.
const downlevelTranslatedCode = getLocalizeCompileTarget(context) < ts.ScriptTarget.ES2015;
const constants =
constantPool.statements.map(stmt => translateStatement(stmt, importManager, {
constantPool.statements.map(stmt => translateStatement(file, stmt, importManager, {
recordWrappedNode,
downlevelTaggedTemplates: downlevelTranslatedCode,
downlevelVariableDeclarations: downlevelTranslatedCode,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,6 @@
* found in the LICENSE file at https://angular.io/license
*/

/**
* The symbol name and import namespace of an imported symbol,
* which has been registered through the ImportGenerator.
*/
export interface NamedImport<TExpression> {
/** The import namespace containing this imported symbol. */
moduleImport: TExpression|null;
/** The (possibly rewritten) name of the imported symbol. */
symbol: string;
}

/**
* A request to import a given symbol from the given module.
*/
Expand Down Expand Up @@ -49,7 +38,6 @@ export interface ImportRequest<TFile> {
* Implementations of these methods return a specific identifier that corresponds to the imported
* module.
*/
export interface ImportGenerator<TExpression> {
generateNamespaceImport(moduleName: string): TExpression;
generateNamedImport(moduleName: string, originalSymbol: string): NamedImport<TExpression>;
export interface ImportGenerator<TFile, TExpression> {
addImport(request: ImportRequest<TFile>): TExpression;
}
36 changes: 19 additions & 17 deletions packages/compiler-cli/src/ngtsc/translator/src/translator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,16 @@ export interface TranslatorOptions<TExpression> {
annotateForClosureCompiler?: boolean;
}

export class ExpressionTranslatorVisitor<TStatement, TExpression> implements o.ExpressionVisitor,
o.StatementVisitor {
export class ExpressionTranslatorVisitor<TFile, TStatement, TExpression> implements
o.ExpressionVisitor, o.StatementVisitor {
private downlevelTaggedTemplates: boolean;
private downlevelVariableDeclarations: boolean;
private recordWrappedNode: RecordWrappedNodeFn<TExpression>;

constructor(
private factory: AstFactory<TStatement, TExpression>,
private imports: ImportGenerator<TExpression>, options: TranslatorOptions<TExpression>) {
private imports: ImportGenerator<TFile, TExpression>, private contextFile: TFile,
options: TranslatorOptions<TExpression>) {
this.downlevelTaggedTemplates = options.downlevelTaggedTemplates === true;
this.downlevelVariableDeclarations = options.downlevelVariableDeclarations === true;
this.recordWrappedNode = options.recordWrappedNode || (() => {});
Expand Down Expand Up @@ -210,11 +211,11 @@ export class ExpressionTranslatorVisitor<TStatement, TExpression> implements o.E
private createES5TaggedTemplateFunctionCall(
tagHandler: TExpression, {elements, expressions}: TemplateLiteral<TExpression>): TExpression {
// Ensure that the `__makeTemplateObject()` helper has been imported.
const {moduleImport, symbol} =
this.imports.generateNamedImport('tslib', '__makeTemplateObject');
const __makeTemplateObjectHelper = (moduleImport === null) ?
this.factory.createIdentifier(symbol) :
this.factory.createPropertyAccess(moduleImport, symbol);
const __makeTemplateObjectHelper = this.imports.addImport({
exportModuleSpecifier: 'tslib',
exportSymbolName: '__makeTemplateObject',
requestedFile: this.contextFile,
});

// Collect up the cooked and raw strings into two separate arrays.
const cooked: TExpression[] = [];
Expand Down Expand Up @@ -244,20 +245,21 @@ export class ExpressionTranslatorVisitor<TStatement, TExpression> implements o.E
if (ast.value.moduleName === null) {
throw new Error('Invalid import without name nor moduleName');
}
return this.imports.generateNamespaceImport(ast.value.moduleName);
return this.imports.addImport({
exportModuleSpecifier: ast.value.moduleName,
exportSymbolName: null,
requestedFile: this.contextFile,
});
}
// If a moduleName is specified, this is a normal import. If there's no module name, it's a
// reference to a global/ambient symbol.
if (ast.value.moduleName !== null) {
// This is a normal import. Find the imported module.
const {moduleImport, symbol} =
this.imports.generateNamedImport(ast.value.moduleName, ast.value.name);
if (moduleImport === null) {
// The symbol was ambient after all.
return this.factory.createIdentifier(symbol);
} else {
return this.factory.createPropertyAccess(moduleImport, symbol);
}
return this.imports.addImport({
exportModuleSpecifier: ast.value.moduleName,
exportSymbolName: ast.value.name,
requestedFile: this.contextFile,
});
} else {
// The symbol is ambient, so just reference it.
return this.factory.createIdentifier(ast.value.name);
Expand Down

0 comments on commit 4d8d324

Please sign in to comment.