Skip to content

Commit 7163dcb

Browse files
filipesilvaKeen Yee Liau
authored andcommitted
fix(@ngtools/webpack): import factory from original declaration file
Fix #14399
1 parent 4ff1640 commit 7163dcb

File tree

3 files changed

+71
-24
lines changed

3 files changed

+71
-24
lines changed

packages/ngtools/webpack/src/angular_compiler_plugin.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -928,7 +928,7 @@ export class AngularCompilerPlugin {
928928
// Import ngfactory in loadChildren import syntax
929929
if (this._useFactories) {
930930
// Only transform imports to use factories with View Engine.
931-
this._transformers.push(importFactory(msg => this._warnings.push(msg)));
931+
this._transformers.push(importFactory(msg => this._warnings.push(msg), getTypeChecker));
932932
}
933933
}
934934

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

Lines changed: 36 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@
55
* Use of this source code is governed by an MIT-style license that can be
66
* found in the LICENSE file at https://angular.io/license
77
*/
8+
import { dirname, relative } from 'path';
89
import * as ts from 'typescript';
10+
import { forwardSlashPath } from '../utils';
911

1012

1113
/**
@@ -51,6 +53,7 @@ import * as ts from 'typescript';
5153

5254
export function importFactory(
5355
warningCb: (warning: string) => void,
56+
getTypeChecker: () => ts.TypeChecker,
5457
): ts.TransformerFactory<ts.SourceFile> {
5558
return (context: ts.TransformationContext) => {
5659
// TODO(filipesilva): change the link to https://angular.io/guide/ivy once it is out.
@@ -69,7 +72,7 @@ Visit https://next.angular.io/guide/ivy for more information on using Ivy.
6972
const emitWarning = () => warningCb(warning);
7073
const visitVariableStatement: ts.Visitor = (node: ts.Node) => {
7174
if (ts.isVariableDeclaration(node)) {
72-
return replaceImport(node, context, emitWarning);
75+
return replaceImport(node, context, emitWarning, sourceFile.fileName, getTypeChecker());
7376
}
7477

7578
return ts.visitEachChild(node, visitVariableStatement, context);
@@ -95,6 +98,8 @@ function replaceImport(
9598
node: ts.VariableDeclaration,
9699
context: ts.TransformationContext,
97100
emitWarning: () => void,
101+
fileName: string,
102+
typeChecker: ts.TypeChecker,
98103
): ts.Node {
99104
// This ONLY matches the original source code format below:
100105
// loadChildren: () => import('IMPORT_STRING').then(M => M.EXPORT_NAME)
@@ -157,16 +162,20 @@ function replaceImport(
157162
return node;
158163
}
159164

165+
// Now that we know it's both `ɵ0` (generated by NGC) and a `import()`, start emitting a warning
166+
// if the structure isn't as expected to help users identify unusable syntax.
167+
const warnAndBail = () => {
168+
emitWarning();
169+
170+
return node;
171+
};
172+
160173
// ɵ0 = () => import('IMPORT_STRING').then(m => m.EXPORT_NAME)
161174
if (!(
162175
topArrowFnBody.arguments.length === 1
163176
&& ts.isArrowFunction(topArrowFnBody.arguments[0])
164177
)) {
165-
// Now that we know it's both `ɵ0` (generated by NGC) and a `import()`, start emitting a warning
166-
// if the structure isn't as expected to help users identify unusable syntax.
167-
emitWarning();
168-
169-
return node;
178+
return warnAndBail();
170179
}
171180

172181
const thenArrowFn = topArrowFnBody.arguments[0] as ts.ArrowFunction;
@@ -175,20 +184,36 @@ function replaceImport(
175184
&& ts.isPropertyAccessExpression(thenArrowFn.body)
176185
&& ts.isIdentifier(thenArrowFn.body.name)
177186
)) {
178-
emitWarning();
179-
180-
return node;
187+
return warnAndBail();
181188
}
182189

183190
// At this point we know what are the nodes we need to replace.
184-
const importStringLit = importCall.arguments[0] as ts.StringLiteral;
185191
const exportNameId = thenArrowFn.body.name;
192+
const importStringLit = importCall.arguments[0] as ts.StringLiteral;
193+
194+
// Try to resolve the import. It might be a reexport from somewhere and the ngfactory will only
195+
// be present next to the original module.
196+
const exportedSymbol = typeChecker.getSymbolAtLocation(exportNameId);
197+
if (!exportedSymbol) {
198+
return warnAndBail();
199+
}
200+
const exportedSymbolDecl = exportedSymbol.getDeclarations();
201+
if (!exportedSymbolDecl || exportedSymbolDecl.length === 0) {
202+
return warnAndBail();
203+
}
204+
205+
// Get the relative path from the containing module to the imported module.
206+
const relativePath = relative(dirname(fileName), exportedSymbolDecl[0].getSourceFile().fileName);
207+
208+
// node's `relative` call doesn't actually add `./` so we add it here.
209+
// Also replace the 'ts' extension with just 'ngfactory'.
210+
const newImportString = `./${forwardSlashPath(relativePath)}`.replace(/ts$/, 'ngfactory');
186211

187212
// The easiest way to alter them is with a simple visitor.
188213
const replacementVisitor: ts.Visitor = (node: ts.Node) => {
189214
if (node === importStringLit) {
190215
// Transform the import string.
191-
return ts.createStringLiteral(importStringLit.text + '.ngfactory');
216+
return ts.createStringLiteral(newImportString);
192217
} else if (node === exportNameId) {
193218
// Transform the export name.
194219
return ts.createIdentifier(exportNameId.text + 'NgFactory');

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

Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,17 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88
import { tags } from '@angular-devkit/core';
9-
import { transformTypescript } from './ast_helpers';
9+
import { createTypescriptContext, transformTypescript } from './ast_helpers';
1010
import { importFactory } from './import_factory';
1111

1212
describe('@ngtools/webpack transformers', () => {
1313
describe('import_factory', () => {
1414
it('should support arrow functions', () => {
15+
const additionalFiles: Record<string, string> = {
16+
'lazy/lazy.module.ts': `
17+
export const LazyModule = {};
18+
`,
19+
};
1520
const input = tags.stripIndent`
1621
const ɵ0 = () => import('./lazy/lazy.module').then(m => m.LazyModule);
1722
const routes = [{
@@ -28,14 +33,20 @@ describe('@ngtools/webpack transformers', () => {
2833
`;
2934

3035
let warningCalled = false;
31-
const transformer = importFactory(() => warningCalled = true);
32-
const result = transformTypescript(input, [transformer]);
36+
const { program, compilerHost } = createTypescriptContext(input, additionalFiles, true);
37+
const transformer = importFactory(() => warningCalled = true, () => program.getTypeChecker());
38+
const result = transformTypescript(undefined, [transformer], program, compilerHost);
3339

3440
expect(tags.oneLine`${result}`).toEqual(tags.oneLine`${output}`);
3541
expect(warningCalled).toBeFalsy();
3642
});
3743

3844
it('should not transform if the format is different than expected', () => {
45+
const additionalFiles: Record<string, string> = {
46+
'lazy/lazy.module.ts': `
47+
export const LazyModule = {};
48+
`,
49+
};
3950
const input = tags.stripIndent`
4051
const ɵ0 = () => import('./lazy/lazy.module').then(function (m) { return m.LazyModule; });
4152
const routes = [{
@@ -45,35 +56,46 @@ describe('@ngtools/webpack transformers', () => {
4556
`;
4657

4758
let warningCalled = false;
48-
const transformer = importFactory(() => warningCalled = true);
49-
const result = transformTypescript(input, [transformer]);
59+
const { program, compilerHost } = createTypescriptContext(input, additionalFiles, true);
60+
const transformer = importFactory(() => warningCalled = true, () => program.getTypeChecker());
61+
const result = transformTypescript(undefined, [transformer], program, compilerHost);
5062

5163
expect(tags.oneLine`${result}`).toEqual(tags.oneLine`${input}`);
5264
expect(warningCalled).toBeTruthy();
5365
});
5466

55-
it('should support different arg name', () => {
67+
it('should support resolving reexports', () => {
68+
const additionalFiles: Record<string, string> = {
69+
'shared/index.ts': `
70+
export * from './path/to/lazy/lazy.module';
71+
`,
72+
'shared/path/to/lazy/lazy.module.ts': `
73+
export const LazyModule = {};
74+
`,
75+
};
5676
const input = tags.stripIndent`
57-
const ɵ0 = () => import('./lazy/lazy.module').then(a => a.LazyModule);
77+
const ɵ0 = () => import('./shared').then(m => m.LazyModule);
5878
const routes = [{
5979
path: 'lazy',
6080
loadChildren: ɵ0
6181
}];
6282
`;
83+
84+
// tslint:disable: max-line-length
6385
const output = tags.stripIndent`
64-
const ɵ0 = () => import("./lazy/lazy.module.ngfactory").then(a => a.LazyModuleNgFactory);
86+
const ɵ0 = () => import("./shared/path/to/lazy/lazy.module.ngfactory").then(m => m.LazyModuleNgFactory);
6587
const routes = [{
6688
path: 'lazy',
6789
loadChildren: ɵ0
6890
}];
6991
`;
92+
// tslint:enable: max-line-length
7093

71-
let warningCalled = false;
72-
const transformer = importFactory(() => warningCalled = true);
73-
const result = transformTypescript(input, [transformer]);
94+
const { program, compilerHost } = createTypescriptContext(input, additionalFiles, true);
95+
const transformer = importFactory(() => { }, () => program.getTypeChecker());
96+
const result = transformTypescript(undefined, [transformer], program, compilerHost);
7497

7598
expect(tags.oneLine`${result}`).toEqual(tags.oneLine`${output}`);
76-
expect(warningCalled).toBeFalsy();
7799
});
78100
});
79101
});

0 commit comments

Comments
 (0)