jest-preset-angular icon indicating copy to clipboard operation
jest-preset-angular copied to clipboard

[Bug]: type-only imports cause ReferenceError

Open Simon-Hayden-iteratec opened this issue 2 years ago • 9 comments

Version

11.0.1

Steps to reproduce

@leosvelperez created a repo showing the failure: https://github.com/leosvelperez/ng-jest-type-issue

Just clone the repo, install depdencies and execute the tests using jest.

(For reference, this bug was originally reported to the Nx team here: https://github.com/nrwl/nx/issues/7845)

Expected behavior

Tests which use import type should pass. See the official Typescript documentation.

Actual behavior

An error is thrown:

  ● AppService › should be created

    ReferenceError: Request is not defined

      1 | import { Inject, Injectable, Optional } from '@angular/core';
    > 2 | import type { Request } from 'express';
        |               ^
      3 | import { REQUEST } from './app.tokens';
      4 |
      5 | @Injectable({

Additional context

The context of this issue is, that we are using Angular Universal for Server Side Rendering. Hence we sometimes have to inject the Browser's request/response into a service or similar. Since we do not want to accidentally bundle express into our Angular app, we want to use type-only imports.

Environment

npx: installed 1 in 0.89s

  System:
    OS: Linux 5.14 Fedora Linux 35 (Workstation Edition)
    CPU: (8) x64 Intel(R) Core(TM) i7-7820HQ CPU @ 2.90GHz
  Binaries:
    Node: 14.17.6 - ~/.nvm/versions/node/v14.17.6/bin/node
    Yarn: 1.22.15 - ~/.nvm/versions/node/v14.17.6/bin/yarn
    npm: 6.14.15 - ~/.nvm/versions/node/v14.17.6/bin/npm
  npmPackages:
    jest: ^27.3.1 => 27.3.1

Simon-Hayden-iteratec avatar Nov 24 '21 10:11 Simon-Hayden-iteratec

workaround is you should not use import type if you have anything injected in constructor.

Another workaround is adjust the include field in tsconfig.spec.json to be

{
    //...
   "include": ["src/**/*.ts"]
}

ahnpnl avatar Nov 24 '21 20:11 ahnpnl

Thanks for answering so swiftly!

workaround is you should not use import type if you have anything injected in constructor.

Sorry, not quite sure what you mean by that. Should we remove the type from the import type or should we remove the parameter from the constructor? Or just the type of the parameter?

So given the example provided here, what would you change:

import { Inject, Injectable, Optional } from '@angular/core';
import type { Request } from 'express';
import { REQUEST } from './app.tokens';

@Injectable({
  providedIn: 'root',
})
export class AppService {
  constructor(@Inject(REQUEST) @Optional() private request: Request) {}
}

Another workaround is adjust the include field in tsconfig.spec.json to be [...]

I added it to our tsconfig.spec.json and it works! I was just wondering, is there any drawback in doing it that way? Since the default configuration (generated by the nx CLI in our case) omits it, there must be a good reason for it, right? (Sorry if this is a dumb question to ask, jest is not my strong point)

Simon-Hayden-iteratec avatar Nov 25 '21 06:11 Simon-Hayden-iteratec

About how the syntax, I meant import { Request } instead of import type { Request }.

The drawback of changing include in tsconfig might be that Jess might start slower. But anyways the current startup speed is already slow. The value of include impacts how many files ts-jest (we use ts-jest to compile) would need to process before the actual tests run.

ahnpnl avatar Nov 25 '21 08:11 ahnpnl

Thanks for your explanation.

About how the syntax, I meant import { Request } instead of import type { Request }.

While this would work too - obviously - I'm afraid the Angular compiler would then include the express library in our (frontend) bundle. I might be wrong here, but I think it would easily be possible to "misuse" the import in an unintended way.

Anyway, thanks for confirming the bug :)

Simon-Hayden-iteratec avatar Nov 25 '21 08:11 Simon-Hayden-iteratec

I added a bit explanation also in a similar issue.

ahnpnl avatar Nov 25 '21 10:11 ahnpnl

@ahnpnl Any news about the bug? include didn't help with type-only imports, still can't get import cross nx libraries

hollowb4ck avatar Feb 10 '22 12:02 hollowb4ck

I'm experiencing the same problem in our monorepository (Nx 14.3.6, Angular 14.0.3, Jest 28.1.1, Typescript 4.7.4, jest-preset-angular 12.1.0).

This workaround:

ahnpnl commented on Nov 24, 2021 Another workaround is adjust the include field in tsconfig.spec.json to be

{ //... "include": ["src/**/*.ts"] }

seemed to do the trick, so thanks for that solution. I don't really understand why this fixes the problem though!

JohanHeyvaert avatar Jun 28 '22 14:06 JohanHeyvaert

Also having the same issue

image

this is failing for every type that we are importing, we don't use import type but we are having the same problem.

We do have have setup paths in tsconfig to improve our imports, @mypathDefinition , aside from that there is nothing else worth mentioning

Adding the "include": ["src/**/*.ts"] to our "tsconfig.spec.json" did not help at all.

Any updates on this issue or any other way on how we can sort this out?

Angular 13.3.9 / jest 28.1.0 / types-jest 27.0.2 / jest-preset-angular 12.0.1

JoaoP57 avatar Sep 05 '22 14:09 JoaoP57

The same problem for Angular v14.2.1 / jest v28.1.3 / jest-preset-angular v12.2.2 The workaround didn't help

zotovamk avatar Sep 05 '22 14:09 zotovamk

I can replicate this problem on Angular 14, just by updating jest-preset-angular to 12.0.0 from 11.1.2.

However, if I set the resolutions section in package.json to point to ts-jest@^27.0.0, and then patch the files inside jest-preset-angular back to the old ts-jest import paths I can get it to work again. So for our repo at least, something changed in ts-jest@28 that caused type-only imports to no longer work.

Changing from import to import type does work, but simply adding "src/**/*.ts" to the tsconfig.spec.json does not.

jreidgreer avatar Sep 16 '22 18:09 jreidgreer

So have been seeing reports of this for Nx workspaces.

here is a reproduce-able for an nx workspace that @jcabannes provided me, turning emitDecoratorMetadata: false is kind of a work around I've found, but not an ideal solution since I think it's just kicking the ball down the road.

https://github.com/nrwl/nx/issues/10660#issuecomment-1251030448

What we are seeing is basically the transformed code is missing an import being declared but is checking for that import in the decorator metadata of the compiled TS code. so removing the decorator metadata removes the check, causing the error to go away. Of course if the metadata is needed then tests will fail for other reasons, so bandaid solution for now. unsure if this is a jest-preset-angular/ts-jest/typescript/angular issue right now.

barbados-clemens avatar Sep 21 '22 13:09 barbados-clemens

Just to add more to this. it looks like this is only happening when a decorator in involved. Almost like tsc emits incomplete info, such as not being able to transpile the import? this is why turning off emitDecoratorMetadata works for some, but isn't a full solution since tests can need that metadata.

Any more info anyone has come across?

barbados-clemens avatar Oct 05 '22 20:10 barbados-clemens

The same problem for Angular v14.2.1 / jest v28.1.3 / jest-preset-angular v12.2.2 The workaround didn't help

did you find any workaround?

philippesc avatar Oct 23 '22 13:10 philippesc

I am also having this kinda issue:

Screenshot 2022-10-23 at 15 11 23

We are using Typescript Paths to have readable import paths. We are not using Type-only imports.

We have Angular 14.2.5, Jest 28.1.3 and jest-preset-angular 12.2.2.

None of the above mentioned workarounds is working.

philippesc avatar Oct 23 '22 13:10 philippesc

I'm also getting the same error: ReferenceError: lcl_routing_1 is not defined

Workaround does not help.

Angular v14.2.10 / jest v28.1.3 / ts-jest 28.0.8 / jest-preset-angular v12.2.2

dylannnn avatar Nov 28 '22 09:11 dylannnn

Getting the same issues with jest-preset-angular v12.2.3.

anschm avatar Nov 30 '22 17:11 anschm

It seems something is broken in ts-jest@28. Is their any bug opend at ts-jest?

anschm avatar Dec 01 '22 12:12 anschm

This is the limitation of current architecture when transforming files for Angular. Technically, when a Jest transformer does the work, we use TypeScript Language Service to transform Angular codes with some AST transformers in a scope of a Jest worker.

This bug is caused by the fact that, the Language Service is in a Jest worker doesn't have enough information to transform Angular codes which leads to this error. The way how Angular codes are processed is, it relies on a top level TypeScript Program which has all information.

To fix the problem problem, we need to follow how Angular does with their codes, which means we need to transform all the files even before Jest starts. When Jest starts, those transformed codes should be used and give it to Jest so Jest can run the tests.

ahnpnl avatar Dec 01 '22 12:12 ahnpnl

@ahnpnl could you fix this problem or does anybody else doing this?

anschm avatar Dec 05 '22 15:12 anschm

Idk how to fix it yet:(

ahnpnl avatar Dec 05 '22 15:12 ahnpnl

I also have the error with custom TypeScript decorators (only have this since migrating to Nx workspace 14.7.6 - Jest 28):

image

The class in which the error occurs:

import { DataMinerTypes, DMAObject } from '@cube-mobile/global';
import { IDMAGenericInterfaceQueryOption } from '../interfaces/QueryOption.interface';
import { DMAGenericInterfaceQueryOptionValues } from '../types/QueryOptionValue.type';
import { GIQueryOptionType } from '../types/QueryOption.type';
import { String as GI_String } from './Primitives';

@DataMinerTypes.registerType('DMAGenericInterfaceQueryOption')
export class DMAGenericInterfaceQueryOption<T extends DMAGenericInterfaceQueryOptionValues> implements IDMAGenericInterfaceQueryOption<T> {
    public Value: T;
    public MetaData: DMAObject[];

    public constructor(public ID: string, public Name: string, public IsMandatory: boolean, public Type: GIQueryOptionType) { }

    public getDisplayValue(): GI_String | GI_String[] {
        return this.Value ? new GI_String(this.Value.toString()) : null;
    }
}

The error is caused by the decorator: @DataMinerTypes.registerType('DMAGenericInterfaceQueryOption'):

@Injectable()
export class DataMinerTypes {
    public static registerType(typeId: string): Function {
        return <T extends new (...args: any[]) => object>(type: T) => {
            ...
        };
    }
}

Apart of the @Injectable() this is actually pure TypeScript, there's no Angular specific code in this.

The error disappears when putting emitDecoratorMetadata on false but this introduces other issues down the road as the decorator isn't properly executed.

wimme avatar Dec 05 '22 15:12 wimme

for me this patch fixes the issue:

diff --git a/node_modules/jest-preset-angular/build/transformers/downlevel_decorators_transform/downlevel_decorators_transform.js b/node_modules/jest-preset-angular/build/transformers/downlevel_decorators_transform/downlevel_decorators_transform.js
index 7ee0201061e..c30b326b127 100644
--- a/node_modules/jest-preset-angular/build/transformers/downlevel_decorators_transform/downlevel_decorators_transform.js
+++ b/node_modules/jest-preset-angular/build/transformers/downlevel_decorators_transform/downlevel_decorators_transform.js
@@ -299,7 +299,7 @@ function getDownlevelDecoratorsTransform(typeChecker, host, diagnostics, isCore,
                 newMembers.push(createPropDecoratorsClassProperty(diagnostics, decoratedProperties));
             }
             const members = typescript_1.default.setTextRange(typescript_1.default.factory.createNodeArray(newMembers, classDecl.members.hasTrailingComma), classDecl.members);
-            return (0, ts_compatibility_1.createClassDeclaration)((0, ts_compatibility_1.combineModifiers)(decoratorsToKeep.size ? Array.from(decoratorsToKeep) : undefined, (0, ts_compatibility_1.getModifiers)(classDecl)), classDecl.name, classDecl.typeParameters, classDecl.heritageClauses, members);
+            return (0, ts_compatibility_1.updateClassDeclaration)(classDecl, (0, ts_compatibility_1.combineModifiers)(decoratorsToKeep.size ? Array.from(decoratorsToKeep) : undefined, (0, ts_compatibility_1.getModifiers)(classDecl)), classDecl.name, classDecl.typeParameters, classDecl.heritageClauses, members);
         }
         function decoratorDownlevelVisitor(node) {
             if (typescript_1.default.isClassDeclaration(node)) {

Idk for sure, but suspect that calling create* methods instead of update* makes TS program to forget information about whether a particular import is type-only or not.

n9niwas avatar Dec 11 '22 03:12 n9niwas

The transformer is downloaded from Angular source codes. If you wish to change it, you should consult with Angular team.

ahnpnl avatar Dec 11 '22 07:12 ahnpnl

The transformer is downloaded from Angular source codes. If you wish to change it, you should consult with Angular team.

@ahnpnl Can you elaborate on where that transformer comes from / how it is downloaded? I'll try to discuss this with angular team but it's hard to understand the actual mechanism. In downlevel-ctor.ts, getDownlevelDecoratorsTransform is imported from ./downlevel_decorators_transform which does not exist.

jbjhjm avatar Dec 12 '22 10:12 jbjhjm

You can check here https://github.com/thymikee/jest-preset-angular/blob/de881987c681d45907fb4f298704006813eb3912/scripts/prebuild.js#L8 it is a prebuild script that we first download and then build the package.

ahnpnl avatar Dec 12 '22 10:12 ahnpnl

@ahnpnl - over at angular repo, crisbeto says it would be ok to revert to use updateClassDeclaration as it seems the change to createClassDecoration was done without any particular reason.

However he/she states it would be good to add a unit test to ensure correct behavior in the future. I guess atm you know best about the differences caused by this and how they lead to reference errors...

Do you think you can provide some insights on what is wrong exactly? Think that will speed up the fix on angular side and make this work fine again :)

jbjhjm avatar Dec 15 '22 17:12 jbjhjm

Alright, crisbeto just freed up the way to make it work with ng15 compiler. Details: https://github.com/angular/angular/commit/33f35b04ef0f32f25624a6be59f8635675e3e131

So with the next ng minor release @ahnpnl should be able to make it work correctly again :)

jbjhjm avatar Jan 05 '23 09:01 jbjhjm

@ahnpnl - angular devs created a preview release (15.1.0-rc.0) which includes the mentioned fix just before the weekend. Can you release a preview too based on the [email protected]? Then we can all investigate if the issues are gone!

jbjhjm avatar Jan 10 '23 11:01 jbjhjm

@jbjhjm I tested this in angular 15.1 but its still not working

gmfun avatar Jan 13 '23 12:01 gmfun

@jbjhjm I tested this in angular 15.1 but its still not working

Thanks for the report but I'm only trying to manage communication between the devs... There's no new release of jest-preset-angular afaik so I'm wondering what you tested with?

jbjhjm avatar Jan 16 '23 09:01 jbjhjm