Skip to content

Commit

Permalink
fix(@ngtools/webpack): import factory from original declaration file
Browse files Browse the repository at this point in the history
Fix #14399
  • Loading branch information
filipesilva authored and Keen Yee Liau committed May 15, 2019
1 parent 85b2016 commit a491b09
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 24 deletions.
2 changes: 1 addition & 1 deletion packages/ngtools/webpack/src/angular_compiler_plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -928,7 +928,7 @@ export class AngularCompilerPlugin {
// Import ngfactory in loadChildren import syntax
if (this._useFactories) {
// Only transform imports to use factories with View Engine.
this._transformers.push(importFactory(msg => this._warnings.push(msg)));
this._transformers.push(importFactory(msg => this._warnings.push(msg), getTypeChecker));
}
}

Expand Down
47 changes: 36 additions & 11 deletions packages/ngtools/webpack/src/transformers/import_factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
* 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 { dirname, relative } from 'path';
import * as ts from 'typescript';
import { forwardSlashPath } from '../utils';


/**
Expand Down Expand Up @@ -51,6 +53,7 @@ import * as ts from 'typescript';

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

return ts.visitEachChild(node, visitVariableStatement, context);
Expand All @@ -95,6 +98,8 @@ function replaceImport(
node: ts.VariableDeclaration,
context: ts.TransformationContext,
emitWarning: () => void,
fileName: string,
typeChecker: ts.TypeChecker,
): ts.Node {
// This ONLY matches the original source code format below:
// loadChildren: () => import('IMPORT_STRING').then(M => M.EXPORT_NAME)
Expand Down Expand Up @@ -157,16 +162,20 @@ function replaceImport(
return node;
}

// Now that we know it's both `ɵ0` (generated by NGC) and a `import()`, start emitting a warning
// if the structure isn't as expected to help users identify unusable syntax.
const warnAndBail = () => {
emitWarning();

return node;
};

// ɵ0 = () => import('IMPORT_STRING').then(m => m.EXPORT_NAME)
if (!(
topArrowFnBody.arguments.length === 1
&& ts.isArrowFunction(topArrowFnBody.arguments[0])
)) {
// Now that we know it's both `ɵ0` (generated by NGC) and a `import()`, start emitting a warning
// if the structure isn't as expected to help users identify unusable syntax.
emitWarning();

return node;
return warnAndBail();
}

const thenArrowFn = topArrowFnBody.arguments[0] as ts.ArrowFunction;
Expand All @@ -175,20 +184,36 @@ function replaceImport(
&& ts.isPropertyAccessExpression(thenArrowFn.body)
&& ts.isIdentifier(thenArrowFn.body.name)
)) {
emitWarning();

return node;
return warnAndBail();
}

// At this point we know what are the nodes we need to replace.
const importStringLit = importCall.arguments[0] as ts.StringLiteral;
const exportNameId = thenArrowFn.body.name;
const importStringLit = importCall.arguments[0] as ts.StringLiteral;

// Try to resolve the import. It might be a reexport from somewhere and the ngfactory will only
// be present next to the original module.
const exportedSymbol = typeChecker.getSymbolAtLocation(exportNameId);
if (!exportedSymbol) {
return warnAndBail();
}
const exportedSymbolDecl = exportedSymbol.getDeclarations();
if (!exportedSymbolDecl || exportedSymbolDecl.length === 0) {
return warnAndBail();
}

// Get the relative path from the containing module to the imported module.
const relativePath = relative(dirname(fileName), exportedSymbolDecl[0].getSourceFile().fileName);

// node's `relative` call doesn't actually add `./` so we add it here.
// Also replace the 'ts' extension with just 'ngfactory'.
const newImportString = `./${forwardSlashPath(relativePath)}`.replace(/ts$/, 'ngfactory');

// The easiest way to alter them is with a simple visitor.
const replacementVisitor: ts.Visitor = (node: ts.Node) => {
if (node === importStringLit) {
// Transform the import string.
return ts.createStringLiteral(importStringLit.text + '.ngfactory');
return ts.createStringLiteral(newImportString);
} else if (node === exportNameId) {
// Transform the export name.
return ts.createIdentifier(exportNameId.text + 'NgFactory');
Expand Down
46 changes: 34 additions & 12 deletions packages/ngtools/webpack/src/transformers/import_factory_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,17 @@
* found in the LICENSE file at https://angular.io/license
*/
import { tags } from '@angular-devkit/core';
import { transformTypescript } from './ast_helpers';
import { createTypescriptContext, transformTypescript } from './ast_helpers';
import { importFactory } from './import_factory';

describe('@ngtools/webpack transformers', () => {
describe('import_factory', () => {
it('should support arrow functions', () => {
const additionalFiles: Record<string, string> = {
'lazy/lazy.module.ts': `
export const LazyModule = {};
`,
};
const input = tags.stripIndent`
const ɵ0 = () => import('./lazy/lazy.module').then(m => m.LazyModule);
const routes = [{
Expand All @@ -28,14 +33,20 @@ describe('@ngtools/webpack transformers', () => {
`;

let warningCalled = false;
const transformer = importFactory(() => warningCalled = true);
const result = transformTypescript(input, [transformer]);
const { program, compilerHost } = createTypescriptContext(input, additionalFiles, true);
const transformer = importFactory(() => warningCalled = true, () => program.getTypeChecker());
const result = transformTypescript(undefined, [transformer], program, compilerHost);

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

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

let warningCalled = false;
const transformer = importFactory(() => warningCalled = true);
const result = transformTypescript(input, [transformer]);
const { program, compilerHost } = createTypescriptContext(input, additionalFiles, true);
const transformer = importFactory(() => warningCalled = true, () => program.getTypeChecker());
const result = transformTypescript(undefined, [transformer], program, compilerHost);

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

it('should support different arg name', () => {
it('should support resolving reexports', () => {
const additionalFiles: Record<string, string> = {
'shared/index.ts': `
export * from './path/to/lazy/lazy.module';
`,
'shared/path/to/lazy/lazy.module.ts': `
export const LazyModule = {};
`,
};
const input = tags.stripIndent`
const ɵ0 = () => import('./lazy/lazy.module').then(a => a.LazyModule);
const ɵ0 = () => import('./shared').then(m => m.LazyModule);
const routes = [{
path: 'lazy',
loadChildren: ɵ0
}];
`;

// tslint:disable: max-line-length
const output = tags.stripIndent`
const ɵ0 = () => import("./lazy/lazy.module.ngfactory").then(a => a.LazyModuleNgFactory);
const ɵ0 = () => import("./shared/path/to/lazy/lazy.module.ngfactory").then(m => m.LazyModuleNgFactory);
const routes = [{
path: 'lazy',
loadChildren: ɵ0
}];
`;
// tslint:enable: max-line-length

let warningCalled = false;
const transformer = importFactory(() => warningCalled = true);
const result = transformTypescript(input, [transformer]);
const { program, compilerHost } = createTypescriptContext(input, additionalFiles, true);
const transformer = importFactory(() => { }, () => program.getTypeChecker());
const result = transformTypescript(undefined, [transformer], program, compilerHost);

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

0 comments on commit a491b09

Please sign in to comment.