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

[Bug]: referencing jest inside jest.mock block throws, if jest is imported from @jest/globals #12422

Closed
mrazauskas opened this issue Feb 18, 2022 · 10 comments · Fixed by #13188
Closed

Comments

@mrazauskas
Copy link
Contributor

Version

28.0.0-alpha.3

Steps to reproduce

Here is a repo with minimal reproduction: https://github.com/mrazauskas/x-jest-globals

I simply added explicit import to the example from docs (and used TS to trigger transpiration) – https://jestjs.io/docs/next/mock-functions#mocking-partials

Running yarn jest will throw an ReferenceError:

Screenshot 2022-02-18 at 09 38 22

Expected behavior

It should not throw ReferenceError.

Actual behavior

It throws ReferenceError, probably because of transpilation. I also tried transpiling using ts-jest and the error was gone. It seems like only users of babel-jest are affected.

Additional context

No response

Environment

System:
    OS: macOS 10.15.7
    CPU: (8) x64 Intel(R) Core(TM) i7-3615QM CPU @ 2.30GHz
  Binaries:
    Node: 17.3.1 - ~/.nvm/versions/node/v17.3.1/bin/node
    Yarn: 1.22.15 - ~/.nvm/versions/node/v17.3.1/bin/yarn
    npm: 8.3.0 - ~/.nvm/versions/node/v17.3.1/bin/npm
  npmPackages:
    jest: ^28.0.0-alpha.3 => 28.0.0-alpha.3
@mrazauskas
Copy link
Contributor Author

mrazauskas commented Feb 18, 2022

@SimenB As I was mentioning, if an import { jest } from "@jest/globals"; is present in a file Babel playground is transpiling jest.requireActual(); to _globals.jest.requireActual(); Have to double check if it is the same here, but should be similar.

In this case it might be enough to add _globals.jest to this list, right?

https://github.com/facebook/jest/blob/efc9e67152721aa9571ecc95c5bb0e6184baffdf/packages/babel-plugin-jest-hoist/src/index.ts#L85-L88

@SimenB
Copy link
Member

SimenB commented Feb 18, 2022

@nicolo-ribaudo any tips here? 🙂 The hoist plugin transforms the jest object to _globals.jest, then complains it's an illegal access 🙈

Failing unit test:

diff --git i/packages/babel-plugin-jest-hoist/src/__tests__/hoistPlugin.test.ts w/packages/babel-plugin-jest-hoist/src/__tests__/hoistPlugin.test.ts
index c6ad87de81..b03ff10728 100644
--- i/packages/babel-plugin-jest-hoist/src/__tests__/hoistPlugin.test.ts
+++ w/packages/babel-plugin-jest-hoist/src/__tests__/hoistPlugin.test.ts
@@ -90,6 +90,17 @@ pluginTester({
       formatResult,
       snapshot: true,
     },
+    'imported jest within jest': {
+      code: formatResult(`
+        import {jest} from '@jest/globals';
+
+        jest.mock('some-module', () => {
+          jest.mock('some-module');
+        });
+      `),
+      formatResult,
+      snapshot: true,
+    },
   },
   /* eslint-enable */
 });

If it's not an import, it's transformed into a function, maybe we should do the same for the import?

@nicolo-ribaudo
Copy link
Contributor

When you are checking if a variable is safe to use, you could get binding = path.scope.getBinding(varName).

  • If it's undefined, it's a global (so probably it's safe?)
  • Otherwise, check if binding.path is a variable declaration whose RHS is a call expression calling require with "@jest/globals".

An alternative is to remove the @jest/globals import before that the module transform runs, and the add it back in post().

@SimenB
Copy link
Member

SimenB commented Feb 21, 2022

Awesome, thanks @nicolo-ribaudo!

@SimenB
Copy link
Member

SimenB commented Feb 21, 2022

I've found another issue while playing with this - we only replace calls to jest.mock. So

jest.mock('some-module', () => {
  jest.requireActual('some-module');
});

is transformed into

_getJestObj().mock('some-module', () => {
  jest.requireActual('some-module');
});

function _getJestObj() {
  const {jest} = require('@jest/globals');

  _getJestObj = () => jest;

  return jest;
}

Notice how the inner jest.requireActual is not _getJestObj().requireActual.

Diff with both tests (first fails due to the check discussed here, and the other produces the wrong result)

diff --git i/packages/babel-plugin-jest-hoist/src/__tests__/hoistPlugin.test.ts w/packages/babel-plugin-jest-hoist/src/__tests__/hoistPlugin.test.ts
index c6ad87de81..950ff71647 100644
--- i/packages/babel-plugin-jest-hoist/src/__tests__/hoistPlugin.test.ts
+++ w/packages/babel-plugin-jest-hoist/src/__tests__/hoistPlugin.test.ts
@@ -90,6 +90,26 @@ pluginTester({
       formatResult,
       snapshot: true,
     },
+    'imported jest within jest': {
+      code: formatResult(`
+        import {jest} from '@jest/globals';
+
+        jest.mock('some-module', () => {
+          jest.requireActual('some-module');
+        });
+      `),
+      formatResult,
+      snapshot: true,
+    },
+    '"global" jest within jest': {
+      code: formatResult(`
+        jest.mock('some-module', () => {
+          jest.requireActual('some-module');
+        });
+      `),
+      formatResult,
+      snapshot: true,
+    },
   },
   /* eslint-enable */
 });

@SimenB SimenB changed the title [Bug]: referencing jest inside jest.mock block throws, if jest is imported from @jest/globals in a TS test file [Bug]: referencing jest inside jest.mock block throws, if jest is imported from @jest/globals Feb 21, 2022
@github-actions
Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 30 days.

@ScottAwesome
Copy link

I've ran into this as well, any thoughts on how we can get around the issue? The workaround I currently use is this

import { jest, test, expect } from '@jest/globals';
import b from './b'
import a from '../module';

jest.mock('../module');

test('some test', () => {
    a.mockImplementationOnce(() => true);
    expect(b()).toBe(false);
})

contrived example, but it does work.

Its a little bit more work than just mocking the module out directly, it is more work overall. I haven't hit any gotchas with named imports though, so it does seem like a solid workaround

@SimenB
Copy link
Member

SimenB commented Aug 28, 2022

@ScottAwesome I think something like this should work:

import {jest, jest as mockJest} from '@jest/globals';

jest.mock('some-module', () => {
  mockJest.mock('some-module');
});

@SimenB
Copy link
Member

SimenB commented Sep 3, 2022

@github-actions
Copy link

github-actions bot commented Oct 4, 2022

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants