jest-preset-angular: InlineHtmlStripStylesTransformer is not strict enough

Current Behavior

Any object literals have their property assignments transformed thus causing source code such as:

console.log({
  styles: ['styles.css']
})

to print out

{
  styles: []
}

Expected Behavior

Only object literals passed to @angular/core’s @Component decorator should have it’s property assignments transformed.

About this issue

  • Original URL
  • State: closed
  • Created 5 years ago
  • Reactions: 5
  • Comments: 15 (2 by maintainers)

Most upvoted comments

Ok, so one more thought I just had, templateUrl is actually more important to be transformed than styles to be removed. Without templateUrl being transformed to template, it will not even load.

styles on the other hand is just removed for performance issues (correct me if I’m wrong), as we assume styles are usually not tested in jest, that is better done in e2e frameworks.

Proposal

We could also think about handling them differently, although this might complicate the code unnecessarily.

templateUrl:

  • replaced everywhere
  • alternatively only (as suggested by @FrozenPandaz) in files where Component gets imported from @angular/core or TestBed gets imported from @angular/core/testing

styles:

  • replaced only if declared directly inside component decorators

This would seem more practical to me as styles is also used in more contexts than templateUrl.

Yeah, I am almost done with the PR, will submit it this weekend.

It’s not really an error, it is an intrusive caveat that kept the transformer simple, but can create side-effects.

I agree, the second example is not very important.

The third and fourth are the problematic ones.

@electrichead The AST transformer is written in TypeScript, but it works with AST objects. AST is a representation of the code text, not of the evaluated objects.

So this situation

const componentConfig = { styles: [], templateUrl: './file.html' };

@Component(componentConfig) export class MyThirdComp { }

is definitely a problem, as in this example each line is an independent object. References are made when JavaScript is interpreted, not when the AST transformer is executed. The styles assignment is not inside the decorator and therefore we would have to search for the componentConfig variable in the whole file, maybe even in other files if it is imported.

Yeah, this caveat is known and discussed

Problem is, that theoretically someone might rename the annotation, or pull an object into the annotation, that is not declared inside the annotation itself.

The AST transformer does not know if an object is actually the Angular Component Decorator or if an object with styles and templateUrl is actually a component configuration, it only knows in what relation code pieces are and how they are named.

Consider these examples:

import { Component } from '@angular/core';


// classic and easy
@Component({ templateUrl: './file.html' }) export class MyFirstComp { }

// difficult
const MyAnnotation = Component;
@MyAnnotation({ templateUrl: './file.html' }) export class MySecondComp { }

// difficult
const componentConfig = { styles: [], templateUrl: './file.html' };
@Component(componentConfig) export class MyThirdComp { }

// difficult
const partialComponentConfig = { templateUrl: './file.html' };
@Component({...partialComponentConfig, styles: []}) export class MyThirdComp { }

We should discuss approaches and their caveats before implementing it.


Edit: fixed linked issues