Skip to content

Commit

Permalink
fix(@ngtools/webpack): replace resources should effect only class dec…
Browse files Browse the repository at this point in the history
…orators (#12503)

At the moment we are processing all property assignments in object literals irrespective if they are in a decorator or not. With this change we will process only property assignments found under in a component decorator.

Fixes #12488, Fixes #6007, Fixes: #6498 and Fixes: #8295
  • Loading branch information
alan-agius4 authored and vikerman committed Nov 8, 2018
1 parent bd4dc38 commit 96606b3
Show file tree
Hide file tree
Showing 6 changed files with 512 additions and 127 deletions.
6 changes: 2 additions & 4 deletions packages/ngtools/webpack/src/angular_compiler_plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -817,7 +817,7 @@ export class AngularCompilerPlugin {

if (this._JitMode) {
// Replace resources in JIT.
this._transformers.push(replaceResources(isAppPath));
this._transformers.push(replaceResources(isAppPath, getTypeChecker));
} else {
// Remove unneeded angular decorators.
this._transformers.push(removeDecorators(isAppPath, getTypeChecker));
Expand Down Expand Up @@ -1013,9 +1013,7 @@ export class AngularCompilerPlugin {
.filter(x => x);

const resourceImports = findResources(sourceFile)
.map((resourceReplacement) => resourceReplacement.resourcePaths)
.reduce((prev, curr) => prev.concat(curr), [])
.map((resourcePath) => resolve(dirname(resolvedFileName), normalize(resourcePath)));
.map(resourcePath => resolve(dirname(resolvedFileName), normalize(resourcePath)));

// These paths are meant to be used by the loader so we must denormalize them.
const uniqueDependencies = new Set([
Expand Down
68 changes: 68 additions & 0 deletions packages/ngtools/webpack/src/transformers/find_resources.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/**
* @license
* Copyright Google Inc. All Rights Reserved.
*
* 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 * as ts from 'typescript';
import { collectDeepNodes } from './ast_helpers';
import { getResourceUrl } from './replace_resources';

export function findResources(sourceFile: ts.SourceFile): string[] {
const resources: string[] = [];
const decorators = collectDeepNodes<ts.Decorator>(sourceFile, ts.SyntaxKind.Decorator);

for (const node of decorators) {
if (!ts.isCallExpression(node.expression)) {
continue;
}

const decoratorFactory = node.expression;
const args = decoratorFactory.arguments;
if (args.length !== 1 || !ts.isObjectLiteralExpression(args[0])) {
// Unsupported component metadata
continue;
}

ts.visitNodes(
(args[0] as ts.ObjectLiteralExpression).properties,
(node: ts.ObjectLiteralElementLike) => {
if (!ts.isPropertyAssignment(node) || ts.isComputedPropertyName(node.name)) {
return node;
}

const name = node.name.text;
switch (name) {
case 'templateUrl':
const url = getResourceUrl(node.initializer);

if (url) {
resources.push(url);
}
break;

case 'styleUrls':
if (!ts.isArrayLiteralExpression(node.initializer)) {
return node;
}

ts.visitNodes(node.initializer.elements, (node: ts.Expression) => {
const url = getResourceUrl(node);

if (url) {
resources.push(url);
}

return node;
});
break;
}

return node;
},
);
}

return resources;
}
49 changes: 49 additions & 0 deletions packages/ngtools/webpack/src/transformers/find_resources_spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/**
* @license
* Copyright Google Inc. All Rights Reserved.
*
* 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 { tags } from '@angular-devkit/core'; // tslint:disable-line:no-implicit-dependencies
import * as ts from 'typescript';
import { findResources } from './find_resources';


describe('@ngtools/webpack transformers', () => {
describe('find_resources', () => {
it('should return resources', () => {
const input = tags.stripIndent`
import { Component } from '@angular/core';
@Component({
selector: 'app-root',
templateUrl: './app.component.html',
styleUrls: ['./app.component.css', './app.component.2.css']
})
export class AppComponent {
title = 'app';
}
`;

const result = findResources(ts.createSourceFile('temp.ts', input, ts.ScriptTarget.ES2015));
expect(result).toEqual([
'./app.component.html',
'./app.component.css',
'./app.component.2.css',
]);
});

it('should not return resources if they are not in decorator', () => {
const input = tags.stripIndent`
const foo = {
templateUrl: './app.component.html',
styleUrls: ['./app.component.css', './app.component.2.css']
}
`;

const result = findResources(ts.createSourceFile('temp.ts', input, ts.ScriptTarget.ES2015));
expect(result).toEqual([]);
});
});
});
1 change: 1 addition & 0 deletions packages/ngtools/webpack/src/transformers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,4 @@ export * from './export_lazy_module_map';
export * from './register_locale_data';
export * from './replace_resources';
export * from './remove_decorators';
export * from './find_resources';
Loading

0 comments on commit 96606b3

Please sign in to comment.