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

@@ -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
+          "enableResourceInlining": ctx.attr.inline_resources,
-          # TODO(alexeagle): if someone has a legitimate use case for dynamic
-          # template loading, maybe we need to make this configurable.
-          "enableResourceInlining": True,
           "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(

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'];
+    const styles = arg['styles'] || [];
-      if (Array.isArray(styleUrls)) {
+    const styleUrls = arg['styleUrls'] || [];
-        arg['styles'] = styleUrls.map(styleUrl => loader.get(styleUrl));
+    if (!Array.isArray(styles)) throw new Error('styles should be an array');
-        delete arg.styleUrls;
+    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) => {
+  const newProperties: ts.ObjectLiteralElementLike[] = [];
-        if (!ts.isPropertyAssignment(node)) {
+  const newStyleExprs: ts.Expression[] = [];
-          // Error: unsupported
+  componentArg.properties.forEach(prop => {
-          return node;
+    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)) {
+      case 'styleUrls':
-          // computed names are not supported
+        if (!ts.isArrayLiteralExpression(prop.initializer)) {
-          return node;
+          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;
+      case 'templateUrl':
-        switch (name) {
+        if (!ts.isStringLiteral(prop.initializer) &&
-          case 'styleUrls':
+            !ts.isNoSubstitutionTemplateLiteral(prop.initializer)) {
-            if (!ts.isArrayLiteralExpression(node.initializer)) {
+          throw new Error(
-              // Error: unsupported
+              'Can only accept a string literal argument to templateUrl. ' + PRECONDITIONS_TEXT);
-              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;
         }
-      }));
+        const template = loader.get(prop.initializer.text);
-  return ts.createNodeArray<ts.Expression>([newArgument]);
+        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)]);
 }

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,
+	        otherProp: 3,
-	}) export class Foo {}`);
+	      }) 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 {}"]');
       }
     }
   });