From 50106ff256c776bd1d0d8621b4b07ed7ba347c6e Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Thu, 21 Jan 2021 13:17:46 -0500 Subject: [PATCH] fix(@ngtools/webpack): perform import eliding before TypeScript transforms Due to the method used by TypeScript to emit decorator metadata via the `emitDecoratorMetadata` function, the import eliding algorithm within the Angular compiler plugin may errantly remove imports to type metadata included in the emitted decorator calls. By moving the eliding before TypeScript, the eliding can use the Type nodes to analyze for used imports when `emitDecoratorMetadata` is enabled. This fix also reworks the previous fix to prevent the eliding to errantly remove certain factory functions that TypeScript may use in emitted code but are not yet referenced in the input code. The previous fix was to move the eliding after the TypeScript transformations. The new fix is therefore not as comprehensive as the original but covers the usecase within the originating issue (#13297). (cherry picked from commit c2a449b685be72737d50e4730101a784425b50fb) --- .../ngtools/webpack/src/ivy/transformation.ts | 2 +- .../webpack/src/transformers/elide_imports.ts | 5 +++ .../src/transformers/elide_imports_spec.ts | 44 +++++++++++++++++++ .../webpack/src/transformers/spec_helpers.ts | 6 ++- 4 files changed, 54 insertions(+), 3 deletions(-) diff --git a/packages/ngtools/webpack/src/ivy/transformation.ts b/packages/ngtools/webpack/src/ivy/transformation.ts index a76f3ccd0d0d..b635d5413e55 100644 --- a/packages/ngtools/webpack/src/ivy/transformation.ts +++ b/packages/ngtools/webpack/src/ivy/transformation.ts @@ -25,7 +25,7 @@ export function createAotTransformers( const removeNgModuleScope = !options.emitNgModuleScope; if (removeClassMetadata || removeNgModuleScope) { // tslint:disable-next-line: no-non-null-assertion - transformers.after!.push( + transformers.before!.push( removeIvyJitSupportCalls(removeClassMetadata, removeNgModuleScope, getTypeChecker), ); } diff --git a/packages/ngtools/webpack/src/transformers/elide_imports.ts b/packages/ngtools/webpack/src/transformers/elide_imports.ts index ea3e3cff67c9..5a1daa6292a9 100644 --- a/packages/ngtools/webpack/src/transformers/elide_imports.ts +++ b/packages/ngtools/webpack/src/transformers/elide_imports.ts @@ -118,6 +118,11 @@ export function elideImports( } const isUnused = (node: ts.Identifier) => { + // Do not remove JSX factory imports + if (node.text === compilerOptions.jsxFactory) { + return false; + } + const symbol = typeChecker.getSymbolAtLocation(node); return symbol && !usedSymbols.has(symbol); diff --git a/packages/ngtools/webpack/src/transformers/elide_imports_spec.ts b/packages/ngtools/webpack/src/transformers/elide_imports_spec.ts index a324a9bd0c49..997e60ec8af6 100644 --- a/packages/ngtools/webpack/src/transformers/elide_imports_spec.ts +++ b/packages/ngtools/webpack/src/transformers/elide_imports_spec.ts @@ -50,6 +50,9 @@ describe('@ngtools/webpack transformers', () => { [propName: string]: unknown; } `, + 'jsx.ts': ` + export function createElement() {} + `, }; it('should remove unused imports', () => { @@ -348,6 +351,47 @@ describe('@ngtools/webpack transformers', () => { }); }); + it('keeps jsxFactory imports when configured', () => { + const extraCompilerOptions: ts.CompilerOptions = { + jsxFactory: 'createElement', + experimentalDecorators: true, + jsx: ts.JsxEmit.React, + }; + + const input = tags.stripIndent` + import { Decorator } from './decorator'; + import { Service } from './service'; + import { createElement } from './jsx'; + + const test =

123

; + + @Decorator() + export class Foo { + constructor(param: Service) { + } + } + + ${dummyNode} + `; + + const output = tags.stripIndent` + import { __decorate } from "tslib"; + import { Decorator } from './decorator'; + import { createElement } from './jsx'; + + const test = createElement("p", null, "123"); + + let Foo = class Foo { constructor(param) { } }; + Foo = __decorate([ Decorator() ], Foo); + export { Foo }; + `; + + const { program, compilerHost } = createTypescriptContext(input, additionalFiles, true, extraCompilerOptions, true); + const result = transformTypescript(undefined, [transformer(program)], program, compilerHost); + + expect(tags.oneLine`${result}`).toEqual(tags.oneLine`${output}`); + }); + describe('should not elide imports decorator type references when emitDecoratorMetadata is true', () => { const extraCompilerOptions: ts.CompilerOptions = { emitDecoratorMetadata: true, diff --git a/packages/ngtools/webpack/src/transformers/spec_helpers.ts b/packages/ngtools/webpack/src/transformers/spec_helpers.ts index c2c3ba06c852..e4e7ca04f3b4 100644 --- a/packages/ngtools/webpack/src/transformers/spec_helpers.ts +++ b/packages/ngtools/webpack/src/transformers/spec_helpers.ts @@ -14,7 +14,7 @@ import { WebpackCompilerHost } from '../compiler_host'; // Test transform helpers. const basePath = '/project/src/'; -const fileName = basePath + 'test-file.ts'; +const basefileName = basePath + 'test-file.ts'; const typeScriptLibFiles = loadTypeScriptLibFiles(); const tsLibFiles = loadTsLibFiles(); @@ -23,7 +23,9 @@ export function createTypescriptContext( additionalFiles?: Record, useLibs = false, extraCompilerOptions: ts.CompilerOptions = {}, + jsxFile = false, ) { + const fileName = basefileName + (jsxFile ? 'x' : ''); // Set compiler options. const compilerOptions: ts.CompilerOptions = { noEmitOnError: useLibs, @@ -107,7 +109,7 @@ export function transformTypescript( } // Return the transpiled js. - return compilerHost.readFile(fileName.replace(/\.tsx?$/, '.js')); + return compilerHost.readFile(basefileName.replace(/\.tsx?$/, '.js')); } function loadTypeScriptLibFiles(): Record {