Skip to content

Commit 50106ff

Browse files
clydindgp1130
authored andcommitted
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 c2a449b)
1 parent efd5724 commit 50106ff

File tree

4 files changed

+54
-3
lines changed

4 files changed

+54
-3
lines changed

packages/ngtools/webpack/src/ivy/transformation.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ export function createAotTransformers(
2525
const removeNgModuleScope = !options.emitNgModuleScope;
2626
if (removeClassMetadata || removeNgModuleScope) {
2727
// tslint:disable-next-line: no-non-null-assertion
28-
transformers.after!.push(
28+
transformers.before!.push(
2929
removeIvyJitSupportCalls(removeClassMetadata, removeNgModuleScope, getTypeChecker),
3030
);
3131
}

packages/ngtools/webpack/src/transformers/elide_imports.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,11 @@ export function elideImports(
118118
}
119119

120120
const isUnused = (node: ts.Identifier) => {
121+
// Do not remove JSX factory imports
122+
if (node.text === compilerOptions.jsxFactory) {
123+
return false;
124+
}
125+
121126
const symbol = typeChecker.getSymbolAtLocation(node);
122127

123128
return symbol && !usedSymbols.has(symbol);

packages/ngtools/webpack/src/transformers/elide_imports_spec.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,9 @@ describe('@ngtools/webpack transformers', () => {
5050
[propName: string]: unknown;
5151
}
5252
`,
53+
'jsx.ts': `
54+
export function createElement() {}
55+
`,
5356
};
5457

5558
it('should remove unused imports', () => {
@@ -348,6 +351,47 @@ describe('@ngtools/webpack transformers', () => {
348351
});
349352
});
350353

354+
it('keeps jsxFactory imports when configured', () => {
355+
const extraCompilerOptions: ts.CompilerOptions = {
356+
jsxFactory: 'createElement',
357+
experimentalDecorators: true,
358+
jsx: ts.JsxEmit.React,
359+
};
360+
361+
const input = tags.stripIndent`
362+
import { Decorator } from './decorator';
363+
import { Service } from './service';
364+
import { createElement } from './jsx';
365+
366+
const test = <p>123</p>;
367+
368+
@Decorator()
369+
export class Foo {
370+
constructor(param: Service) {
371+
}
372+
}
373+
374+
${dummyNode}
375+
`;
376+
377+
const output = tags.stripIndent`
378+
import { __decorate } from "tslib";
379+
import { Decorator } from './decorator';
380+
import { createElement } from './jsx';
381+
382+
const test = createElement("p", null, "123");
383+
384+
let Foo = class Foo { constructor(param) { } };
385+
Foo = __decorate([ Decorator() ], Foo);
386+
export { Foo };
387+
`;
388+
389+
const { program, compilerHost } = createTypescriptContext(input, additionalFiles, true, extraCompilerOptions, true);
390+
const result = transformTypescript(undefined, [transformer(program)], program, compilerHost);
391+
392+
expect(tags.oneLine`${result}`).toEqual(tags.oneLine`${output}`);
393+
});
394+
351395
describe('should not elide imports decorator type references when emitDecoratorMetadata is true', () => {
352396
const extraCompilerOptions: ts.CompilerOptions = {
353397
emitDecoratorMetadata: true,

packages/ngtools/webpack/src/transformers/spec_helpers.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import { WebpackCompilerHost } from '../compiler_host';
1414

1515
// Test transform helpers.
1616
const basePath = '/project/src/';
17-
const fileName = basePath + 'test-file.ts';
17+
const basefileName = basePath + 'test-file.ts';
1818
const typeScriptLibFiles = loadTypeScriptLibFiles();
1919
const tsLibFiles = loadTsLibFiles();
2020

@@ -23,7 +23,9 @@ export function createTypescriptContext(
2323
additionalFiles?: Record<string, string>,
2424
useLibs = false,
2525
extraCompilerOptions: ts.CompilerOptions = {},
26+
jsxFile = false,
2627
) {
28+
const fileName = basefileName + (jsxFile ? 'x' : '');
2729
// Set compiler options.
2830
const compilerOptions: ts.CompilerOptions = {
2931
noEmitOnError: useLibs,
@@ -107,7 +109,7 @@ export function transformTypescript(
107109
}
108110

109111
// Return the transpiled js.
110-
return compilerHost.readFile(fileName.replace(/\.tsx?$/, '.js'));
112+
return compilerHost.readFile(basefileName.replace(/\.tsx?$/, '.js'));
111113
}
112114

113115
function loadTypeScriptLibFiles(): Record<string, string> {

0 commit comments

Comments
 (0)