mswjs / msw

Industry standard API mocking for JavaScript.
https://mswjs.io
MIT License
15.92k stars 516 forks source link

msw 1.1.0 regresses tests with Testing Library, React and Jest: "The current testing environment is not configured to support act(...)" #1563

Closed TheRealCuran closed 9 months ago

TheRealCuran commented 1 year ago

Prerequisites

Environment check

Node.js version

v16.19.1 (and v18.13.0)

Reproduction repository

https://github.com/TheRealCuran/bugs-reproducer-msw110

Reproduction steps

The reproducing repository contains a stripped down version of a React component and a bare bones test, that show the issue for me with the Node.js versions named above. Downgrading msw to 1.0.1 fixes the issue again.

Run yarn install && yarn msw-bug-test to reproduce.

Current behavior

Running our test suite with msw 1.1.0 leads to (error message as seen as in our test suite, the reproducer test creates a little bit different output due to different fields in the state and differing line numbers; identifying names have been replaced):

    Warning: The current testing environment is not configured to support act(...)

      168 |           err_msg = error.toString()
      169 |         }
    > 170 |         this.setState({
          |              ^
      171 |           isLoaded: true,
      172 |           base: undefined,
      173 |           actions: [],

      at printWarning (node_modules/react-dom/cjs/react-dom.development.js:86:30)
      at error (node_modules/react-dom/cjs/react-dom.development.js:60:7)
      at isConcurrentActEnvironment (node_modules/react-dom/cjs/react-dom.development.js:25260:7)
      at warnIfUpdatesNotWrappedWithActDEV (node_modules/react-dom/cjs/react-dom.development.js:27559:12)
      at scheduleUpdateOnFiber (node_modules/react-dom/cjs/react-dom.development.js:25508:5)
      at Object.enqueueSetState (node_modules/react-dom/cjs/react-dom.development.js:14067:7)
      at MyCompImpl.Object.<anonymous>.Component.setState (node_modules/react/cjs/react.development.js:354:16)
      at setState (src/mycomp/mycomp.tsx:170:14)

    expect(jest.fn()).toHaveBeenCalledTimes(expected)

    Expected number of calls: 1
    Received number of calls: 0

      118 | ) {
      119 |   jest.advanceTimersByTime(waitTime)
    > 120 |   await waitFor(() => expect(spy).toHaveBeenCalledTimes(timesCalled))
          |                                   ^
      121 | }
      122 |

      at src/mycomp/mycomp.spec.tsx:120:35
      at runWithExpensiveErrorDiagnosticsDisabled (node_modules/@testing-library/react/node_modules/@testing-library/dom/dist/config.js:47:12)
      at checkCallback (node_modules/@testing-library/react/node_modules/@testing-library/dom/dist/wait-for.js:127:77)
      at node_modules/@testing-library/react/node_modules/@testing-library/dom/dist/wait-for.js:70:9

This in turn leads to the state not being updated properly and thus the test failure.

Downgrading to 1.0.1 restores functionality again. Looking at the changelog I suspect it has to be #1543, which is causing this.

Side note: msw pulls in @mswjs/interceptors at version 0.17.9, the repository here on GitHub shows 0.22.7 as the latest version. Maybe it's only the dependency, that needs to be updated?

Expected behavior

I expect our tests to run through as before with msw 1.0.1 (or a clear documentation, which is indicating the breaking change and what to do to migrate – though I would have expected a major version change then).

So far this looks like a regression to me.

victoria100 commented 1 year ago

Same problem for me so I used v0.49.3 at the moment. I hope it will be resolved in the latest one.

scytalezero commented 1 year ago

MSW 1.1.0 and 1.1.1 break most of my tests that use mocks. I'm on node 16 using unfetch/polyfill, but even updating to node 18, my mocks no longer work in my tests. I've tried removing unfetch/polyfill, switching to node-fetch, and running with no fetch polyfill at node 18. At this point I'm unsure how to get MSW 1.1.0+ to work at all.

gamliela commented 1 year ago

I had similar regression with 1.1.0 version and then came here.

The new version adds a new interceptor to fetch requests. Essentialy replacing the global fetch with a new one. The relevant code is here: https://github.com/mswjs/msw/blob/main/src/node/setupServer.ts#L17

Previously I mocked fetch with whatwg-fetch so I could test timeouts on fetch requests with AbortSignal. This cannot be done anymore with the new version because the new interceptor does not respect aborted signals.

Agree with previous comments that 1.1.0 is a breaking change.

bartektelec commented 1 year ago

Any updates on this one? I am having the same issue, I used to polyfill my globalThis.fetch with cross-fetch. Upgrade to msw@1.1.0 breaks all my tests, and I have no idea how to make it work.

u-yas commented 1 year ago

About workaround measures

I also encountered this problem today. I would like to leave the workaround measures at that time here, so I would appreciate it if you could refer to it.

The comments below were very helpful in my research. thank you!

prerequisite

  1. msw version is v1.2.3
  2. install @mswjs/interceptors v0.17.7 https://github.com/mswjs/msw/blob/v1.2.3/pnpm-lock.yaml#L77

I believe these issues are the FetchInterceptor used internally by msw. Construct the SetupServerApi in your own setup file like the code below.

// your setup server typescript file.
import { ClientRequestInterceptor } from '@mswjs/interceptors/lib/interceptors/ClientRequest/index.js'
import { XMLHttpRequestInterceptor } from '@mswjs/interceptors/lib/interceptors/XMLHttpRequest/index.js'
import { SetupServerApi ,SetupServer } from 'msw/node'
import { handlers } from './my-handlers'

export const server: SetupServer =  new SetupServerApi(
     // do not use FetchInterceptors
     // Use something like whatwg-fetch in setupAfterEnv etc.
     // ex: https://github.com/mswjs/examples/blob/master/examples/with-jest/jest.setup.js#L1-L2
    [ClientRequestInterceptor, XMLHttpRequestInterceptor],
    ...handlers,
  )

beforeAll(() => server.listen())

afterEach(() => server.resetHandlers())

afterAll(() => server.close())
christo8989 commented 1 year ago

So... I'm using jest, msw, and graphql-upload.

I need the latest msw version to fix an issue with graphql-upload. However, the latest msw breaks jest, as described in this issue.

Side note, with the latest version of msw, graphql-upload breaks the fetch polyfill, a completely different package. Which, I can patch, so no biggie there. Just interesting. (https://github.com/JakeChampion/fetch/issues/460)

Craziness. All this to test file upload with graphql.

Anywho, I'm hoping this can get resolved soon. :)

kettanaito commented 1 year ago

Thanks for reporting this. I will share my thoughts on this below.

Side note: msw pulls in @mswjs/interceptors at version 0.17.9, the repository here on GitHub shows 0.22.7 as the latest version. Maybe it's only the dependency, that needs to be updated?

This is correct. MSW 1.x uses the older versions of Interceptors that are not based on the Fetch API primitives. MSW 2.x uses the latest version of Interceptors that are. No updates are necessary on this end.

Previously I mocked fetch with whatwg-fetch so I could test timeouts on fetch requests with AbortSignal. This cannot be done anymore with the new version because the new interceptor does not respect aborted signals.

We are aware of the missing AbortSignal support (reported in #1701). There's a pull request opened to address it https://github.com/mswjs/interceptors/pull/394 but I've encountered a few edge cases that may not be covered there, I have to have a detailed look at it. We also need to backport it to 0.17 to have this fixed in MSW 1.x.

Help with reviewing that pull request is highly welcome!

So, as I understand, the issue was introduced by the addition of FetchInterceptor, is that correct? I struggle to see why it would break things even if you consider the missing lack for AbortController (that should result in custom abort controllers not being respected but not things breaking all around, which is the impression I get from this thread).

What to do now?

Pin your msw version to the latest version you find working with your setup. This is the easiest option to keep having functional tests.

I will look into the issue but it's nowhere on my priority list right now. You are also welcome to dive into it, review pull requests, contribute tests, and improve MSW together. The beauty of open source.

You also have another option: migrate to msw@next, read #1464, migrate to modern Node.js (18+) and fight with Jest to understand the global Fetch API. I haven't encountered any issues with AbortController or React tests on that version.

christo8989 commented 1 year ago

I'm triaging a bit and I found the smoking gun, although I don't fully understand it at this point. Thought I'd share though.

Normal execution for multiple tests would be:

reset mocks
run test 1
call handler

reset mocks
run test 2
call handler

With the FetchInterceptor, I believe because of line https://github.com/mswjs/interceptors/blob/v0.17.7/src/interceptors/fetch/index.ts#L48, the following happens.

reset mocks
run test 1

reset mocks
run test 2

call handler
call handler

Where test 2 is asserting expect(handler).toBeCalledTimes(1), this assertion will now fail.

If there was a synchronous way to call arrayBuffer, I think that would solve the problem. But there isn't, and so I don't have a solution.

EDIT: I've dug a little further and it seems the only way to get the ArrayBuffer from the Request is to use an asynchronous operation. Things are NOT coming up Milhouse today.

EDIT2: The bug shows up only when I use render from @testing-library/react. I updated to the latest react/testing packages and the issue still exists.

CroquetMickael commented 1 year ago

Hi there,

i've try to migrate for the 2.X.X of MSW but same problem here, to be full node 18+ compatible, this is the last piece I need.

"node": "18.17.0"
msw: 2.0.2
Vitest: 0.34.6

"@testing-library/react": "13.4.0",
"@testing-library/user-event": "14.5.1",
"react": "18.2.0",
"react-dom": "18.2.0",
"react-hook-form": "7.47.0",
"react-router-dom": "6.16.0",

and for fetch call : 

ky: 1.1.2
atsikov commented 11 months ago

We also need to backport it to 0.17 to have this fixed in MSW 1.x.

@kettanaito are there any estimates for a backport (and is it something still planned)?

It seems it is not possible to use MSW with AbortController support and Node 16 at the moment. 1.0.1 is broken because of headers-polyfill@3.3.0; the latest v1, which has headers-polyfill pinned to a working version, is broken because of @mswjs/interceptors; and v2 requires Node 18+ 😞

Node 16 might be not the biggest issue (as it's anyway EoL by now), however v2 integration with Jest is also far from being seamless which complicates things.

msw@1.0.1 with headers-polyfill@3.2.5 in "resolutions" could be a working setup, however things are more complex when msw is a dependency in a library.

kettanaito commented 9 months ago

Conclusion

I'm sorry for any inconvenience this may cause. There are no plans for any backporting to 1.x. That version of the library is no longer maintained due to the lack of people to do that. My effort is focused on 2.0 and the future of the library. Spending effort of patching unsupported versions of Node.js and old tools is draining and I'd rather see that energy spent on better things.

How to fix this issue then?

Migrate to mws@latest. Here are a few materials to help you along the way:

There's also a Codemod that can boost your migration speed drastically.

Thanks for understanding.