Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(compiler-cli): resolve resource URLs before loading them under en… #22688

Closed
wants to merge 2 commits into from
Closed

fix(compiler-cli): resolve resource URLs before loading them under en… #22688

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8594afe
fix(compiler-cli): resolve resource URLs before loading them under en…
alexeagle Mar 9, 2018
629cd37
fix(compiler-cli): enableResourceInlining handles both styles and sty…
alexeagle Mar 9, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions packages/bazel/src/ng_module.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ def _ngc_tsconfig(ctx, files, srcs, **kwargs):

return dict(tsc_wrapped_tsconfig(ctx, files, srcs, **kwargs), **{
"angularCompilerOptions": {
"enableResourceInlining": ctx.attr.inline_resources,
"generateCodeForLibraries": False,
"allowEmptyCodegenFiles": True,
"enableSummariesForJit": True,
Expand Down Expand Up @@ -342,6 +343,8 @@ NG_MODULE_ATTRIBUTES = {

"type_check": attr.bool(default = True),

"inline_resources": attr.bool(default = True),

"no_i18n": attr.bool(default = False),

"compiler": attr.label(
Expand Down
196 changes: 107 additions & 89 deletions packages/compiler-cli/src/transformers/inline_resources.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,70 +12,81 @@ import {MetadataObject, MetadataValue, isClassMetadata, isMetadataImportedSymbol

import {MetadataTransformer, ValueTransform} from './metadata_cache';

export type ResourceLoader = {
const PRECONDITIONS_TEXT =
'angularCompilerOptions.enableResourceInlining requires all resources to be statically resolvable.';

/** A subset of members from AotCompilerHost */
export type ResourcesHost = {
resourceNameToFileName(resourceName: string, containingFileName: string): string | null;
loadResource(path: string): Promise<string>| string;
};

export type StaticResourceLoader = {
get(url: string | MetadataValue): string;
};

function getResourceLoader(host: ResourcesHost, containingFileName: string): StaticResourceLoader {
return {
get(url: string | MetadataValue): string{
if (typeof url !== 'string') {
throw new Error('templateUrl and stylesUrl must be string literals. ' + PRECONDITIONS_TEXT);
} const fileName = host.resourceNameToFileName(url, containingFileName);
if (fileName) {
const content = host.loadResource(fileName);
if (typeof content !== 'string') {
throw new Error('Cannot handle async resource. ' + PRECONDITIONS_TEXT);
}
return content;
} throw new Error(`Failed to resolve ${url} from ${containingFileName}. ${PRECONDITIONS_TEXT}`);
}
};
}

export class InlineResourcesMetadataTransformer implements MetadataTransformer {
constructor(private host: ResourceLoader) {}
constructor(private host: ResourcesHost) {}

start(sourceFile: ts.SourceFile): ValueTransform|undefined {
const loader = getResourceLoader(this.host, sourceFile.fileName);
return (value: MetadataValue, node: ts.Node): MetadataValue => {
if (isClassMetadata(value) && ts.isClassDeclaration(node) && value.decorators) {
value.decorators.forEach(d => {
if (isMetadataSymbolicCallExpression(d) &&
isMetadataImportedSymbolReferenceExpression(d.expression) &&
d.expression.module === '@angular/core' && d.expression.name === 'Component' &&
d.arguments) {
d.arguments = d.arguments.map(this.updateDecoratorMetadata.bind(this));
d.arguments = d.arguments.map(this.updateDecoratorMetadata.bind(this, loader));
}
});
}
return value;
};
}

inlineResource(url: MetadataValue): string|undefined {
if (typeof url === 'string') {
const content = this.host.loadResource(url);
if (typeof content === 'string') {
return content;
}
}
}

updateDecoratorMetadata(arg: MetadataObject): MetadataObject {
updateDecoratorMetadata(loader: StaticResourceLoader, arg: MetadataObject): MetadataObject {
if (arg['templateUrl']) {
const template = this.inlineResource(arg['templateUrl']);
if (template) {
arg['template'] = template;
delete arg.templateUrl;
}
arg['template'] = loader.get(arg['templateUrl']);
delete arg.templateUrl;
}
if (arg['styleUrls']) {
const styleUrls = arg['styleUrls'];
if (Array.isArray(styleUrls)) {
let allStylesInlined = true;
const newStyles = styleUrls.map(styleUrl => {
const style = this.inlineResource(styleUrl);
if (style) return style;
allStylesInlined = false;
return styleUrl;
});
if (allStylesInlined) {
arg['styles'] = newStyles;
delete arg.styleUrls;
}
}

const styles = arg['styles'] || [];
const styleUrls = arg['styleUrls'] || [];
if (!Array.isArray(styles)) throw new Error('styles should be an array');
if (!Array.isArray(styleUrls)) throw new Error('styleUrls should be an array');

styles.push(...styleUrls.map(styleUrl => loader.get(styleUrl)));
if (styles.length > 0) {
arg['styles'] = styles;
delete arg.styleUrls;
}

return arg;
}
}

export function getInlineResourcesTransformFactory(
program: ts.Program, host: ResourceLoader): ts.TransformerFactory<ts.SourceFile> {
program: ts.Program, host: ResourcesHost): ts.TransformerFactory<ts.SourceFile> {
return (context: ts.TransformationContext) => (sourceFile: ts.SourceFile) => {
const loader = getResourceLoader(host, sourceFile.fileName);
const visitor: ts.Visitor = node => {
// Components are always classes; skip any other node
if (!ts.isClassDeclaration(node)) {
Expand All @@ -86,7 +97,7 @@ export function getInlineResourcesTransformFactory(
// @Component()
const newDecorators = ts.visitNodes(node.decorators, (node: ts.Decorator) => {
if (isComponentDecorator(node, program.getTypeChecker())) {
return updateDecorator(node, host);
return updateDecorator(node, loader);
}
return node;
});
Expand All @@ -95,7 +106,7 @@ export function getInlineResourcesTransformFactory(
// static decorators: {type: Function, args?: any[]}[]
const newMembers = ts.visitNodes(
node.members,
(node: ts.ClassElement) => updateAnnotations(node, host, program.getTypeChecker()));
(node: ts.ClassElement) => updateAnnotations(node, loader, program.getTypeChecker()));

// Create a new AST subtree with our modifications
return ts.updateClassDeclaration(
Expand All @@ -110,27 +121,28 @@ export function getInlineResourcesTransformFactory(
/**
* Update a Decorator AST node to inline the resources
* @param node the @Component decorator
* @param host provides access to load resources
* @param loader provides access to load resources
*/
function updateDecorator(node: ts.Decorator, host: ResourceLoader): ts.Decorator {
function updateDecorator(node: ts.Decorator, loader: StaticResourceLoader): ts.Decorator {
if (!ts.isCallExpression(node.expression)) {
// User will get an error somewhere else with bare @Component
return node;
}
const expr = node.expression;
const newArguments = updateComponentProperties(expr.arguments, host);
const newArguments = updateComponentProperties(expr.arguments, loader);
return ts.updateDecorator(
node, ts.updateCall(expr, expr.expression, expr.typeArguments, newArguments));
}

/**
* Update an Annotations AST node to inline the resources
* @param node the static decorators property
* @param host provides access to load resources
* @param loader provides access to load resources
* @param typeChecker provides access to symbol table
*/
function updateAnnotations(
node: ts.ClassElement, host: ResourceLoader, typeChecker: ts.TypeChecker): ts.ClassElement {
node: ts.ClassElement, loader: StaticResourceLoader,
typeChecker: ts.TypeChecker): ts.ClassElement {
// Looking for a member of this shape:
// PropertyDeclaration called decorators, with static modifier
// Initializer is ArrayLiteralExpression
Expand Down Expand Up @@ -172,7 +184,7 @@ function updateAnnotations(

const newDecoratorArgs = ts.updatePropertyAssignment(
prop, prop.name,
ts.createArrayLiteral(updateComponentProperties(prop.initializer.elements, host)));
ts.createArrayLiteral(updateComponentProperties(prop.initializer.elements, loader)));

return newDecoratorArgs;
});
Expand Down Expand Up @@ -239,11 +251,11 @@ function isComponentSymbol(identifier: ts.Node, typeChecker: ts.TypeChecker) {
* For each property in the object literal, if it's templateUrl or styleUrls, replace it
* with content.
* @param node the arguments to @Component() or args property of decorators: [{type:Component}]
* @param host provides access to the loadResource method of the host
* @param loader provides access to the loadResource method of the host
* @returns updated arguments
*/
function updateComponentProperties(
args: ts.NodeArray<ts.Expression>, host: ResourceLoader): ts.NodeArray<ts.Expression> {
args: ts.NodeArray<ts.Expression>, loader: StaticResourceLoader): ts.NodeArray<ts.Expression> {
if (args.length !== 1) {
// User should have gotten a type-check error because @Component takes one argument
return args;
Expand All @@ -254,53 +266,59 @@ function updateComponentProperties(
// argument
return args;
}
const newArgument = ts.updateObjectLiteral(
componentArg, ts.visitNodes(componentArg.properties, (node: ts.ObjectLiteralElementLike) => {
if (!ts.isPropertyAssignment(node)) {
// Error: unsupported
return node;
}

if (ts.isComputedPropertyName(node.name)) {
// computed names are not supported
return node;
const newProperties: ts.ObjectLiteralElementLike[] = [];
const newStyleExprs: ts.Expression[] = [];
componentArg.properties.forEach(prop => {
if (!ts.isPropertyAssignment(prop) || ts.isComputedPropertyName(prop.name)) {
newProperties.push(prop);
return;
}

switch (prop.name.text) {
case 'styles':
if (!ts.isArrayLiteralExpression(prop.initializer)) {
throw new Error('styles takes an array argument');
}
newStyleExprs.push(...prop.initializer.elements);
break;

const name = node.name.text;
switch (name) {
case 'styleUrls':
if (!ts.isArrayLiteralExpression(node.initializer)) {
// Error: unsupported
return node;
}
const styleUrls = node.initializer.elements;

return ts.updatePropertyAssignment(
node, ts.createIdentifier('styles'),
ts.createArrayLiteral(ts.visitNodes(styleUrls, (expr: ts.Expression) => {
if (ts.isStringLiteral(expr)) {
const styles = host.loadResource(expr.text);
if (typeof styles === 'string') {
return ts.createLiteral(styles);
}
}
return expr;
})));


case 'templateUrl':
if (ts.isStringLiteral(node.initializer)) {
const template = host.loadResource(node.initializer.text);
if (typeof template === 'string') {
return ts.updatePropertyAssignment(
node, ts.createIdentifier('template'), ts.createLiteral(template));
}
}
return node;

default:
return node;
case 'styleUrls':
if (!ts.isArrayLiteralExpression(prop.initializer)) {
throw new Error('styleUrls takes an array argument');
}
}));
return ts.createNodeArray<ts.Expression>([newArgument]);
newStyleExprs.push(...prop.initializer.elements.map((expr: ts.Expression) => {
if (!ts.isStringLiteral(expr) && !ts.isNoSubstitutionTemplateLiteral(expr)) {
throw new Error(
'Can only accept string literal arguments to styleUrls. ' + PRECONDITIONS_TEXT);
}
const styles = loader.get(expr.text);
return ts.createLiteral(styles);
}));
break;

case 'templateUrl':
if (!ts.isStringLiteral(prop.initializer) &&
!ts.isNoSubstitutionTemplateLiteral(prop.initializer)) {
throw new Error(
'Can only accept a string literal argument to templateUrl. ' + PRECONDITIONS_TEXT);
}
const template = loader.get(prop.initializer.text);
newProperties.push(ts.updatePropertyAssignment(
prop, ts.createIdentifier('template'), ts.createLiteral(template)));
break;

default:
newProperties.push(prop);
}
});

// Add the non-inline styles
if (newStyleExprs.length > 0) {
const newStyles = ts.createPropertyAssignment(
ts.createIdentifier('styles'), ts.createArrayLiteral(newStyleExprs));
newProperties.push(newStyles);
}

return ts.createNodeArray([ts.updateObjectLiteral(componentArg, newProperties)]);
}
35 changes: 27 additions & 8 deletions packages/compiler-cli/test/transformers/inline_resources_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,18 @@ describe('inline resources transformer', () => {
const actual = convert(`import {Component} from '@angular/core';
@Component({
templateUrl: './thing.html',
otherProp: 3,
}) export class Foo {}`);
otherProp: 3,
}) export class Foo {}`);
expect(actual).not.toContain('templateUrl:');
expect(actual.replace(/\s+/g, ' '))
.toContain(
'Foo = __decorate([ core_1.Component({ template: "Some template", otherProp: 3, }) ], Foo)');
'Foo = __decorate([ core_1.Component({ template: "Some template", otherProp: 3 }) ], Foo)');
});
it('should allow different quotes', () => {
const actual = convert(`import {Component} from '@angular/core';
@Component({"templateUrl": \`./thing.html\`}) export class Foo {}`);
expect(actual).not.toContain('templateUrl:');
expect(actual).toContain('{ template: "Some template" }');
});
it('should replace styleUrls', () => {
const actual = convert(`import {Component} from '@angular/core';
Expand All @@ -58,11 +64,21 @@ describe('inline resources transformer', () => {
expect(actual).not.toContain('styleUrls:');
expect(actual).toContain('styles: [".some_style {}", ".some_other_style {}"]');
});
it('should preserve existing styles', () => {
const actual = convert(`import {Component} from '@angular/core';
@Component({
styles: ['h1 { color: blue }'],
styleUrls: ['./thing1.css'],
})
export class Foo {}`);
expect(actual).not.toContain('styleUrls:');
expect(actual).toContain(`styles: ['h1 { color: blue }', ".some_style {}"]`);
});
it('should handle empty styleUrls', () => {
const actual = convert(`import {Component} from '@angular/core';
@Component({styleUrls: []}) export class Foo {}`);
@Component({styleUrls: [], styles: []}) export class Foo {}`);
expect(actual).not.toContain('styleUrls:');
expect(actual).toContain('styles: []');
expect(actual).not.toContain('styles:');
});
});
describe('annotation input', () => {
Expand Down Expand Up @@ -115,14 +131,16 @@ describe('metadata transformer', () => {
@Component({
templateUrl: './thing.html',
styleUrls: ['./thing1.css', './thing2.css'],
styles: ['h1 { color: red }'],
})
export class Foo {}
`;
const sourceFile = ts.createSourceFile(
'someFile.ts', source, ts.ScriptTarget.Latest, /* setParentNodes */ true);
const cache = new MetadataCache(
new MetadataCollector(), /* strict */ true,
[new InlineResourcesMetadataTransformer({loadResource})]);
[new InlineResourcesMetadataTransformer(
{loadResource, resourceNameToFileName: (u: string) => u})]);
const metadata = cache.getMetadata(sourceFile);
expect(metadata).toBeDefined('Expected metadata from test source file');
if (metadata) {
Expand All @@ -134,7 +152,7 @@ describe('metadata transformer', () => {
expect(JSON.stringify(classData)).toContain('"template":"Some template"');
expect(JSON.stringify(classData)).not.toContain('styleUrls');
expect(JSON.stringify(classData))
.toContain('"styles":[".some_style {}",".some_other_style {}"]');
.toContain('"styles":["h1 { color: red }",".some_style {}",".some_other_style {}"]');
}
}
});
Expand Down Expand Up @@ -164,7 +182,8 @@ function convert(source: string) {
host);
const moduleSourceFile = program.getSourceFile(fileName);
const transformers: ts.CustomTransformers = {
before: [getInlineResourcesTransformFactory(program, {loadResource})]
before: [getInlineResourcesTransformFactory(
program, {loadResource, resourceNameToFileName: (u: string) => u})]
};
let result = '';
const emitResult = program.emit(
Expand Down