jest-preset-angular feat: alternative setup with babel/typescript

This would add an alternative compiling method alongside ts-jest.

Done so far

Babel config is in plain js inside new babel folder
second preset is also located in there
added second preset inside babel folder, which can be used via preset: 'jest-preset-angular/babel' in jest config
added basic preconfigured babel.config.js
added load-html-transformer.js to deal with html files
added inline-template.plugin.js to do what InlineFilesTransformer.ts does
removed ts-jest from dependencies, which means it would have to be be installed when using this preset with ts-jest from now on

The following libraries have to be installed alongside the project, when setting up jest-preset-angular + babel

@babel/core
@babel/preset-env
@babel/preset-typescript
@babel/plugin-proposal-decorators
@babel/plugin-proposal-class-properties
babel-plugin-transform-typescript-metadata
babel-plugin-const-enum

This would be especially interesting regarding some feedback from the community about speed difference in bigger projects. Babel does no TypeChecking in the .test.ts/.spec.ts files itself, but going through the issues of ts-jest (e. g. https://github.com/kulshekhar/ts-jest/issues/1115, surely ahnpnl knows more) some projects are falling back to isolatedModules: true already, which is also skipping the type checking. Furthermore type checks can still be achieved separately using tsc --noEmit.

Personally I think we need some more restructuring in the preset to keep ts-jest and babel separated as two exclusive usage options, so this might take some time. I think this can land some time later this year.

TODOs

[x] Write tests for babel inline template plugin
[x] Figure out preset folder setup with ts-jest and babel separated (see discussion in comments below)
[x] Run example app for both, babel and ts-jest (see discussion)
[x] Make PluginInlineAngularTemplate to also remove styleUrls to behave like the ts-jest transformer
[x] Babel and ts-jest Usage Documentation in README.md
[x] Quick user guidance to decide between the two presets

Closes #308

Edit1: updated list of babel dependencies Edit2: added TODO list Edit3: update TODOs

Oct 02 '19 21:10 wtho

I think we need also plugin for babel plugin const enum for this PR

Oct 03 '19 05:10 ahnpnl

Yeah! Let's add a test with enums in the example app!

Oct 03 '19 10:10 wtho

In last force push:

Added a test with enum and const enum and added the babel plugin for const enums to ensure the preset can deal with this TS language feature when using babel.

Oct 16 '19 15:10 wtho

In the newest push I added some opiniated changes, so feel free to correct it wherever it looks seldom.

Proposal: Folder Structure with babel & ts-jest

.
├── README.md
├── package.json
├── [other-base-level-files]
└── src
    ├── AngularNoNgAttributesSnapshotSerializer.js
    ├── AngularSnapshotSerializer.js
    ├── HTMLCommentSerializer.js
    ├── index.ts
    ├── reflectMetadata.ts
    ├── setupJest.ts
    ├── babel
    │   ├── babel.config.ts
    │   ├── jest-preset.ts
    │   ├── load-html-transformer.ts
    │   └── plugin-inline-angular-template.ts
    ├── ts-jest
    │   ├── InlineFilesTransformer.ts
    │   ├── jest-preset.ts
    │   ├── StripStylesTransformer.ts
    │   └── TransformUtils.ts
    ├── __tests__ [folder with tests]
    └── zone-patch [folder with sources]

ts-jest and babel would be built to jest-preset-angular/build/ts-jest and jest-preset-angular/build/babel respectively.

This would lead for users to adjust their jest.config.js:

module.exports = {
  preset: "jest-preset-angular/build/ts-jest",
  // or
  preset: "jest-preset-angular/build/babel"
}

We could add additional folders with a proxy jest-preset.js at jest-preset-angular/babel and jest-preset-angular/ts-jest, but that would add additional confusion to our project itself. Anyway there should be a jest-preset.js in the root that points at build/ts-jest/jest-preset.js for backwards compatibility.

Proposal: Run project example app with ts-jest and babel To run the project with both the settings, I suggest we set the preset value in example/jest.config.js programmatically, e. g. depending on a env variable: export JPA_TEST_MODE=ts-jest yarn test

// jest.config.js
const testMode = process.env.JPA_TEST_MODE || 'ts-jest';

module.exports = {
  preset: `jest-preset-angular/build/${testMode}`,
  ...

This way we do not have to duplicate any code base and make sure the two transpilation modes are actually easily exchangable. Both transpilation plugins from babel and ts-jest have to be installed initially, which would also speed up the CI test time.

There is one issue (see CI tests): tsc transpiles undefined class properties differently than babel/typescript does (babel adds the property):

  <app-calc
    hasAClass="false"
    image={[Function String]}
+   observable$="undefined"
    prop1={[Function Number]}
  >

We have to plugin optional tests somehow. Any experience with this?

This PR would be a big difference for this preset, so it is quite important to me what you two think, @ahnpnl and @thymikee. Note that the speedup of the example app testrun is similar to the one of ts-jest with isolatedModules: true (~4s). Would be interested if there is any difference in bigger projects though!

Oct 22 '19 20:10 wtho

I like the idea of having 2 internal presets for babel and ts-jest and let users choose what they want to use. Regarding to the folder structure, all LGTM but imo I like putting tests outside src folder. It will be easier for us to build and release (less configuration in excluding something).

I haven't tried to use babel yet so I wasn't aware of the different in compiling. Looks interesting. Is it intended behavior of babel team ?

Oct 23 '19 05:10 ahnpnl

I think the different compilation output is a minor design decision. All the plugins are created by different people, even the "official" babel ones, and babel and typescript clearly do not work too closely together. It behaves the same when running the code, and that's the important part.

class TestComponent {
  field: boolean;
}

compile output using tsc (swallows the undefined field)

var ClassComponent = /** @class */ (function () {
    function ClassComponent() {
    }
    return ClassComponent;
}());

compile output using babel (adds the undefined field)

var ClassComponent = function ClassComponent() {
  _classCallCheck(this, ClassComponent);
  this.field = void 0;
};

I am not so sure about the __tests__ being outside of src, it was there since we added the build folder approach (#307) and it is just a single exclude in tsconfig.json on the __tests__ folder. The tests will never be compiled to build and src anyway will never be included in the release, it's only the build folder.

Oct 23 '19 13:10 wtho

Now it is definitely ready for review, although I do not feel good about the test mode detection. Maybe you have a better idea?

Also with this test that behaves differently (observable$="undefined") I am not sure if my inline snapshot is a good solution.

Good thing is I used the very same test cases for the babel ast transformer as for the ts-jest ast transformer. I duplicated them for now, but can extract them to a single source.

Oct 25 '19 20:10 wtho

Now it is definitely ready for review, although I do not feel good about the test mode detection. Maybe you have a better idea?

I'd do similar things as well.

Also with this test that behaves differently (observable$="undefined") I am not sure if my inline snapshot is a good solution.

Maybe use fixture.nativeElement instead of fixture. I think testing HTML template shouldn't use that fixture but rather than HTML elements rendering. And for the different output related to ts-jest vs babel compiling, I think there should be a note highlighted in the documentation about that.

Oct 27 '19 10:10 ahnpnl

Maybe use fixture.nativeElement instead of fixture. I think testing HTML template shouldn't use that fixture but rather than HTML elements rendering.

Snapshotting fixture.nativeElement does solve the issue, as it does not render the properties, but I guess that was the point of the original test. The test then will diff this:

- <app-calc
-   hasAClass="false"
-   image={[Function String]}
-   prop1={[Function Number]}
+ <div
+   id="root0"
  >
    <p
      class="a-default-class"
    >
       calc works! 1337 another text node test-image-stub 
    </p>
- </app-calc>
+ </div>

I am not so sure if that's a good snapshot anymore.

Oct 27 '19 14:10 wtho

The snapshot thing indeed depends on the purpose of users when testing a component. If users care about the property rendered by ts file, fixture is the right one. Otherwise if users only care about html change, fixture.nativeElement is the choice.

I think we don't need to care about this too much. Perhaps we can add a note in README to warn about the difference of snapshot relative to usage of tsc vs babel. I think that should be fine.

At work I usually use fixture.nativeElement because I only want to check about html change.

Oct 28 '19 16:10 ahnpnl

👍 Already added a small note in the README.md. I know, it's not very visible, but someone who does read through the README will encounter it.

Oct 28 '19 16:10 wtho

@thymikee normally, I don't comment on pull requests. But I'm pretty interested in using this if it leads to test suite speed increases, so your feedback is valued.

For now, I'm actually going to be converting my teams jest tests to use babel. As my initial testing lead to some serious speed improvements with ~400 individual tests.

@wtho I can report rough performance improvements if you'd like. Also, I'm using the isolatedModules: true option in our tsconfig.

Dec 02 '19 15:12 BensonBen

Making it easier to use Babel sounds like a good idea, I just didn't find the time to have a look yet :D

Dec 02 '19 15:12 thymikee

@BensonBen Yeah, a performance evaluation would be super helpful! I do not have a big codebase at hand currenlty. Maybe you can install jpa using npm install -D git://github.com/wtho/jest-preset-angular.git#feat/babel-jest-angular and run them with the new configuration.

Also I'm interested in which adjustments were required to use babel. Making switching as easy as possible would be desired from our side.

Dec 02 '19 18:12 wtho

True, we can keep the structure and just point it at ts-jest as default. I just liked the equality between the two options and disregarded backwards-compatibility.

I would also suggest to create a new issue to gather babel feedback and link the issue next to the babel setup notes.

Changes in last commit

add default jest-preset.js to provide full backwards-compatibility
add ts-jest to dependencies
Set package version to 8.1.0 (ready for squash, tag & push)
adjust CHANGELOG.md
rephrase README.md, add notes for preset comparison feedback (can be removed later again)

Dec 02 '19 23:12 wtho

I would like to test, but i get errors with babel

ts-jest

babel I get errors like: Can't resolve all parameters for ...: (?).

babel is a lot slower too, using following additional settings for ts-jest

diagnostics: false,
isolatedModules: true,

Dec 03 '19 07:12 viceice

Interesting!

Can you create a gist just containing the classes that have injection issues (and the classes/providers that could not be injected)? Decorators, classname and constructor signatures are sufficient. Methods, class fields and templates are not involved, so better remove them.

This information would help a ton!

Dec 03 '19 07:12 wtho

looks like it is always the same class MyHandler which is used to provide a service. I hope this is enough to reproduce.

Technically I'm using a nrwl workspace with multiple library packages.

import { NgModule, Injectable, Optional } from '@angular/core';
// import { NGXLogger } from 'ngx-logger';
// import { MessageService } from 'primeng/api';
import {
    MissingTranslationHandler,
    MissingTranslationHandlerParams,
    TranslateCompiler,
    TranslateLoader,
    TranslateModule,
    TranslateService,
} from '@ngx-translate/core';

@Injectable({ providedIn: 'root' })
export class Logger {
  constructor(/* private readonly _log: NGXLogger */) {}
}

@Injectable()
export class MessageService {
}

class MyHandler {
  constructor(@Optional() private messageService: MessageService, private readonly _log: Logger) {}
  handle(params: MissingTranslationHandlerParams) { return params.key; }
}

@NgModule({
  imports: [ TranslateModule ],
})
export class SharedModule {
constructor(ts: TranslateService, private readonly _log: Logger) {}
}

describe('SharedModule', () => {

	beforeEach(async(() => {
		TestBed.configureTestingModule({
		  imports: [
			TranslateModule.forRoot({
			  missingTranslationHandler: { provide: MissingTranslationHandler, useClass: MyHandler },
			  useDefaultLang: false,
			}),
		  ],
		  providers: [{ provide: MessageService, useValue: { add: jest.fn() } }],
		}).compileComponents();
	});
	
	it('should create', () => {
        expect(SharedModule).toBeDefined();
    });
});

Dec 03 '19 08:12 viceice

@ViceIce I tried out the tests and as soon as I add @Injectable to MyHandler it works in babel, but not in ts-jest, so this seems contradictary to what you said.

Maybe we can figure it out in this minimal repro: wtho/jpa-babel-bug

Dec 03 '19 22:12 wtho

@wtho Next bug wtho/jpa-babel-bug#2

Dec 04 '19 13:12 viceice

found workarounds for the bugs, here are the new timings:

second run

Dec 04 '19 14:12 viceice

While we have to wait for https://github.com/leonardfactory/babel-plugin-transform-typescript-metadata/pull/24, you guys can help us by doing further tests on bigger codebases.

Please try out the babel preset using a branch where I included the babel-plugin PR and the built js files (which would not be included normally checking out a git branch as npm dep).

npm i -D jest-preset-angular@git://github.com/wtho/jest-preset-angular#test/tryout-babel-jest-angular

(make sure you followed the other instructions in the jest-preset-angular/babel README)

Please report any problems. Feel free to open issues on https://github.com/wtho/jpa-babel-bug

Thanks!

Dec 20 '19 16:12 wtho

News: Angular 9 is out and they require @Injectable more aggressive. https://angular.io/guide/migration-injectable

Feb 08 '20 22:02 viceice

Finally! I think that's no problem for the babel scenario (or the preset in general).

Feb 09 '20 11:02 wtho

Yes, but fixes one of my bugs by design. 😊

Feb 09 '20 12:02 viceice

FYI: https://github.com/kulshekhar/ts-jest/pull/1549 will be in alpha version of ts-jest (possibly today). You can test the alpha version and give some feedbacks for https://github.com/kulshekhar/ts-jest/issues/1115

Apr 21 '20 11:04 ahnpnl

I guess we can close this issue since we have to use Program

Oct 27 '20 10:10 ahnpnl

I think we can keep open just in case this gets interesting again. I'll make it a draft though.

Oct 27 '20 11:10 wtho

jest-preset-angular jest-preset-angular copied to clipboard

feat: alternative setup with babel/typescript

jest-preset-angular
jest-preset-angular copied to clipboard