From 7f25548085076d0c3d041c728a67d1aa19777102 Mon Sep 17 00:00:00 2001 From: Hans Date: Mon, 6 Feb 2017 19:39:58 -0800 Subject: [PATCH] fix(@ngtools/webpack): better ctor parameters in AOT (#4428) We missed a few cases that were generating errors. Sorry. Fixes #4427 --- packages/@ngtools/webpack/src/loader.ts | 77 +++++++++++-------- .../webpack/test-app/app/app.component.ts | 7 +- .../assets/webpack/test-app/app/injectable.ts | 8 ++ tests/e2e/tests/packages/webpack/test.ts | 15 +++- 4 files changed, 72 insertions(+), 35 deletions(-) create mode 100644 tests/e2e/assets/webpack/test-app/app/injectable.ts diff --git a/packages/@ngtools/webpack/src/loader.ts b/packages/@ngtools/webpack/src/loader.ts index 400bbedf2595..a995a341d4ac 100644 --- a/packages/@ngtools/webpack/src/loader.ts +++ b/packages/@ngtools/webpack/src/loader.ts @@ -61,35 +61,57 @@ function _angularImportsFromNode(node: ts.ImportDeclaration, sourceFile: ts.Sour function _ctorParameterFromTypeReference(paramNode: ts.ParameterDeclaration, angularImports: string[], refactor: TypeScriptFileRefactor) { - if (paramNode.type.kind == ts.SyntaxKind.TypeReference) { - const type = paramNode.type as ts.TypeReferenceNode; - const decorators = refactor.findAstNodes(paramNode, ts.SyntaxKind.Decorator) as ts.Decorator[]; - const decoratorStr = decorators - .map(decorator => { - const fnName = - (refactor.findFirstAstNode(decorator, ts.SyntaxKind.CallExpression) as ts.CallExpression) - .expression.getText(refactor.sourceFile); - - if (angularImports.indexOf(fnName) === -1) { - return null; + let typeName = 'undefined'; + + if (paramNode.type) { + switch (paramNode.type.kind) { + case ts.SyntaxKind.TypeReference: + const type = paramNode.type as ts.TypeReferenceNode; + if (type.typeName) { + typeName = type.typeName.getText(refactor.sourceFile); } else { - return fnName; + typeName = type.getText(refactor.sourceFile); } - }) - .filter(x => !!x) - .map(name => `{ type: ${name} }`) - .join(', '); - - if (type.typeName.kind == ts.SyntaxKind.Identifier) { - const typeName = type.typeName as ts.Identifier; - if (decorators.length > 0) { - return `{ type: ${typeName.text}, decorators: [${decoratorStr}] }`; - } - return `{ type: ${typeName.text} }`; + break; + case ts.SyntaxKind.AnyKeyword: + typeName = 'undefined'; + break; + default: + typeName = 'null'; } } - return 'null'; + const decorators = refactor.findAstNodes(paramNode, ts.SyntaxKind.Decorator) as ts.Decorator[]; + const decoratorStr = decorators + .map(decorator => { + const call = + refactor.findFirstAstNode(decorator, ts.SyntaxKind.CallExpression) as ts.CallExpression; + + if (!call) { + return null; + } + + const fnName = call.expression.getText(refactor.sourceFile); + const args = call.arguments.map(x => x.getText(refactor.sourceFile)).join(', '); + if (angularImports.indexOf(fnName) === -1) { + return null; + } else { + return [fnName, args]; + } + }) + .filter(x => !!x) + .map(([name, args]: string[]) => { + if (args) { + return `{ type: ${name}, args: [${args}] }`; + } + return `{ type: ${name} }`; + }) + .join(', '); + + if (decorators.length > 0) { + return `{ type: ${typeName}, decorators: [${decoratorStr}] }`; + } + return `{ type: ${typeName} }`; } @@ -106,12 +128,7 @@ function _addCtorParameters(classNode: ts.ClassDeclaration, } const params = Array.from(ctor.parameters).map(paramNode => { - switch (paramNode.type.kind) { - case ts.SyntaxKind.TypeReference: - return _ctorParameterFromTypeReference(paramNode, angularImports, refactor); - default: - return 'null'; - } + return _ctorParameterFromTypeReference(paramNode, angularImports, refactor); }); const ctorParametersDecl = `static ctorParameters() { return [ ${params.join(', ')} ]; }`; diff --git a/tests/e2e/assets/webpack/test-app/app/app.component.ts b/tests/e2e/assets/webpack/test-app/app/app.component.ts index 09a19ad8f1ac..82a4059565d3 100644 --- a/tests/e2e/assets/webpack/test-app/app/app.component.ts +++ b/tests/e2e/assets/webpack/test-app/app/app.component.ts @@ -1,4 +1,5 @@ import {Component, ViewEncapsulation} from '@angular/core'; +import {MyInjectable} from './injectable'; @Component({ @@ -7,4 +8,8 @@ import {Component, ViewEncapsulation} from '@angular/core'; styleUrls: ['./app.component.scss'], encapsulation: ViewEncapsulation.None }) -export class AppComponent { } +export class AppComponent { + constructor(public inj: MyInjectable) { + console.log(inj); + } +} diff --git a/tests/e2e/assets/webpack/test-app/app/injectable.ts b/tests/e2e/assets/webpack/test-app/app/injectable.ts new file mode 100644 index 000000000000..04d8486586c4 --- /dev/null +++ b/tests/e2e/assets/webpack/test-app/app/injectable.ts @@ -0,0 +1,8 @@ +import {Injectable, Inject, ViewContainerRef} from '@angular/core'; +import {DOCUMENT} from '@angular/platform-browser'; + + +@Injectable() +export class MyInjectable { + constructor(public viewContainer: ViewContainerRef, @Inject(DOCUMENT) public doc) {} +} diff --git a/tests/e2e/tests/packages/webpack/test.ts b/tests/e2e/tests/packages/webpack/test.ts index ff844d957913..d59139d2620d 100644 --- a/tests/e2e/tests/packages/webpack/test.ts +++ b/tests/e2e/tests/packages/webpack/test.ts @@ -1,7 +1,7 @@ import {normalize} from 'path'; import {createProjectFromAsset} from '../../../utils/assets'; import {exec} from '../../../utils/process'; -import {expectFileSizeToBeUnder, replaceInFile} from '../../../utils/fs'; +import {expectFileSizeToBeUnder, replaceInFile, expectFileToMatch} from '../../../utils/fs'; export default function(skipCleaning: () => void) { @@ -9,13 +9,20 @@ export default function(skipCleaning: () => void) { .then(() => createProjectFromAsset('webpack/test-app')) .then(() => exec(normalize('node_modules/.bin/webpack'), '-p')) .then(() => expectFileSizeToBeUnder('dist/app.main.js', 420000)) - .then(() => expectFileSizeToBeUnder('dist/0.app.main.js', 40000)) + .then(() => expectFileSizeToBeUnder('dist/0.app.main.js', 10000)) // test resource urls without ./ .then(() => replaceInFile('app/app.component.ts', './app.component.html', 'app.component.html')) .then(() => replaceInFile('app/app.component.ts', './app.component.scss', 'app.component.scss')) - .then(() => exec(normalize('node_modules/.bin/webpack'), '-p')) - // test + // test the inclusion of metadata + // This build also test resource URLs without ./ + .then(() => exec(normalize('node_modules/.bin/webpack'))) + .then(() => expectFileToMatch('dist/app.main.js', + new RegExp('MyInjectable.ctorParameters = .*' + + 'type: .*ViewContainerRef.*' + + 'type: undefined, decorators.*Inject.*args: .*DOCUMENT.*')) + .then(() => expectFileToMatch('dist/app.main.js', + new RegExp('AppComponent.ctorParameters = .*MyInjectable')) .then(() => skipCleaning()); }