Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Angular v13 #1090

Closed
brandonroberts opened this issue Oct 18, 2021 · 43 comments · Fixed by #1122
Closed

Support Angular v13 #1090

brandonroberts opened this issue Oct 18, 2021 · 43 comments · Fixed by #1122
Labels
💣 Breaking Changes Includes a breaking change and should probably wait until we're preparing for the release of a major

Comments

@brandonroberts
Copy link

brandonroberts commented Oct 18, 2021

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

  1. Clone this repo: github.com:brandonroberts/jest-preset-angular
  2. Checkout Angular V13 branch:
git checkout angular-v13-rc
  1. Go to the examples/example-app-v13 directory and install dependencies
cd examples/example-app-v13
yarn
  1. 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
Copy link

I think the solution is to change the following line:

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

to be something like:

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

@ahnpnl
Copy link
Collaborator

ahnpnl commented Oct 18, 2021

 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
Copy link

udos86 commented Oct 18, 2021

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
Copy link

petebacondarwin commented Oct 18, 2021

It looks like the only deep import that you have is

import { Decorator, ReflectionHost, TypeScriptReflectionHost } from '@angular/compiler-cli/src/ngtsc/reflection';

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

@ahnpnl
Copy link
Collaborator

ahnpnl commented Oct 18, 2021

It looks like the only deep import that you have is

import { Decorator, ReflectionHost, TypeScriptReflectionHost } from '@angular/compiler-cli/src/ngtsc/reflection';

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
Copy link

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
Copy link

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
Copy link
Collaborator

ahnpnl commented Oct 18, 2021

According to jestjs/jest#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

@ahnpnl ahnpnl pinned this issue Oct 18, 2021
@petebacondarwin
Copy link

petebacondarwin commented Oct 18, 2021

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
Copy link
Collaborator

ahnpnl commented Oct 18, 2021

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
Copy link

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

@brandonroberts
Copy link
Author

brandonroberts commented Oct 23, 2021

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
Copy link
Collaborator

ahnpnl commented Oct 24, 2021

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
Copy link

@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
Copy link
Collaborator

ahnpnl commented Oct 24, 2021

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
Copy link
Collaborator

ahnpnl commented Oct 24, 2021

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.

@ahnpnl ahnpnl added 💣 Breaking Changes Includes a breaking change and should probably wait until we're preparing for the release of a major and removed 🐛 Bug Confirmed Bug is confirmed labels Oct 29, 2021
@ahnpnl ahnpnl changed the title [BUG]: Jest fails to run with Angular v13 RC0 Support Angular v13 Oct 29, 2021
@alan-agius4
Copy link

alan-agius4 commented Oct 29, 2021

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
Copy link
Collaborator

ahnpnl commented Oct 30, 2021

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:

@alan-agius4
Copy link

@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
Copy link

alan-agius4 commented Nov 1, 2021

@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 jestjs/jest#9771 (comment) 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
Copy link
Collaborator

ahnpnl commented Nov 1, 2021

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
Copy link

alan-agius4 commented Nov 1, 2021

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
Copy link
Collaborator

ahnpnl commented Nov 1, 2021

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
Copy link

alan-agius4 commented Nov 1, 2021

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

@tja4472
Copy link
Contributor

tja4472 commented Nov 3, 2021

Firebase recommended this resolver.

Upgrade to 9.2.0 gives error in jest unit tests:
firebase/firebase-js-sdk#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
Copy link
Collaborator

ahnpnl commented Nov 3, 2021

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
Copy link
Contributor

@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
Copy link

JoostK commented Nov 5, 2021

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

@michaelfaith
Copy link
Contributor

@JoostK Excellent! Thank you for the heads up

@michaelfaith
Copy link
Contributor

michaelfaith commented Nov 9, 2021

@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 angular/angular#44113 (comment) 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
Copy link
Collaborator

ahnpnl commented Nov 9, 2021

You can check this #1149 (comment)

@michaelfaith
Copy link
Contributor

@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
Copy link

pinich commented Nov 17, 2021

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
Copy link
Collaborator

ahnpnl commented Nov 17, 2021

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

@KonkypenT
Copy link

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?

image

@ahnpnl
Copy link
Collaborator

ahnpnl commented Dec 21, 2021

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
Copy link

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

@JaapBarnhoorn
Copy link

How did you fix it @KonkypenT?

@KonkypenT
Copy link

KonkypenT commented Dec 22, 2021

@JaapBarnhoorn , I changed the library config, copied from the example that send @ahnpnl.

image
image
image

@JaapBarnhoorn
Copy link

Thank you @KonkypenT! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💣 Breaking Changes Includes a breaking change and should probably wait until we're preparing for the release of a major
Projects
None yet
Development

Successfully merging a pull request may close this issue.