jest.useFakeTimers() stuck the test in MSW 2

[joel-daros]

Prerequisites

• I confirm my issue is not in the opened issues
• I confirm the Frequently Asked Questions didn't contain the answer to my issue

Environment check

• I'm using the latest msw version
• I'm using Node.js version 18 or higher

Node.js version

v18.17.1

Reproduction repository

https://github.com/joel-daros/msw2-text-encoder-issue

Reproduction steps

Clone the repo and run:

npm install --legacy-peer-deps
npm run test

Current behavior

After adding this line, the test stuck, and the only way to finishing is aborting the jest execution:

  // when this line is added, the test stuck and never finishes
  jest.useFakeTimers({ now: new Date(2023, 9, 15), doNotFake: ["setTimeout"] });

I noticed this issue has been addressed here in MSW 1.x (and actually works well in 1.x version), but seems like it's back in MSW 2.x with the addition of the jest.pollyfills.js file.

Expected behavior

jest.useFakeTimers() should be work in MSW 2.

[jeffwong14]

Looks similar issue happens to me in #1829

My Code Sandbox stucks at await response.json() if using fakeTimers

And the issue gone if I remove those fakeTimers code

[mattcosta7]

Looks similar issue happens to me in #1829

My Code Sandbox stucks at await response.json() if using fakeTimers

And the issue gone if I remove those fakeTimers code

A little attempt at some initial debugging:

    let x = resp.json();
    jest.runOnlyPendingTimers();
    x = await x;

I modified like this, to see if we could run it once jest progressed timers, and we can but this failed with a json parsing error.

SyntaxError: Unexpected non-whitespace character after JSON at position 20

    let x = resp.text();
    jest.runOnlyPendingTimers();
    x = await x;

this shows a response that's wrong, which is interesting

{"result":"success"}{"result":"success"}

which doubles the response in the body.

(just leaving these breadcrumbs for debugging further)

[mattcosta7]

With legacy fake timers it appears to work properly:

  jest.useFakeTimers({
    now: hardCodeNow,
    legacyFakeTimers: true,
  });
//
    let x = await resp.json();

which I discovered as a setting from this: nock/nock#2200

this config option in jest is

  /**
   * Use the old fake timers implementation instead of one backed by `@sinonjs/fake-timers`.
   * The default is `false`.
   */
  legacyFakeTimers?: boolean;

which makes it seem like there's some issue related to @sinonjs/fake-timers somewhere, since it works with the other implementation, with no other changes. It seems like they recently started mocking the timers module, and I wonder if this is causing issues somehow?

[mattcosta7]

Found another workaround you can see here

  jest.useFakeTimers({
    now: hardCodeNow,
    doNotFake: ["queueMicrotask"],
  });

It looks like it's related to this issue - nodejs/undici#1251 in undici, which is really less of an undici issue and more of a "jest does some things in not great ways" issue.

As long as you don't need to fake calls to queueMicrotask this should be a viable workaround that maintains the 'modern' fake timer implementation

So in summary - I don't believe msw can directly solve this issue, but we can suggest that when using fake timers in jest the configuration be extended to either

1. use legacy fake times

  jest.useFakeTimers({
    now: hardCodeNow,
    legacyFakeTimers: true,
  });

2. avoid faking queueMicrotask

  jest.useFakeTimers({
    now: hardCodeNow,
    doNotFake: ["queueMicrotask"],
  });

Both of these configurations seem to work properly

[mattcosta7]

Since i'm not sure that msw can effectively fix this directly and there are at least 2 workarounds, I'm going to close this one - but please let me know if neither of those 2 help to resolve the issue which stems from how jest and undici (the node fetch library) interact.

[mattcosta7]

@kettanaito - do you think it might be wise add some documentation here: https://mswjs.io/docs/runbook for this?

If so (or if you think there's a better place) I'm happy to write up a draft

[kettanaito]

@mattcosta7, thanks for looking into this! Yes, I believe the Runbook is the best place for this. Please, would you have a minute to open a pull request with the doNotFake approach? I think we shouldn't influence whether the developer uses legacy/modern timers. Let them decide.

[mattcosta7]

@mattcosta7, thanks for looking into this! Yes, I believe the Runbook is the best place for this. Please, would you have a minute to open a pull request with the doNotFake approach? I think we shouldn't influence whether the developer uses legacy/modern timers. Let them decide.

Opened mswjs/mswjs.io#290

[joel-daros]

@mattcosta7 Thanks for the solution, but it seems that none of them are working in my reproduction repo in the first thread message. I didn't find any major differences in my testing approach compared to @jeffwong14's Codesandbox.

[mattcosta7]

@mattcosta7 Thanks for the solution, but it seems that none of them are working in my reproduction repo in the first thread message. I didn't find any major differences in my testing approach compared to @jeffwong14's Codesandbox.

@joel-daros I haven't used create-react-app in a long time. From the docs, it looks like src/setupTests is configured to set setupFilesAfterEnv and not setupFiles.

As we discuss in the docs for msw

warning
Pay attention it’s the setupFiles option, and not setupFilesAfterEnv. The missing Node.js globals must be injected before the environment (e.g. JSDOM).

I'm not sure it can be configured in create-react-app without ejecting (but you can always just use jest directly instead of via react-scripts and pass a custom configuration for jest that mirrors cra but with additional configuration

https://github.com/facebook/create-react-app/blob/0a827f69ab0d2ee3871ba9b71350031d8a81b7ae/packages/react-scripts/scripts/utils/createJestConfig.js#L30C5-L35C1

so you may need to avoid using react-scripts test and manually configure jest to make this work - unless there's another way to override the actual setupFiles jest is using (maybe forking cra or using patch-package as 2 potential options)

[kettanaito]

I suppose node --require ./setup.js node_modules/.bin/react-scripts test should work too. Depends on what react-scripts actually does process-wise.

[mattcosta7]

I suppose node --require ./setup.js node_modules/.bin/react-scripts test should work too. Depends on what react-scripts actually does process-wise.

Maybe! I think it might conflict on react-app-polyfill/jsdom which will polyfill an alternative version of fetch, and I imagine the competing fetch/request/response impls will be problematic

[michalstrzelecki]

@mattcosta7 2nd fixed the issue, I have not checked 1st.