From 40315bef3dd0120c9e0242343f32d54eb2519c49 Mon Sep 17 00:00:00 2001 From: Alex Eagle Date: Fri, 9 Mar 2018 15:27:05 -0800 Subject: [PATCH] fix(compiler-cli): enableResourceInlining handles both styles and styleUrls (#22688) When both are present, the inlined styles are appended to the end of the styles PR Close #22688 --- packages/bazel/src/ng_module.bzl | 7 +- .../src/transformers/inline_resources.ts | 106 ++++++++++-------- .../transformers/inline_resources_spec.ts | 29 ++++- 3 files changed, 86 insertions(+), 56 deletions(-) diff --git a/packages/bazel/src/ng_module.bzl b/packages/bazel/src/ng_module.bzl index e12157e9a2..2125c5b9ab 100644 --- a/packages/bazel/src/ng_module.bzl +++ b/packages/bazel/src/ng_module.bzl @@ -88,10 +88,7 @@ def _ngc_tsconfig(ctx, files, srcs, **kwargs): return dict(tsc_wrapped_tsconfig(ctx, files, srcs, **kwargs), **{ "angularCompilerOptions": { - # Always assume that resources can be loaded statically at build time - # TODO(alexeagle): if someone has a legitimate use case for dynamic - # template loading, maybe we need to make this configurable. - "enableResourceInlining": True, + "enableResourceInlining": ctx.attr.inline_resources, "generateCodeForLibraries": False, "allowEmptyCodegenFiles": True, "enableSummariesForJit": True, @@ -346,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( diff --git a/packages/compiler-cli/src/transformers/inline_resources.ts b/packages/compiler-cli/src/transformers/inline_resources.ts index 2f53e9b9cc..cf2a4c401c 100644 --- a/packages/compiler-cli/src/transformers/inline_resources.ts +++ b/packages/compiler-cli/src/transformers/inline_resources.ts @@ -67,12 +67,16 @@ export class InlineResourcesMetadataTransformer implements MetadataTransformer { arg['template'] = loader.get(arg['templateUrl']); delete arg.templateUrl; } - if (arg['styleUrls']) { - const styleUrls = arg['styleUrls']; - if (Array.isArray(styleUrls)) { - arg['styles'] = styleUrls.map(styleUrl => loader.get(styleUrl)); - 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; @@ -262,49 +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; + + 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; - if (ts.isComputedPropertyName(node.name)) { - // computed names are not supported - return node; + case 'styleUrls': + if (!ts.isArrayLiteralExpression(prop.initializer)) { + throw new Error('styleUrls takes an array argument'); } + 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; - 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 = loader.get(expr.text); - return ts.createLiteral(styles); - } - return expr; - }))); - - - case 'templateUrl': - if (ts.isStringLiteral(node.initializer)) { - const template = loader.get(node.initializer.text); - return ts.updatePropertyAssignment( - node, ts.createIdentifier('template'), ts.createLiteral(template)); - } - return node; - - default: - return node; + 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); } - })); - return ts.createNodeArray([newArgument]); + 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)]); } diff --git a/packages/compiler-cli/test/transformers/inline_resources_spec.ts b/packages/compiler-cli/test/transformers/inline_resources_spec.ts index 156fa529e7..290dbb4929 100644 --- a/packages/compiler-cli/test/transformers/inline_resources_spec.ts +++ b/packages/compiler-cli/test/transformers/inline_resources_spec.ts @@ -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'; @@ -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', () => { @@ -115,6 +131,7 @@ describe('metadata transformer', () => { @Component({ templateUrl: './thing.html', styleUrls: ['./thing1.css', './thing2.css'], + styles: ['h1 { color: red }'], }) export class Foo {} `; @@ -135,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 {}"]'); } } });