Skip to content

Commit

Permalink
fix(@ngtools/webpack): better ctor parameters in AOT (#4428)
Browse files Browse the repository at this point in the history
We missed a few cases that were generating errors. Sorry.

Fixes #4427
  • Loading branch information
hansl authored Feb 7, 2017
1 parent 6ff0f80 commit 7f25548
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 35 deletions.
77 changes: 47 additions & 30 deletions packages/@ngtools/webpack/src/loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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} }`;
}


Expand All @@ -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(', ')} ]; }`;
Expand Down
7 changes: 6 additions & 1 deletion tests/e2e/assets/webpack/test-app/app/app.component.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {Component, ViewEncapsulation} from '@angular/core';
import {MyInjectable} from './injectable';


@Component({
Expand All @@ -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);
}
}
8 changes: 8 additions & 0 deletions tests/e2e/assets/webpack/test-app/app/injectable.ts
Original file line number Diff line number Diff line change
@@ -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) {}
}
15 changes: 11 additions & 4 deletions tests/e2e/tests/packages/webpack/test.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,28 @@
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) {
return Promise.resolve()
.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());
}

0 comments on commit 7f25548

Please sign in to comment.