Transform templateUrl, styleUrls and styles everywhere (#211)

* Fix transformer to handle templateUrl everywhere

templateUrl can also occur e. g. in tests, outside of decorators. These
should also be transformed.

* Update readme - preprocessor => transformer

* Make comments match new implementation

* Update comments and docs

README.md

@@ -81,11 +81,9 @@ import './jestGlobalMocks'; // browser mocks globally available for every test
 * `"setupTestFrameworkScriptFile"` – this is the heart of our config, in this file we'll setup and patch environment within tests are running
 * `"transformIgnorePatterns"` – unfortunately some modules (like @ngrx ) are released as TypeScript files, not pure JavaScript; in such cases we cannot ignore them (all node_modules are ignored by default), so they can be transformed through TS compiler like any other module in our project.
 
-## [Preprocessor](https://github.com/thymikee/jest-preset-angular/blob/master/preprocessor.js)
+## [AST Transformer](https://github.com/thymikee/jest-preset-angular/blob/master/src/InlineHtmlStripStylesTransformer.ts)
 Jest doesn't run in browser nor through dev server. It uses jsdom to abstract browser environment. So we have to cheat a little and inline our templates and get rid of styles (we're not testing CSS) because otherwise Angular will try to make XHR call for our templates and fail miserably.
 
-I used a scrappy regex to accomplish this with minimum effort, but you can also write a babel plugin to make it bulletproof. And btw, don't bother about perf here – Jest heavily caches transforms. That's why you need to run Jest with `--no-cache` flag every time you change it.
-
 ## Angular testing environment setup
 
 If you look at your `src/test.ts` (or similar bootstrapping test file) file you'll see similarities to [`setupJest.js`](https://github.com/thymikee/jest-preset-angular/blob/master/setupJest.js). What we're doing here is we're adding globals required by Angular. With [jest-zone-patch](https://github.com/thymikee/jest-zone-patch) we also make sure Jest test methods run in Zone context. Then we initialize the Angular testing environment like normal.

__tests__/InlineHtmlStripStylesTransformer.test.ts

@@ -88,6 +88,37 @@ const CODE_WITH_CUSTOM_DECORATOR = `
   }
 `
 
+const CODE_TEST_WITH_TEMPLATE_URL_OVERRIDE = `
+import { async, ComponentFixture, TestBed } from '@angular/core/testing';
+
+import { AComponent } from './a.component';
+
+describe('AComponent', () => {
+  let fixture: ComponentFixture<AComponent>,
+  instance: AComponent;
+
+  beforeEach(async(() => {
+    TestBed.configureTestingModule({
+      declarations: [
+        AComponent,
+      ],
+    }).overrideComponent(AComponent, {
+      set: {
+        templateUrl: '../__mocks__/alert-follow-stub.component.html',
+      },
+    });
+
+    fixture = TestBed.createComponent(AComponent);
+    instance = fixture.componentInstance;
+    fixture.detectChanges();
+  }));
+
+  it('should render the component', () => {
+    expect(fixture).toMatchSnapshot();
+  });
+});
+`
+
 
 
 const createFactory = () => {
@@ -138,4 +169,10 @@ describe('inlining template and stripping styles', () => {
 
     expect(out.outputText).toMatchSnapshot()
   })
+
+  it('should handle templateUrl in test file outside decorator', () => {
+    const out = transpile(CODE_TEST_WITH_TEMPLATE_URL_OVERRIDE)
+
+    expect(out.outputText).toMatchSnapshot()
+  })
 })

__tests__/__snapshots__/InlineHtmlStripStylesTransformer.test.ts.snap

@@ -57,6 +57,34 @@ exports.AngularComponent = AngularComponent;
 "
 `;
 
+exports[`inlining template and stripping styles should handle templateUrl in test file outside decorator 1`] = `
+"\\"use strict\\";
+Object.defineProperty(exports, \\"__esModule\\", { value: true });
+var testing_1 = require(\\"@angular/core/testing\\");
+var a_component_1 = require(\\"./a.component\\");
+describe('AComponent', function () {
+    var fixture, instance;
+    beforeEach(testing_1.async(function () {
+        testing_1.TestBed.configureTestingModule({
+            declarations: [
+                a_component_1.AComponent,
+            ],
+        }).overrideComponent(a_component_1.AComponent, {
+            set: {
+                template: require('../__mocks__/alert-follow-stub.component.html'),
+            },
+        });
+        fixture = testing_1.TestBed.createComponent(a_component_1.AComponent);
+        instance = fixture.componentInstance;
+        fixture.detectChanges();
+    }));
+    it('should render the component', function () {
+        expect(fixture).toMatchSnapshot();
+    });
+});
+"
+`;
+
 exports[`inlining template and stripping styles should inline non-relative templateUrl assignment and make it relative 1`] = `
 "\\"use strict\\";
 var __decorate = (this && this.__decorate) || function (decorators, target, key, desc) {

package.json

@@ -12,6 +12,7 @@
     "ts-jest": "~23.10.0"
   },
   "devDependencies": {
+    "@types/node": "^10.12.12",
     "jest": "^23.6.0",
     "typescript": "^3.2.1"
   },

src/InlineHtmlStripStylesTransformer.ts

@@ -4,59 +4,30 @@
  * 
  */
 
-
-
 /* 
  * IMPLEMENTATION DETAILS:
- * This transformer handles two concerns: removing styles and inlining referenced templates,
- * as they both are handled at the same location in the AST.
+ * This transformer handles two concerns: removing styles and inlining referenced templates.
  *
- * The Component can be located anywhere in a file, except inside another Angular Component.
- * The Decorator is not checked for the name 'Component', as someone might import it under
- * a different name, or have their custom, modified version of the component decorator.
- * Instead it checks for specific properties inside any class decorator.
+ * The assignments can be located anywhere in a file.
  * Caveats:
- * All properties 'templateUrl', 'styles', 'styleUrls' inside ANY decorator will be modified.
- * If the decorator content is referenced, it will not work:
- * ```ts
- * const componentArgs = {}
- * @Component(componentArgs)
- * class MyComponent { }
- * ```
- * 
- * The AST has to look like this:
- * 
- * ClassDeclaration
- *   Decorator
- *     CallExpression
- *       ObjectLiteralExpression
- *         PropertyAssignment
- *           Identifier
- *           StringLiteral
- * 
- * Where some additional Check have to be made to identify the node as the required kind:
+ * All properties 'templateUrl', 'styles', 'styleUrls' ANYWHERE will be modified, even if they
+ * are not used in the context of an Angular Component.
  * 
- * ClassDeclaration: isClassDeclaration
- *   Decorator
- *     CallExpression: isCallExpression
- *       ObjectLiteralExpression: isObjectLiteral
- *         PropertyAssignment: isPropertyAssignment
- *           Identifier: isIdentifier
- *           StringLiteral: isStringLiteral
+ * The AST has to simply look like this anywhere in a ts file:
  * 
+ * PropertyAssignment
+ *   Identifier
+ *   Initializer
  */
 
 
-// take care of importing only types, for the rest use injected `ts`
+// only import types, for the rest use injected `ConfigSet.compilerModule`
 import TS, {
   Node,
   SourceFile,
   TransformationContext,
   Transformer,
   Visitor,
-  ClassDeclaration,
-  CallExpression,
-  ObjectLiteralExpression,
   PropertyAssignment,
   Identifier,
   StringLiteral,
@@ -104,18 +75,6 @@ export const version = 1
  */
 export function factory(cs: ConfigSet) {
 
-  /**
-   * Array Flatten function, as there were problems to make the compiler use
-   * esnext's Array.flat, can be removed in the future.
-   * @param arr Array to be flattened
-   */
-  function flatten<T>(arr: (T | T[] | T[][])[]): T[] {
-    return arr.reduce(
-      (xs: T[], x) => Array.isArray(x) ? xs.concat(flatten(x as T[])) : xs.concat(x),
-      []
-    ) as T[]
-  }
-
   /**
    * Our compiler (typescript, or a module with typescript-like interface)
    */
@@ -125,71 +84,61 @@ export function factory(cs: ConfigSet) {
    * Traverses the AST down to the relevant assignments in the decorator
    * argument and returns them in an array.
    */
-  function getPropertyAssignmentsToTransform(classDeclaration: ClassDeclaration) {
-    return flatten<PropertyAssignment>(classDeclaration.decorators!
-      .map(dec => dec.expression)
-      .filter(ts.isCallExpression)
-      .map(callExpr => (callExpr as CallExpression).arguments
-        .filter(ts.isObjectLiteralExpression)
-        .map(arg => (arg as ObjectLiteralExpression).properties
-          .filter(ts.isPropertyAssignment)
-          .map(arg => arg as PropertyAssignment)
-          .filter(propAss => ts.isIdentifier(propAss.name))
-          .filter(propAss => TRANSFORM_PROPS.includes((propAss.name as Identifier).text))
-        )
-      )
-    )
+  function isPropertyAssignmentToTransform(node: Node): node is PropertyAssignment {
+    return ts.isPropertyAssignment(node) &&
+      ts.isIdentifier(node.name) &&
+      TRANSFORM_PROPS.includes(node.name.text)
   }
 
   /**
-   * Clones the node, identifies the properties to transform in the decorator and modifies them.
-   * @param node class declaration with decorators
+   * Clones the assignment and manipulates it depending on its name.
+   * @param node the property assignment to change
    */
-  function transfromDecoratorForJest(node: ClassDeclaration) {
-
-    const mutable = ts.getMutableClone(node)
-    const assignments = getPropertyAssignmentsToTransform(mutable)
-
-    assignments.forEach(assignment => {
-      switch ((assignment.name as Identifier).text) {
-        case TEMPLATE_URL:
-          // we can reuse the right-hand-side literal from the assignment
-          let templatePathLiteral = assignment.initializer
-
-          // fix templatePathLiteral if it was a non-relative path
-          if (ts.isStringLiteral(assignment.initializer)) {
-            const templatePathStringLiteral: StringLiteral = assignment.initializer;
-            // match if it starts with ./ or ../ or /
-            if (templatePathStringLiteral.text &&
-                !templatePathStringLiteral.text.match(/^(\.\/|\.\.\/|\/)/)) {
-              // make path relative by appending './'
-              templatePathLiteral = ts.createStringLiteral(`./${templatePathStringLiteral.text}`)
-            }
+  function transfromPropertyAssignmentForJest(node: PropertyAssignment) {
+
+    const mutableAssignment = ts.getMutableClone(node)
+
+    const assignmentNameText = (mutableAssignment.name as Identifier).text
+
+    switch (assignmentNameText) {
+      case TEMPLATE_URL:
+        // reuse the right-hand-side literal from the assignment
+        let templatePathLiteral = mutableAssignment.initializer
+
+        // fix templatePathLiteral if it was a non-relative path
+        if (ts.isStringLiteral(mutableAssignment.initializer)) {
+          const templatePathStringLiteral: StringLiteral = mutableAssignment.initializer;
+          // match if it starts with ./ or ../ or /
+          if (templatePathStringLiteral.text &&
+              !templatePathStringLiteral.text.match(/^(\.\/|\.\.\/|\/)/)) {
+            // make path relative by appending './'
+            templatePathLiteral = ts.createStringLiteral(`./${templatePathStringLiteral.text}`)
           }
+        }
+
+        // replace 'templateUrl' with 'template'
+        mutableAssignment.name = ts.createIdentifier(TEMPLATE)
+        // replace current initializer with require(path)
+        mutableAssignment.initializer = ts.createCall(
+          /* expression */ ts.createIdentifier(REQUIRE),
+          /* type arguments */ undefined,
+          /* arguments array */ [templatePathLiteral]
+        )
+        break;
+      case STYLES:
+      case STYLE_URLS:
+        // replace initializer array with empty array
+        mutableAssignment.initializer = ts.createArrayLiteral()
+        break;
+    }
 
-          // replace 'templateUrl' with 'template'
-          assignment.name = ts.createIdentifier(TEMPLATE)
-          // replace current initializer with require(path)
-          assignment.initializer = ts.createCall(
-            /* expression */ ts.createIdentifier(REQUIRE),
-            /* type arguments */ undefined,
-            /* arguments array */ [templatePathLiteral]
-          )
-          break;
-        case STYLES:
-        case STYLE_URLS:
-          // replace initializer array with empty array
-          assignment.initializer = ts.createArrayLiteral()
-          break;
-      }
-    })
-    return mutable
+    return mutableAssignment
   }
 
   /**
    * Create a source file visitor which will visit all nodes in a source file
    * @param ctx The typescript transformation context
-   * @param sf The owning source file
+   * @param _ The owning source file
    */
   function createVisitor(ctx: TransformationContext, _: SourceFile) {
 
@@ -202,30 +151,22 @@ export function factory(cs: ConfigSet) {
       let resultNode: Node
 
       // before we create a deep clone to modify, we make sure that
-      // this class has the decorator arguments of interest.
-      if (
-        ts.isClassDeclaration(node) &&
-          node.decorators &&
-          getPropertyAssignmentsToTransform(node).length
-      ) {
-        // get mutable node and change properties
-        // NOTE: classes can be inside methods, but we do not
-        // look for them inside Angular Components!
-        // recursion ends here, as ts.visitEachChild is not called.
-        resultNode = transfromDecoratorForJest(node)
+      // this is an assignment which we want to transform
+      if (isPropertyAssignmentToTransform(node)) {
+
+        // get transformed node with changed properties
+        resultNode = transfromPropertyAssignmentForJest(node)
       } else {
-        // look for other classes with decorators
-        // classes can be inside other statements (like if blocks)
+        // look for interesting assignments inside this node
         resultNode = ts.visitEachChild(node, visitor, ctx)
       }
 
-      // finally returns the currently visited node
+      // finally return the currently visited node
       return resultNode
     }
     return visitor
   }
 
-  // returns the transformer factory
   return (ctx: TransformationContext): Transformer<SourceFile> =>
     (sf: SourceFile) => ts.visitNode(sf, createVisitor(ctx, sf))
 }

yarn.lock

@@ -16,6 +16,11 @@
   resolved "https://registry.yarnpkg.com/@types/jest/-/jest-23.3.10.tgz#4897974cc317bf99d4fe6af1efa15957fa9c94de"
   integrity sha512-DC8xTuW/6TYgvEg3HEXS7cu9OijFqprVDXXiOcdOKZCU/5PJNLZU37VVvmZHdtMiGOa8wAA/We+JzbdxFzQTRQ==
 
+"@types/node@^10.12.12":
+  version "10.12.12"
+  resolved "https://registry.yarnpkg.com/@types/node/-/node-10.12.12.tgz#e15a9d034d9210f00320ef718a50c4a799417c47"
+  integrity sha512-Pr+6JRiKkfsFvmU/LK68oBRCQeEg36TyAbPhc2xpez24OOZZCuoIhWGTd39VZy6nGafSbxzGouFPTFD/rR1A0A==
+
 "@types/node@^6.0.46":
   version "6.0.88"
   resolved "https://registry.yarnpkg.com/@types/node/-/node-6.0.88.tgz#f618f11a944f6a18d92b5c472028728a3e3d4b66"