ts-jest does not hoist variables beginning with mock

[GeeWee]

Issue :

Jest docs describe that all variables that begin with mock are hoisted to the top of the file, above any jest.mock calls:
https://jestjs.io/docs/en/es6-class-mocks#calling-jestmock-docs-en-jest-object-jestmockmodulename-factory-options-with-the-module-factory-parameter

Currently it seems like our implementation of hoist only hoists jest.mock and jest.unmock calls: https://github.com/kulshekhar/ts-jest/blob/a6bcec48ef28791235b3eba0b3f2bd1a944ba5b8/src/transformers/hoist-jest.ts

We should further implement it so that it hoists all variables called something with 'mock' to the top

[tonyhallett]

Jest does not describe that all variables that begin with mock are hoisted, well it certainly does not now. It does not hoist variables with name that begins with mock.

A limitation with the factory parameter is that, since calls to jest.mock() are hoisted to the top of the file, it's not possible to first define a variable and then use it in the factory. An exception is made for variables that start with the word 'mock'.

The exception is that there will be no error thrown.

Below is the code for the hoisting :

export default () => {
  const shouldHoistExpression = (expr: NodePath): boolean => {
    if (!expr.isCallExpression()) {
      return false;
    }

    const callee = expr.get('callee');
    const expressionArguments = expr.get('arguments');
    // TODO: avoid type casts - the types can be arrays (is it possible to ignore that without casting?)
    const object = callee.get('object') as NodePath;
    const property = callee.get('property') as NodePath;
    return (
      property.isIdentifier() &&
      FUNCTIONS[property.node.name] &&
      (object.isIdentifier(JEST_GLOBAL) ||
        (callee.isMemberExpression() && shouldHoistExpression(object))) &&
      FUNCTIONS[property.node.name](expressionArguments)
    );
  };

  const visitor: Visitor = {
    ExpressionStatement(path) {
      if (shouldHoistExpression(path.get('expression'))) {
        // @ts-ignore: private, magical property
        path.node._blockHoist = Infinity;
      }
    },
  };

  return {visitor};
};

Note the if (!expr.isCallExpression()) { - the only statements that get hoisted are statements of the form jest.mock/unmock/disableAutomock/enableAutomock/deepUnmock with appropriate arguments. ( Not showing mock )

FUNCTIONS.unmock = args => args.length === 1 && args[0].isStringLiteral();
FUNCTIONS.deepUnmock = args => args.length === 1 && args[0].isStringLiteral();
FUNCTIONS.disableAutomock = FUNCTIONS.enableAutomock = args =>
  args.length === 0;

The code for

An exception is made for variables that start with the word 'mock'.

Can be found here

Also note that code of the form
jest.mock('one').mock('two');
will be hoisted but ( taken from babel-plugin-jest-hoist integration test )
jest.unmock('../__test_modules__/a').dontMock('../__test_modules__/b');
will not.

This leads me to actual differences between jest and ts-jest.

ts-jest does not throw on out of scope variables.
ts-jest does not check the arguments for jest.mock etc.

The two differences are important and the source code for ts-jest should be changed. I am having issues with vscode wsl installation of ts-jest so I cannot create the changes but here are the details.

1. ts-jest does not hoist deepUnmock.
   Just add deepUnmock to the array

2. ts-jest does not work with chained method calls.
   Change the shouldHoistNode method to

function shouldHoistExpression(expression: Node): boolean {
    return (
      ts.isCallExpression(expression) &&
      ts.isPropertyAccessExpression(expression.expression) &&
      HOIST_METHODS.includes(expression.expression.name.text) &&
      ((ts.isIdentifier(expression.expression.expression) && expression.expression.expression.text === 'jest') ||
        shouldHoistExpression(expression.expression.expression))
    )
  }

function shouldHoistNode(node: Node): node is ExpressionStatement {
    return ts.isExpressionStatement(node) && shouldHoistExpression(node.expression)
  }

Of course the tests need to change.
The unit test
Note that even if you do not apply these changes the test name should change from

should hoist jest mock() and unmock() statements

to

should hoist jest.mock(), unmock(), disableAutomock() and enableAutomock()

The end to end test does not test disableAutomock or enableAutomock currently. Also afterAll does nothing useful.

If you are interested I created compare-jest-tsjest-hoisting. This transforms typescript with ts-jest and javascript with babel and babel-preset-jest ( which provides babel-plugin-jest-hoist ) with their APIs and compares the two within a jest test. With the changes mentioned ts-jest and jest will be more aligned and you can see that this is so with this repo.

[salolivares]

Facing the same issue. My current work around is to use jest.doMock() since it does not automatically hoist to the top.

const mockPlaySoundFile = jest.fn();
jest.doMock('./sound-player', () => {
  return jest.fn().mockImplementation(() => {
    return {playSoundFile: mockPlaySoundFile};
  });
});
import SoundPlayer from './sound-player';
*any other import that uses soundplayer*

[G-Rath]

@ahnpnl I'm a bit confused by this, as it seems like ts-jest still doesn't hoist variables that start with the word "mock".

From what I can tell that's a bug in the hoist-jest ast transformer, but I'm a bit confused by @tonyhallett's phrasing, specifically he said:

Jest does not describe that all variables that begin with mock are hoisted, well it certainly does not now. It does not hoist variables with name that begins with mock.

Except that jest does hoist variables whose name begins with "mock".

Currently this code is erroring for me:

const mockPostMessageFn = jest.fn();

jest.mock('@slack/web-api', () => {
  return {
    WebClient: class {
      public chat = {
        postMessage: mockPostMessageFn
      };
    }
  };
});

    ReferenceError: Cannot access 'mockPostMessageFn' before initialization

      20 |     WebClient: class {
      21 |       public chat = {
    > 22 |         postMessage: mockPostMessageFn
         |                      ^
      23 |       };
      24 |     }
      25 |   };

I'm happy to make a PR modifying the hoisting transformer to fix this, but didn't want to open up a new issue if this has already been decided on, since theres already been a few.

I do think this should be documented at the very least, to help avoid confusion :)

[ahnpnl]

hi @G-Rath, it would be very nice that you can help ts-jest with this hoisting fix. The recent change from jest related to how babel-jest hoist certainly changes something related to @jest/global too. Maybe you can tackle that as well ?

The whole point from @tonyhallett is making ts-jest have the same hoisting behavior as babel-jest. Now I could see that his solution only solved the issue with deepUnmock but not with mock.

Look like ts-jest hoist unit test isn't good enough to cover the case you have.

[G-Rath]

Awesome - feel free to assign this issue to me if you like :)
I already have a good idea on the way to tackle this, so I'll do it tomorrow (since it's midnight here in NZ).

certainly changes something related to @jest/global too.

Not too sure what you mean, but what I'll do is just copy the code examples we use in the jest codebase for testing the hoisting into the test suite for ts-jest, and iterate until they pass.

[ahnpnl]

Awesome - feel free to assign this issue to me if you like :)
I already have a good idea on the way to tackle this, so I'll do it tomorrow (since it's midnight here in NZ).

Anytime you feel comfortable with :)

certainly changes something related to @jest/global too.

Not too sure what you mean, but what I'll do is just copy the code examples we use in the jest codebase for testing the hoisting into the test suite for ts-jest, and iterate until they pass.

There was a discussion in jestjs/jest#9806 which I suppose need to be fixed for ts-jest hoisting as well.

[G-Rath]

Looking into this further, I've realised that @tonyhallett is completely correct, and what he was meaning: jest doesn't hoist variables that start with mock.

What happens is that when doing the hoisting for mock, the babel plugin checks variables that are being used in the mock factory to ensure that everything being used will be within scope after hoisting.

So if it finds a variable (such "myFakeTimer") it'll throw; it's effectively doing a little bit of what TypeScript does in general, to be helpful given that this hoisting is happening in the background & so not obvious.

However as an advanced escape hatch, the babel plugin allows variables that start with mock as a way to tell jest that you know what you're doing.

ts-jest doesn't implement any of the above in it's transformer, which I think is "ok": it would be cool to have, but also would require a fair bit of extra work since it's a little complex & babel plugins don't map 1:1 to TypeScript transformers. (so aka I think maybe break it out into a feature-request issue?)

But so tl;dr this bug isn't a bug - it's actually an (understandable) misinterpretation of the docs: jest doesn't hoist variables starting with "mock", it only ignores them when checking in hoisted jest.mock factories for now-out-of-scope variable usage.

---

Meanwhile, I've brought the tests used by the babel plugin over into ts-jest (it's literally just all the files in __tests__ & __test_modules__ renamed to end with .spec.ts & // @ts-nocheck stuck in them).

So far they seem to just be working out of the box after I added react as a dependency, so that's pretty cool. It also shows that the transformer does not match the babel plugin 1:1.

For example, it's failing on scopes:

{
  const jest = { unmock: () => {} }
  // Would error (used before initialization) if hoisted to the top of the scope
  jest.unmock('../__test_modules__/a') // ts-jest is hoisting this to the top of the scope, 💥 
}

That makes sense as this is change required for supporting @jest/globals, which I believe is now the main change that needs to be implemented in the transformer.

It might make sense to close this issue & open a new one to track the adding of support for @jest/globals 🤔

[ahnpnl]

Thank you for a very clear explanation. I will close this issue and please feel free to open a new issue to support @jest/global 👌

I was suspecting that it is required to support @jest/global when I see jest team working on that for Babel plugin, now you confirm then implementing the similar behavior for ts-jest makes sense.

[SimenB]

it's actually an (understandable) misinterpretation of the docs

PR to clarify docs very much welcome 🙂

[ahnpnl]

support hoisting with @jest/global for ts-jest in #1593 :) Would love to get some helps there