brandonroberts commented 2 years ago

Update on support Angular 13 (as of Nov 12, 2021)

The prerelease has been published (11.0.0-rc.3), anybody wanting to use with Angular 13 can run:

ng update jest-preset-angular --next

Version

10.0.1

Steps to reproduce

Clone this repo: github.com:brandonroberts/jest-preset-angular
Checkout Angular V13 branch:

git checkout angular-v13-rc

Go to the examples/example-app-v13 directory and install dependencies

cd examples/example-app-v13
yarn

Run tests

npm run test

Expected behavior

Tests run

Actual behavior

Error is thrown

> jest-preset-angular@10.0.1 test /home/robertsbt/projects/jest-preset-angular
> jest

ngcc-jest-processor: running ngcc
Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: Package subpath './ngcc/main-ngcc.js' is not defined by "exports" in /home/robertsbt/projects/jest-preset-angular/node_modules/@angular/compiler-cli/package.json
    at throwExportsNotFound (internal/modules/esm/resolve.js:290:9)
    at packageExportsResolve (internal/modules/esm/resolve.js:513:3)
    at resolveExports (internal/modules/cjs/loader.js:432:36)
    at Function.Module._findPath (internal/modules/cjs/loader.js:472:31)
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:867:27)
    at Function.resolve (internal/modules/cjs/helpers.js:94:19)
    at Object.<anonymous> (/home/robertsbt/projects/jest-preset-angular/build/utils/ngcc-jest-processor.js:22:21)
    at Module._compile (internal/modules/cjs/loader.js:1063:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1092:10)
    at Module.load (internal/modules/cjs/loader.js:928:32)

Additional context

Angular introduced the exports property in the package.json to be more strict about reaching into internal source code which may have caused this.

Environment

System:
    OS: Linux 5.4 Ubuntu 18.04.5 LTS (Bionic Beaver)
    CPU: (8) x64 Intel(R) Core(TM) i5-1035G1 CPU @ 1.00GHz
  Binaries:
    Node: 14.15.3 - ~/.nvm/versions/node/v14.15.3/bin/node
    Yarn: 1.22.5 - /usr/bin/yarn
    npm: 6.14.9 - ~/.nvm/versions/node/v14.15.3/bin/npm

petebacondarwin commented 2 years ago

I think the solution is to change the following line:

https://github.com/thymikee/jest-preset-angular/blob/209f75c42802e2e34be27aaea60620d4d406d5f2/src/utils/ngcc-jest-processor.ts#L36

to be something like:

    path.resolve(require.resolve('@angular/compiler-cli/ngcc'), 'main-ngcc.js'),

ahnpnl commented 2 years ago

 path.resolve(require.resolve('@angular/compiler-cli/ngcc'), 'main-ngcc.js'),

Thanks for the suggestion! Indeed I have

let ngccPath: string;
    try {
      ngccPath = require.resolve('@angular/compiler-cli/ngcc/main-ngcc.js');
    } catch {
      const compilerCliNgccPath = require.resolve('@angular/compiler-cli/ngcc');
      ngccPath = path.resolve(
        compilerCliNgccPath.substring(0, compilerCliNgccPath.lastIndexOf(path.sep)),
        'main-ngcc.js',
      );
    }

The error about ngcc no longer appears.

However, jest-preset-angular still needs to adopt to the change that Angular packages now defines exports field. Currently we are using @angular/compiler-cli/src/ngtsc/reflection which no longer works with exports field, as well as because of ESM shipping packages.

The workaround I could think of is copy internal codes from @angular/compiler-cli for downlevel-ctor transformer.

udos86 commented 2 years ago

I briefly would like to add here, that I ran into a ngtsc error right away after upgrading some internal Angular app to 13.0.0-rc.0, resulting from the incompatiblity between ngtsc/reflection and exports field that @ahnpnl already mentioned above.

Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: Package subpath './src/ngtsc/reflection' is not defined by "exports" in xxx\node_modules\@angular\compiler-cli\p
ackage.json
    at throwExportsNotFound (internal/modules/esm/resolve.js:299:9)
    at packageExportsResolve (internal/modules/esm/resolve.js:522:3)
    at resolveExports (internal/modules/cjs/loader.js:449:36)
    at Function.Module._findPath (internal/modules/cjs/loader.js:489:31)
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:875:27)
    at Function.Module._load (internal/modules/cjs/loader.js:745:27)
    at Module.require (internal/modules/cjs/loader.js:961:19)
    at require (internal/modules/cjs/helpers.js:92:18)
    at Object.<anonymous> (xxx\node_modules\jest-preset-angular\build\transformers\downlevel-ctor.js:5:22)
    at Module._compile (internal/modules/cjs/loader.js:1072:14)

Thank you for all your hard work!

petebacondarwin commented 2 years ago

It looks like the only deep import that you have is

https://github.com/thymikee/jest-preset-angular/blob/ac9b689d9ba18b5ae2e18e9ed083d2cbbf86b2fd/src/transformers/downlevel-ctor.ts#L11

I think that we could consider adding a field to the exports of the compiler-cli to expose these "reflection" classes.

ahnpnl commented 2 years ago

It looks like the only deep import that you have is

https://github.com/thymikee/jest-preset-angular/blob/ac9b689d9ba18b5ae2e18e9ed083d2cbbf86b2fd/src/transformers/downlevel-ctor.ts#L11

I think that we could consider adding a field to the exports of the compiler-cli to expose these "reflection" classes.

Thanks for your quick response. In fact that will only solve partially the problem. Another problem is that jest-preset-angular uses CommonJS internally due to the constraint from Jest side. I’m asking Jest team to see if we can ship our codes in ESM format.

The 2 main requirements to work with Angular 13 are:

angular exposes reflection or maybe just downlevel ctor function is good enough. I see angular 13 exposes it in @angular/compiler-cli entry point.
jest accepts 3rd party libraries (like custom transformer, custom resolver etc…) to be in ESM format.

Another concern is that, there might be other NodeJs toolings which rely on CommonJS. They might run into same issue like here if they don’t switch to use ESM mode in NodeJs. Does Angular provide a way to fallback this?

petebacondarwin commented 2 years ago

Regarding CommonJS, you could consider bundling the Angular compiler code in your UMD bundles which would resolve that problem. But it would mean that the plugin is tied tightly to the version of the compiler that you build with.

petebacondarwin commented 2 years ago

It appears that Jest plugins can use ESM if you make them async: https://jestjs.io/docs/next/code-transformation#:~:text=Note%20that%20ECMAScript,on%20the%20differences.

ahnpnl commented 2 years ago

According to https://github.com/facebook/jest/issues/11167 so it’s yes to ship jest-preset-angular in ESM syntax. I think following this path should work well with Angular 13 ESM packages

petebacondarwin commented 2 years ago

We just realised that you have made a copy of the downlevel-ctor.js file from the Angular source. And this is what needs the deep imports into Angular.

Your actual requirement is to have access to getDownlevelDecoratorsTransform(), which is actually "privately" exported already in v13 from @angular/compiler-cli/private/tooling via constructorParametersDownlevelTransform(). See https://unpkg.com/browse/@angular/compiler-cli@13.0.0-rc.0/private/tooling.d.ts

ahnpnl commented 2 years ago

Hi, yes we need access to that one constructorParametersDownlevelTransform. I just made a PR today to change internal codes https://github.com/thymikee/jest-preset-angular/blob/main/src/compiler/ng-jest-compiler.ts#L1 to import directly from @angular/compiler-cli. That should work I think?

petebacondarwin commented 2 years ago

Oh yes, because of line 10 here: https://unpkg.com/browse/@angular/compiler-cli@13.0.0-rc.0/index.d.ts

brandonroberts commented 2 years ago

I tried the latest build on main and am getting a different error now.

Must use import to load ES Module: .../ngrx/platform/node_modules/@angular/compiler-cli/bundles/index.js require() of ES modules is not supported. require() of .../ngrx/platform/node_modules/@angular/compiler-cli/bundles/index.js from .../ngrx/platform/node_modules/jest-preset-angular/build/compiler/ng-jest-compiler.js is an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which defines all .js files in that package scope as ES modules. Instead rename index.js to end in .cjs, change the requiring code to use import(), or remove "type": "module" from .../ngrx/platform/node_modules/@angular/compiler-cli/package.json.

ahnpnl commented 2 years ago

Hi, yes we fixed the issue with ngcc-jest-preprocessor as well as using directly downlevel-ctor transformer import from @angular/compiler-cli. The next step will be handling import from @angular/compiler-cli because now we cannot use require anymore since it's an ESM package.

petebacondarwin commented 2 years ago

@ahnpnl - are you able to load the import asynchronously? If so then awaiting a dynamic import expression is the best approach to get ESM from CommonJS.

ahnpnl commented 2 years ago

Yes, it is possible

(node:55511) ExperimentalWarning: VM Modules is an experimental feature. This feature could change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
[Function: constructorParametersDownlevelTransform]

I don't see any new technical challenges from Angular packages for now. Need to spend time a bit to work on refactoring codes to adapt to this.

ahnpnl commented 2 years ago

Jest only uses async transformer when running under Node ESM flag. Migrating test suites from CJS to ESM can take lots of time for big repos.

This seems to be quite a huge migration for end users.

alan-agius4 commented 2 years ago

Hi @ahnpnl, I think in that case the only viable option would be to bundle the constructorParametersDownlevelTransform which is coming from@angular/compiler-cli as part the preset.

You can do this using https://github.com/developit/microbundle. Let me know if you need any help.

ahnpnl commented 2 years ago

Hi, thank you for your help. Actually the problem is no longer the way how to use downlevel ctor transformer anymore but it’s in general about consuming public things from Angular packages.

Currently users can run tests with Jest CJS mode in which ts codes are compiled to js with module: commonjs. However, since Angular 13 introduced ESM packages, which forces users to switch to Jest ESM mode. Giving an example:

import { Component } from ‘@angular/core’;

is compiled to

const core_1 = require(‘@angular/core’);

when running Jest normally with CJS. This compiled js will no longer work with v13 as cannot use require to import ESM.

Starting from Angular 13, users are required to run Jest with NodeJs ESM enabled. When running in ESM mode, the codes above will be compiled to ESM syntax (will be the same as input codes). I’m more concerned about the migration part for users to move their test codes from CJS to ESM compatible.

alan-agius4 commented 2 years ago

@ahnpnl, I think it should be still possible to use CJS, by using https://babeljs.io/docs/en/babel-plugin-transform-modules-commonjs and configuring it as part of the Jest pipeline.

Disclaimer: I haven’t tried it.

ahnpnl commented 2 years ago

Hi, I setup a branch with a simple test https://github.com/ahnpnl/jest-preset-angular/tree/angular-v13 which failed on CI https://github.com/thymikee/jest-preset-angular/runs/4050533619?check_suite_focus=true

I noticed that, Jest module resolution couldn't find it anymore while Angular 12 still works fine. I'm not sure this is a bug for Angular.

The reason why require('@angular/core') still works in the test because I explicitly tell Jest to transform everything from @angular to cjs https://github.com/ahnpnl/jest-preset-angular/blob/angular-v13/jest.config.js#L24

The workaround for Jest module resolving issue is configuring moduleNameMapper, but imo this is not too ideal.

ahnpnl commented 2 years ago

I managed to make CJS tests passed https://github.com/thymikee/jest-preset-angular/runs/4054772234?check_suite_focus=true . I followed the suggestion from @alan-agius4 but not sure if it can be done better:

I used github-files-fetcher to download files for downlevel transformer via a Node script https://github.com/ahnpnl/jest-preset-angular/blob/angular-v13/scripts/prebuild.js and then bundle it together with this repo codes.
The most troublesome part is configuring Jest module resolution to understand new Angular package format https://github.com/ahnpnl/jest-preset-angular/blob/angular-v13/e2e/async/jest.config.js#L16 which we don’t have problems with Angular 12

alan-agius4 commented 2 years ago

@ahnpnl, I was going to look into this on Monday.

I suspect that to get ride of the module mapper workaround you might need to enable Jests’ module which would Instruct it to take the module property in the package.json of secondary entrypoints into consideration.

alan-agius4 commented 2 years ago

@ahnpnl, I did a take a look at the CJS tests and it appears that the reason why you need the moduleNameMapper is because Jests' default resolver doesn't take into consideration the module field. https://github.com/facebook/jest/blob/97e1eac39d4506e6a7a86898a083e73c5bdad37e/packages/jest-resolve/src/defaultResolver.ts#L102-L124

From the docs, it appears that one needs a custom resolve for this: https://jestjs.io/docs/configuration#resolver-string

I did monkey patch the resolver instead of implementing my own and it I was able to get rid of the module mappers

https://github.com/facebook/jest/blob/97e1eac39d4506e6a7a86898a083e73c5bdad37e/packages/jest-resolve/src/defaultResolver.ts#L102-L124

     if (filteredPkg.main != null) {      
       return filteredPkg;
     }

+    if (filteredPkg.module != null) {
+      filteredPkg.main = filteredPkg.module;
+    }

For Jest, it is also important to use FESM2015 and not FESM2020, for 2 main reasons.

Unlike browsers, FESM2020 contains syntax which is not yet supported by all Node.JS versions such as optional chaining.
Zone.JS cannot intercept native async/await which are present in ES2017+. For users wishing to use ES2017+, they will need to downlevel these using @babel/plugin-transform-async-to-generator.

Maybe we can include the resolver as part of this preset?

Edit: Sadly, from https://github.com/facebook/jest/issues/9771#issuecomment-946052045 it also appears that Jest, will never really respect the module field out-of-the-box. This can eventually also be problematic for other non-Angular dependencies which shift towards shipping packages in ESM only.

ahnpnl commented 2 years ago

Thanks a lot @alan-agius4 !! I was suspecting that a custom resolver seems to be the only way to solve moduleNameMapper config in a better way. I think yes it makes sense to include a custom resolver in this repository.

Indeed I was confused between FESM2015 vs FESM2020 and I was thinking FESM2015 seems to be a better fit, which is related to #1058 which was explained by your team as well. Thanks for clarifying this!

Does internal Angular CLI use any custom resolvers for webpack to deal with module field? If yes, it would be nice if you can reference me there so I can implement a similar resolver for Jest.

alan-agius4 commented 2 years ago

Does internal Angular CLI use any custom resolvers for webpack to deal with module field? If yes, it would be nice if you can reference me there so I can implement a similar resolver for Jest.

No, we don't have any custom resolver. The tools used by the Angular CLI, Webpack, Node.js and Rollup (Which are used in different pipeline and phases) all support package exports and the module field.

For Webpack we only provide the fields and the order that the resolver should consider.

https://github.com/angular/angular-cli/blob/ebccb5de4a455af813c5e82483db6af20666bdbd/packages/angular_devkit/build_angular/src/webpack/configs/common.ts#L302-L305

I think in your case, the resolver should look like the below

module.exports = (request, options) =>
  options.defaultResolver(request, {
    ...options,
    packageFilter: (pkg) => ({
      ...pkg,
      main: pkg.main || pkg.es2015 || pkg.module,
    }),
  });

ahnpnl commented 2 years ago

that's simpler than I expected. Thanks again. I will test out this resolver. A quick test I could see that e2e test passed 👍

alan-agius4 commented 2 years ago

Feel free to ping me if you need any help or have any questions. You can also reach me on Twitter.

tja4472 commented 2 years ago

Firebase recommended this resolver.

Upgrade to 9.2.0 gives error in jest unit tests: https://github.com/firebase/firebase-js-sdk/issues/5687

// my-module-resolve.js
module.exports = (request, options) => {
  // Call the defaultResolver, so we leverage its cache, error handling, etc.
  return options.defaultResolver(request, {
    ...options,
    // Use packageFilter to process parsed `package.json` before the resolution (see https://www.npmjs.com/package/resolve#resolveid-opts-cb)
    packageFilter: pkg => {
      if(pkg.name.startsWith('@firebase')) {
        return {
          ...pkg,
          // Alter the value of `main` before resolving the package
          main: pkg.esm5 || pkg.module,
        };
      }

      return pkg;
    },
  });
};

module.exports = {
  ....
  // set the value to the custom resolver created in step 1
  resolver: '<rootDir>/my-module-resolve.js',
  // browser bundles in firebase are ESM, transform them to CJS to work in Jest
  transformIgnorePatterns: [
    "<rootDir>/node_modules/(?!(@firebase.*)/)"
  ]
};

ahnpnl commented 2 years ago

Thanks for your input @tja4472, but here we are making a generic resolver for Angular 13, not specific to @firebase case. Of course end users feel free to copy the generic resolver of this preset and extend on their own to fit with their usage.

michaelfaith commented 2 years ago

@ahnpnl I'm not sure if this is an issue with the configuration, or with the Angular Material library, but we're seeing failures to import on a few of their components (but oddly not all).

Test suite failed to run

    ReferenceError: Cannot access 'MatButtonToggle' before initialization

       6 |
       7 | import { ComponentFixture, TestBed } from '@angular/core/testing';
    >  8 | import { MatButtonToggleModule } from '@angular/material/button-toggle';
         | ^
       9 | import { MatIconModule } from '@angular/material/icon';
      10 |
      11 | import { ButtonToggleDemoComponent } from './button-toggle-demo.component';

      at Object.<anonymous> (../../node_modules/@angular/node_modules/@angular/material/fesm2015/button-toggle.mjs:254:63)
      at Object.<anonymous> (src/app/component-demos/button-toggle-demo/button-toggle-demo.component.spec.ts:8:1)

Does this look like something that would be a configuration issue (this project), or something more on their side?

JoostK commented 2 years ago

@michaelfaith That's a bug in Angular's partial compilation output. @petebacondarwin is working on a fix in the Angular compiler.

michaelfaith commented 2 years ago

@joostk Excellent! Thank you for the heads up

michaelfaith commented 2 years ago

@ahnpnl I'm seeing a slightly different issue with testing of a library built with Angular 13. Similar to Material, these errors are popping up in Jest unit tests that involve components from the library, but it runs fine with ng serve (aot). According to https://github.com/angular/angular/pull/44113#issuecomment-964199439 this doesn't appear to be related to the compiler issue currently being worked on affecting Angular Material. Are there additional config changes needed in order to test Angular 13 compiled libraries?

Details:

    C:\Users\<user>\src\fabric-starter\node_modules\@fabric\components\fesm2015\fabric-components-breadcrumbs.mjs:1
    ({"Object.<anonymous>":function(module,exports,require,__dirname,__filename,jest){import * as i4 from '@angular/common';
                                                                                      ^^^^^^

    SyntaxError: Cannot use import statement outside a module

      10 | import { BrowserAnimationsModule } from '@angular/platform-browser/animations';
      11 | import { RouterTestingModule } from '@angular/router/testing';
    > 12 | import { FabBreadcrumbsModule } from '@fabric/components/breadcrumbs';
         | ^
      13 | import { FabHeaderModule } from '@fabric/components/header';

      at Runtime.createScriptFromCode (node_modules/jest-runtime/build/index.js:1728:14)
      at Object.<anonymous> (src/app/app.component.spec.ts:12:1)

and

 Details:

    C:\Users\<user>\src\fabric-starter\node_modules\@fabric\components\fesm2015\fabric-components-utilities.mjs:196
    export { compareById, convertDateTimeToTimestamp, doesContainOnlyStrings, findObjWithKey, getCoreRoute, isNotNull, isNotNullOrUndefined, isNull, isNullOrUndefined, isObject, isUndefined, removeIntersection, removeItemFromArray,
removeLeadingSlash, removeQueryParams, replaceAll };
    ^^^^^^

    SyntaxError: Unexpected token 'export'

       7 | import { Injectable } from '@angular/core';
    >  8 | import { isNotNull } from '@fabric/components/utilities';
         | ^
        9 | import { AuthenticationService } from '../authentication/authentication.service';
      10 |
      11 | /**

      at Runtime.createScriptFromCode (node_modules/jest-runtime/build/index.js:1728:14)
      at Object.<anonymous> (src/app/core/user/user.service.ts:9:1)

Comment from @JoostK:

@michaelfaith that does not look related to what is being fixed here, it seems. I believe that Jest creates a CommonJS wrapper for any module it loads, in order to sandbox things like require and module and the like. That does require however that CommonJS code is used, whereas it appears that in your case the ESM ends up within the CommonJS wrapper. That is invalid, as ESM must be top-level.

ahnpnl commented 2 years ago

You can check this https://github.com/thymikee/jest-preset-angular/issues/1149#issuecomment-963506942

michaelfaith commented 2 years ago

@ahnpnl perfect! That did the trick. I knew you'd have the solution. Thanks a ton. And for working through this upgrade with us all. Really appreciate it. You as well @JoostK. You guys rock.

pinich commented 2 years ago

You closed this issue but the problem remains, I'm facing the following error and I can't find any workaround for it.

  ● Test suite failed to run

    Error [ERR_REQUIRE_ESM]: require() of ES Module C:\...\node_modules\@angular\compiler-cli\bundles\index.js from C:\...\node_modules\jest-preset-angular\build\compiler\ng-jest-compiler.js not supported.
    Instead change the require of index.js in C:\...\node_modules\jest-preset-angular\build\compiler\ng-jest-compiler.js to a dynamic import() which is available in all CommonJS modules.

      at Object.<anonymous> (node_modules/jest-preset-angular/build/compiler/ng-jest-compiler.js:4:24)
      at ScriptTransformer._getTransformer (node_modules/@jest/transform/build/ScriptTransformer.js:347:21)
      at ScriptTransformer.transformSource (node_modules/@jest/transform/build/ScriptTransformer.js:427:28)
      at ScriptTransformer._transformAndBuildScript (node_modules/@jest/transform/build/ScriptTransformer.js:569:40)
      at ScriptTransformer.transform (node_modules/@jest/transform/build/ScriptTransformer.js:607:25)

ahnpnl commented 2 years ago

@pinich please use next tag, we haven’t published official release yet.

KonkypenT commented 2 years ago

I am still unable to run tests on angular 13. I am using the latest version of the library, as written in this thread. Please tell me if this is a configuration or angular issue?

ahnpnl commented 2 years ago

I think you missed some configuration. You can take a look at our example app https://github.com/thymikee/jest-preset-angular/tree/main/examples/example-app-v13

KonkypenT commented 2 years ago

I think you missed some configuration. You can take a look at our example app https://github.com/thymikee/jest-preset-angular/tree/main/examples/example-app-v13

Great! Thanks a lot! The problem was in the configuration