msw icon indicating copy to clipboard operation
msw copied to clipboard

Support Node.js v18

Open ericmasiello opened this issue 1 year ago • 3 comments

Prerequisites

Environment check

  • [X] I'm using the latest msw version
  • [X] I'm using Node.js version 14 or higher

Node.js version

v18.8.0

Reproduction repository

https://github.com/ericmasiello/msw-node-18

Reproduction steps

  1. Verify you are using node 18.8.0
  2. npm ci
  3. npm run test

Note that the tests pass but they actually should not. I'm using a toMatchInlineSnapshot(). That data snapshotted is the live data coming from real Pokemon API. what should be snapshotted is the mocked response I created in foo.test.ts

Current behavior

The tests pass because the msw is not intercepting the response. Therefore, the toMatchInlineSnapshot matches live data from the API.

Expected behavior

The tests should fail because the data should match the mocked response in foo.test.ts.

ericmasiello avatar Sep 04 '22 11:09 ericmasiello

Hey! I actually have a PR for this. Well, I have one simple PR, and then a series of more robust PRs. Let me summarize...

Background

  • Polyfilled fetch continues to be intercepted in Node 18 (mostly, I think)
  • Requests to IPv6 addresses will cause errors, regardless of fetch implementation. This might happen in tests that use localhost on an IPv6 system, because in Node 18, the resolution order has changed to prefer the order of OS resolution, instead of the previous behavior of preferring IPv4. You can revert to the previous behavior with the optional Node flag node --dns-result-order=ipv4first. This exposes the actual bug in msw (and some of its dependencies), which is incorrect URL serialization of IPv6 hosts (should be surrounded by brackets but is not, e.g. http://::1:80 should be http://[::1]:80).
  • "Node-native fetch" (aka Undici), which is enabled by default since Node 18, is not intercepted
  • The existing globalThis.fetch interceptor is able to intercept native fetch, but msw does not instantiate FetchInterceptor during createSetupServer, nor is it exported, so there is no easy way to call it from user code
  • We do not need to add any "special" interceptor for Undici, but we do need Undici as a dev dependency in the test/CI environment. Node bundles undici as internal code, but there is no way to import its package directly, so if you want to e.g. setGlobalDispatcher, you must add undici to your project and import it from there.

Quick fix

  • Draft PR in my fork: https://github.com/milesforks/msw/pull/1/files
  • Changing two lines in setupServer will make it "mostly" work.
  • This might not be the right way to make it available, since it could have impact on bundle size
  • This will not be covered by tests, and it will break with IPv6. It might also have some issues with TLS, can't remember (lots of moving parts here).
  • I wouldn't advocate merging this PR into this repository, but you might find it helpful as a quick fix in your own codebase. (For example, you could make the change to your local dependency with yarn patch or equivalent.)

Robust fix

  • Use that "quick fix," then migrate to Node 18, fix newly broken tests, and add tests for native fetch
  • For Node 18 fetch to be testable, we need to migrate the development and CI environment of this repository to Node 18
  • Migrating to Node 18 breaks tests in GitHub CI runners due (primarily) to new IPv6 resolution behavior described earlier
  • These problems unfortunately cascade through a few dependencies, including mswjs/interceptors, page-with and open-draft/test-server (source unavailable). In my PRs, I fixed these problems with hacky prototype patching, and @kettanaito also merged the fix into page-with (but still needs to release its package to npm, so I can remove the prototype patching and bump the page-with dependency version)
  • In addition to migrating this repository, I also have a PR to migrate mswjs/interceptors to Node 18: https://github.com/mswjs/interceptors/pull/283
  • I successfully migrated most of this main repository (msw) to Node v18, and added v18 to the testing matrix (along with v14 and v16), but there are still a few tests failing due to a memory leak. Here's the draft PR from my fork: https://github.com/milesforks/msw/pull/2

Summary

To fully migrate to Node 18, we need to open/merge the following PRs:

  • https://github.com/kettanaito/page-with/pull/9
  • https://github.com/mswjs/interceptors/pull/283
    • open, needs rebase and version bump of page-with
  • https://github.com/milesforks/msw/pull/2
    • unopened, draft in my fork, mostly complete, but failing a few tests due to memory leak

Note: These PRs are not as complicated/dangerous as they look. Most of the complexity only affects development/CI environment, and there are hardly any user-facing changes. And the high count of files changed is due to changing an import path to use the patched @open-draft/test-server.

In the meantime, as a "quick fix," we could make the two-line change described above (https://github.com/milesforks/msw/pull/1). If @kettanaito is okay with that approach, I can rebase my PR and open it here, or he can just make the change himself, since it's literally two lines.

Blockers

  • I need @kettanaito to release the latest version of page-with to npm so I can rebase https://github.com/mswjs/interceptors/pull/283
  • Optionally, @kettanaito needs to fix IPv6 handling in @open-draft/test-server and release the new version of it. (optional because the hacky prototype patching in my PR works fine)
  • I need to fix the last failing tests in my fork (https://github.com/milesforks/msw/pull/2) (also, @kettanaito maybe you can help me track down the source of the memory leak in the most recent GitHub Actions run?)

Next steps

I spent most of last week on this (thanks to ipv6...), so I would love to get this merged! Please let me know how you'd like to coordinate the order of merges here, and if you have any input on that last memory leak blocking my PR from being ready.

milesrichardson avatar Sep 08 '22 20:09 milesrichardson

@milesrichardson has put is perfectly. There's no Node 18 support at this moment but you can learn more about the strategy to support it above. I'm slowly getting through those pull requests to have it all in place. Be patient.

kettanaito avatar Sep 09 '22 09:09 kettanaito

The preparatory work on the Node 18 support should be done. Both page-with and @open-draft/test-server packages are bumped to support IPv6 in their server address handling. Both are internal packages but are still crucial for our testing setup.

kettanaito avatar Sep 09 '22 10:09 kettanaito

What is left for this? Node 18.12.0 is now the LTS and this is preventing us from updating.

joshuajaco avatar Oct 26 '22 07:10 joshuajaco

I would also like to understand the status of this. I want to move my testing framework to Vitest from Jest. However, Vitest is unstable on anything less than Node 18. But I can't yet move to Node 18 (or Vitest) until MSW supports Node 18.

ericmasiello avatar Oct 26 '22 10:10 ericmasiello

We also have issues with it in Node 18. Any updates?

max-sym avatar Nov 01 '22 11:11 max-sym

Are there viable alternatives to MSW that are Node18 compatible?

ericmasiello avatar Nov 01 '22 16:11 ericmasiello

I was actually able to solve it using cross-fetch. Tried node-fetch too, but it was buggy as well 🤦‍♂️

max-sym avatar Nov 01 '22 16:11 max-sym