Imported jest in mock

[nicolo-ribaudo]

Summary

This PR:

• Fixes the detection of jest.* refs to imported jest in non-top-level contexts
• Transforms all the jest.* refs inside hoisted code to the "lazy" version

Fixes #12422

Test plan

Tests added

---

I have two questions:

1. This test doesn't make sense to me, since imports are hoisted and thus the jest import is already hoisted, even without any transform:

   import {jest} from '@jest/globals';
   jest.mock('some-module', () => {
     jest.mock('some-module');
   });
         ↓ ↓ ↓ ↓ ↓ ↓
   _getJestObj().mock('some-module', () => {
     _getJestObj().mock('some-module');
   });
   function _getJestObj() {
     const {jest} = require('@jest/globals');
     _getJestObj = () => jest;
     return jest;
   }
   import {jest} from '@jest/globals';

   What's the reason to transform it?

2. Can we always transform jest to _getJestObj, so that we don't have to check if it's hoisted or not and we can simplify the plugin?

3. Since require is cached by default, can we use require('@jest/globals').jest directly instead of _getJestObj()?

[SimenB]

1. This test doesn't make sense to me, since imports are hoisted and thus the jest import is already hoisted, even without any transform:

Normally this would also transform the import to require - that transform is left out for brevity (original reproduction shows a more real use case with preset-env: https://github.com/mrazauskas/x-jest-globals/blob/62d5220129a591defce158c2526b3ba49d23d0bb/babel.config.js#L3). The test is mainly that you're allowed to access jest within the mock closure - i.e. buypass the "you cannot access non-global stuff here as it's hoisted" check

2. Can we always transform jest to _getJestObj, so that we don't have to check if it's hoisted or not and we can simplify the plugin?

Yes (I think). Caveat is it should only be hoisted within its function scope, not (necessarily) to the top

3. Since require is cached by default, can we use require('@jest/globals').jest directly instead of _getJestObj()?

Yeah, that should be fine.

[SimenB]

@nicolo-ribaudo are there more work you'd want to do here, or is it ready to merge? 🙂

[nicolo-ribaudo]

This is ready, but I'll follow up with another PR for those small simplifications :)

[SimenB]

Awesome, thanks!

[nicolo-ribaudo]

This was probably not intended

[SimenB]

Tried editting on mobile, crashed chrome 😅 should probably move old releases to a separate changelog

[SimenB]

Note to self: trying to update a huuuuge changelog on mobile (while getting tattooed) does not work at all 😅

[github-actions]

This pull request 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.