From 4a6c6505d92473727d0c2a5138564e67ae04ed91 Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Fri, 11 Oct 2024 16:39:13 -0400 Subject: [PATCH] refactor(compiler): support generating URL expressions with dynamic imports (#58173) The compiler's AST factories now support generating a dynamic import call expression with either a string literal or an expression. The later is useful for cases where the URL is dynamically created at runtime. Also, a leading comment can now be added to the URL for cases where bundler behavior needs to be included via special comments. PR Close #58173 --- .../linker/babel/src/ast/babel_ast_factory.ts | 10 ++++++--- .../babel/test/ast/babel_ast_factory_spec.ts | 20 +++++++++++++++++- .../ngtsc/translator/src/api/ast_factory.ts | 4 ++-- .../src/ngtsc/translator/src/translator.ts | 10 ++++++++- .../translator/src/typescript_ast_factory.ts | 11 ++++++---- .../test/typescript_ast_factory_spec.ts | 21 ++++++++++++++++++- packages/compiler/src/output/output_ast.ts | 11 +++++++--- 7 files changed, 72 insertions(+), 15 deletions(-) diff --git a/packages/compiler-cli/linker/babel/src/ast/babel_ast_factory.ts b/packages/compiler-cli/linker/babel/src/ast/babel_ast_factory.ts index 290a897f56e..500fff9cb4b 100644 --- a/packages/compiler-cli/linker/babel/src/ast/babel_ast_factory.ts +++ b/packages/compiler-cli/linker/babel/src/ast/babel_ast_factory.ts @@ -27,7 +27,7 @@ export class BabelAstFactory implements AstFactory { private sourceUrl: string, ) {} - attachComments(statement: t.Statement, leadingComments: LeadingComment[]): void { + attachComments(statement: t.Statement | t.Expression, leadingComments: LeadingComment[]): void { // We must process the comments in reverse because `t.addComment()` will add new ones in front. for (let i = leadingComments.length - 1; i >= 0; i--) { const comment = leadingComments[i]; @@ -119,8 +119,12 @@ export class BabelAstFactory implements AstFactory { createIfStatement = t.ifStatement; - createDynamicImport(url: string): t.Expression { - return this.createCallExpression(t.import(), [t.stringLiteral(url)], false /* pure */); + createDynamicImport(url: string | t.Expression): t.Expression { + return this.createCallExpression( + t.import(), + [typeof url === 'string' ? t.stringLiteral(url) : url], + false /* pure */, + ); } createLiteral(value: string | number | boolean | null | undefined): t.Expression { diff --git a/packages/compiler-cli/linker/babel/test/ast/babel_ast_factory_spec.ts b/packages/compiler-cli/linker/babel/test/ast/babel_ast_factory_spec.ts index 0b1d8a2d589..1ae7db4a6da 100644 --- a/packages/compiler-cli/linker/babel/test/ast/babel_ast_factory_spec.ts +++ b/packages/compiler-cli/linker/babel/test/ast/babel_ast_factory_spec.ts @@ -34,6 +34,18 @@ describe('BabelAstFactory', () => { expect(generate(stmt).code).toEqual(['/* comment 1 */', '//comment 2', 'x = 10;'].join('\n')); }); + + it('should add the comments to the given statement', () => { + const expr = expression.ast`x + 10`; + factory.attachComments(expr, [ + leadingComment('comment 1', true), + leadingComment('comment 2', false), + ]); + + expect(generate(expr).code).toEqual( + ['(', '/* comment 1 */', '//comment 2', 'x + 10'].join('\n') + ')', + ); + }); }); describe('createArrayLiteral()', () => { @@ -187,11 +199,17 @@ describe('BabelAstFactory', () => { }); describe('createDynamicImport()', () => { - it('should create a dynamic import', () => { + it('should create a dynamic import with a string URL', () => { const url = './some/path'; const dynamicImport = factory.createDynamicImport(url); expect(generate(dynamicImport).code).toEqual(`import("${url}")`); }); + + it('should create a dynamic import with an expression URL', () => { + const url = expression.ast`'/' + 'abc' + '/'`; + const dynamicImport = factory.createDynamicImport(url); + expect(generate(dynamicImport).code).toEqual(`import('/' + 'abc' + '/')`); + }); }); describe('createIfStatement()', () => { diff --git a/packages/compiler-cli/src/ngtsc/translator/src/api/ast_factory.ts b/packages/compiler-cli/src/ngtsc/translator/src/api/ast_factory.ts index d0ee7c9b1ab..5593c11e6db 100644 --- a/packages/compiler-cli/src/ngtsc/translator/src/api/ast_factory.ts +++ b/packages/compiler-cli/src/ngtsc/translator/src/api/ast_factory.ts @@ -20,7 +20,7 @@ export interface AstFactory { * @param statement the statement where the comments are to be attached. * @param leadingComments the comments to attach. */ - attachComments(statement: TStatement, leadingComments: LeadingComment[]): void; + attachComments(statement: TStatement | TExpression, leadingComments: LeadingComment[]): void; /** * Create a literal array expression (e.g. `[expr1, expr2]`). @@ -136,7 +136,7 @@ export interface AstFactory { * * @param url the URL that should by used in the dynamic import */ - createDynamicImport(url: string): TExpression; + createDynamicImport(url: string | TExpression): TExpression; /** * Create an identifier. diff --git a/packages/compiler-cli/src/ngtsc/translator/src/translator.ts b/packages/compiler-cli/src/ngtsc/translator/src/translator.ts index 07b7c6c1439..108fb252dce 100644 --- a/packages/compiler-cli/src/ngtsc/translator/src/translator.ts +++ b/packages/compiler-cli/src/ngtsc/translator/src/translator.ts @@ -356,7 +356,15 @@ export class ExpressionTranslatorVisitor } visitDynamicImportExpr(ast: o.DynamicImportExpr, context: any) { - return this.factory.createDynamicImport(ast.url); + const urlExpression = + typeof ast.url === 'string' + ? this.factory.createLiteral(ast.url) + : ast.url.visitExpression(this, context); + if (ast.urlComment) { + this.factory.attachComments(urlExpression, [o.leadingComment(ast.urlComment, true)]); + } + + return this.factory.createDynamicImport(urlExpression); } visitNotExpr(ast: o.NotExpr, context: Context): TExpression { diff --git a/packages/compiler-cli/src/ngtsc/translator/src/typescript_ast_factory.ts b/packages/compiler-cli/src/ngtsc/translator/src/typescript_ast_factory.ts index 7663bf836e6..a6781c3b0d0 100644 --- a/packages/compiler-cli/src/ngtsc/translator/src/typescript_ast_factory.ts +++ b/packages/compiler-cli/src/ngtsc/translator/src/typescript_ast_factory.ts @@ -126,11 +126,11 @@ export class TypeScriptAstFactory implements AstFactory { expect(generate(stmt)).toEqual(['/* comment 1 */', '//comment 2', 'x = 10;'].join('\n')); }); + + it('should add the comments to the given expression', () => { + const { + items: [expr], + generate, + } = setupExpressions('x + 10'); + factory.attachComments(expr, [ + leadingComment('comment 1', true), + leadingComment('comment 2', false), + ]); + + expect(generate(expr)).toEqual(['/* comment 1 */', '//comment 2', 'x + 10'].join('\n')); + }); }); describe('createArrayLiteral()', () => { @@ -63,12 +76,18 @@ describe('TypeScriptAstFactory', () => { }); describe('createDynamicImport()', () => { - it('should create a dynamic import expression', () => { + it('should create a dynamic import expression from a string URL', () => { const {generate} = setupExpressions(``); const url = './some/path'; const assignment = factory.createDynamicImport(url); expect(generate(assignment)).toEqual(`import("${url}")`); }); + + it('should create a dynamic import expression from an expression URL', () => { + const {items, generate} = setupExpressions(`'/' + 'abc' + '/'`); + const assignment = factory.createDynamicImport(items[0]); + expect(generate(assignment)).toEqual(`import('/' + 'abc' + '/')`); + }); }); describe('createBlock()', () => { diff --git a/packages/compiler/src/output/output_ast.ts b/packages/compiler/src/output/output_ast.ts index 0d0b95679ef..af8945baac8 100644 --- a/packages/compiler/src/output/output_ast.ts +++ b/packages/compiler/src/output/output_ast.ts @@ -957,14 +957,15 @@ export class ConditionalExpr extends Expression { export class DynamicImportExpr extends Expression { constructor( - public url: string, + public url: string | Expression, sourceSpan?: ParseSourceSpan | null, + public urlComment?: string, ) { super(null, sourceSpan); } override isEquivalent(e: Expression): boolean { - return e instanceof DynamicImportExpr && this.url === e.url; + return e instanceof DynamicImportExpr && this.url === e.url && this.urlComment === e.urlComment; } override isConstant() { @@ -976,7 +977,11 @@ export class DynamicImportExpr extends Expression { } override clone(): DynamicImportExpr { - return new DynamicImportExpr(this.url, this.sourceSpan); + return new DynamicImportExpr( + typeof this.url === 'string' ? this.url : this.url.clone(), + this.sourceSpan, + this.urlComment, + ); } }