Jest/ESBuild mocking JS modules

[gultyayev]

Command

test

Is this a regression?

• [ ] Yes, this behavior used to work in the previous version

The previous version in which this bug was not present was

No response

Description

With Jest Angular started to use ES Build internally. This causes ES Modules in the output with non-configurable methods/properties. As a result, we cannot mock JS modules. This leads to tests that pull lots of external dependencies (utility functions, config objects etc.) that are not provided using the DI and hence cannot be stubbed using Angular DI.

Minimal Reproduction

Scaffold an app and use Jest as a testing tool. Modify AppComponent.ngOnInit

ngOnInit() {
  if (isDevMode())  {
    console.log('dev');
  }
}

In the test try to stub the isDevMode.

jest.mock('@angular/core', () => ({
    ...jest.requireActual('@angular/core'),
    isDevMode: jest.fn().mockReturnValue(false)
}))

describe('AppComponent', () => {
    let component: AppComponent;

    beforeEach(() => {
        component = new AppComponent()
    })

    describe('ngOnInit', () => {
        it('should not log when in prod mode', () => {
            const spy = jest.spyOn(console, 'log')
            component.ngOnInit()
            expect(spy).toHaveBeenCalled()
        });
    })
})

Exception or Error

Test fail due to a number of reasons which are related to ES Build in the end:
a. ES Build outputs a module with a different name and there is a mismatch
b. ES Build embeds a module (e.g. import of the environments file)
c. JS runtime error that you try to change the property that cannot be configurable. This happens if you try to import the module in the tests and spy on it.

import * as module from 'module'

jest.spyOnProperty(module, 'method').mockReturnValue(...)

### Your Environment

```text
Angular CLI: 16.0.5
Node: 18.16.1
Package Manager: npm 9.5.1
OS: darwin arm64

Angular: 
... 

Package                      Version
------------------------------------------------------
@angular-devkit/architect    0.1600.5 (cli-only)
@angular-devkit/core         16.0.5 (cli-only)
@angular-devkit/schematics   16.0.5 (cli-only)
@schematics/angular          16.0.5 (cli-only)

Anything else relevant?

This feels like a big deal, because most of the current state tests will become invalid. Furthermore, current behavior makes unit tests more of the integration sorts.

[JeanMeche]

Note: There is a related discussion at angular/angular#50756.

[gultyayev]

I believe this one is not quite related. It's about how Zones are configured. But here the problem is global. It's about literally every package that you import. Even if you import a function that you have created in a separate file. You still cannot mock it. If ES Build could have been configured to make exports configurable for tests it should have solved that. Otherwise, this path seems rather dangerous to me and using some custom Jest support over good ol' commonjs would be the only working solution.

[johncrim]

@gultyayev - have you tried using jest's Module Mocking in ESM for this use case?

jest.mock() requires commonjs. IMO the angular team should state that commonjs support is out of scope with this builder. This jest builder and the use of ESBuild are valuable thanks to build performance and eliminating the mess/hassle of jest config (jest ESM config is particularly challenging), so I think it's reasonable to exclude such jest features.

[dgp1130]

jest.mock is unfortunately quite complicated and reliant on CommonJS. The main challenge is that you need to run jest.mock prior to importing the mocked module, which is harder than it sounds. ESM format inherently hoists import statements to the top of the file so even the following does not work:

jest.mock('foo');

import { bar } from 'foo'; // Hoisted, happens before `jest.mock`.

There are two ways I'm aware of to get this mock to work. The first is to use CommonJS, which does not hoist require calls:

jest.mock('foo');

const { bar } = require('foo'); // No hoisting.

It sounds like we could fix this by converting authored ESM code into CommonJS tests, however even that doesn't work. ESM format requires this hoisting, so a correct transformation of ESM into CommonJS would also necessitate hoisting the require calls. The only way to put jest.mock before an import is to actually author CommonJS, not just transpile to it.

The second approach is to use a dynamic import, which also isn't subject to hoisting:

jest.mock('foo');

const { bar } = await import('foo'); // Also no hoisting.

Jest does have a built-in transform which automatically hoists jest.mock above require calls, however AFAICT this is only enabled for CommonJS. An alternative approach would be to transform static imports into dynamic imports like so:

jest.mock('foo');

// Transformed from:
// import { bar } from 'foo';
// import { Component } from '@angular/core';

const [ { bar }, { Component } ] = await Promise.all([
  import('foo'),
  import('@angular/core'),
]);

Those are pretty invasive changes and I'm not totally convinced they're the right path forward here. The Jest team is evaluating this approach but hasn't done it yet and doesn't seem to consider jest.mock support a hard blocker for initial ESM support: https://github.com/jestjs/jest/issues/9430#issue-551921631 (see jest.(do|un)mock line item).

Even if those timing issues were fixed, we're currently exploring pre-bundling Jest tests, meaning the import of foo is actually something like ./chunk-abc123.js at runtime (or possibly no import at all). So jest.mock('foo') wouldn't be the right thing to mock. spyOn should generally work as expected because that takes a reference, not a module specifier, though I think it's not possible to spy on imported properties directly because ESM exports are not configurable.

The ideal solution is probably to use Angular's dependency injection to stub out any dependencies you need to mock, though I get that can be a big mental shift for a lot of developers used to using these tools outside of Angular.

For now I'm inclined to follow Jest's lead and leave jest.mock unsupported for the time being. If they come up with an alternative approach or decide to move forward with a transform to fix the hoisting problem, then I think we can re-evaluate. That still wouldn't solve the pre-bundling challenge, but I think this is one of the trade-offs we'll need to weigh as we're experimenting with that approach.

This will likely be confusing for users to have jest.mock appear to work but functionally be a no-op. I'm not aware of a Jest option to explicitly disable it. We could consider injecting something like:

jest.mock = () => {
  throw new Error('jest.mock is unsupported, see: https://github.com/angular/angular-cli/issues/25582');
};

However that would break for legitimate dynamic import use cases which could otherwise work, so I'm not convinced that's the best solution either.

I expect this will be a common challenge for Jest users, so I'll leave this issue open just to collect feedback about this particular rough edge. I suspect the best thing we can do for now though is hope the Jest team is able to come up with a more elegant solution for ESM module mocking which we can better apply to Angular.