Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Node.js v18 #1388

Closed
4 tasks done
ericmasiello opened this issue Sep 4, 2022 · 33 comments · Fixed by #1436
Closed
4 tasks done

Support Node.js v18 #1388

ericmasiello opened this issue Sep 4, 2022 · 33 comments · Fixed by #1436
Assignees
Labels
feature scope:node Related to MSW running in Node

Comments

@ericmasiello
Copy link

Prerequisites

Environment check

  • I'm using the latest msw version
  • 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 ericmasiello added bug Something isn't working needs:triage Issues that have not been investigated yet. scope:node Related to MSW running in Node labels Sep 4, 2022
@milesrichardson
Copy link

milesrichardson commented Sep 8, 2022

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: feat: support Node.js 18 interceptors#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: Draft: Add node v18 support with tests (WIP): use patched @mswjs/interceptors milesforks/msw#2

Summary

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

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 (milesforks#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

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.

@kettanaito
Copy link
Member

@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 kettanaito changed the title Unable to intercept requests on Node 18 Support Node.js v18 Sep 9, 2022
@kettanaito kettanaito added feature and removed bug Something isn't working needs:triage Issues that have not been investigated yet. labels Sep 9, 2022
@kettanaito kettanaito pinned this issue Sep 9, 2022
@kettanaito
Copy link
Member

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.

@joshuajaco
Copy link

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

@ericmasiello
Copy link
Author

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.

@max-sym
Copy link

max-sym commented Nov 1, 2022

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

@ericmasiello
Copy link
Author

Are there viable alternatives to MSW that are Node18 compatible?

@max-sym
Copy link

max-sym commented Nov 1, 2022

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

@tobilen
Copy link

tobilen commented Nov 9, 2022

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

also works for node 19. thanks ❤️

@ericmasiello
Copy link
Author

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

Can you point to an example of what it means to use cross-fetch with msw?

@tobilen
Copy link

tobilen commented Nov 9, 2022

use the fetch from this library instead of the native one from node

import { setupServer, rest } from "msw/node";
import fetch from "cross-fetch"; // <-- this is what it means

// application code
const myFetch = async () => {
  const response = await fetch("https://some-url.com");
  return response.json();
};


// test code
const server = setupServer(
  rest.get("https://some-url.com", (req, res, ctx) =>
    res(ctx.status(200), ctx.json({ someKey: "some value" }))
  )
);

test("msw", async () => {
  server.listen();

  expect(await myFetch()).toEqual({ someKey: "some value" });
});

@ericmasiello
Copy link
Author

Thank you, @tobilen. Ill give this a try.

@yairkukielkazunzunegui
Copy link

yairkukielkazunzunegui commented Dec 9, 2022

For those looking for another workaround on Node 18 until this issue is completed, you can do this in your package.json:

"scripts": {
    "dev": "NODE_OPTIONS='--no-experimental-fetch'  node server.js", // or any script to start your server
    ...
   }

Adding this Node flag will use the old Node fetch (fetch from node version < 18)

@kettanaito
Copy link
Member

Update

MSW v2.0 will drop Node.js < 18 and will ship with Node.js 18+ support. This also includes proper ESM support that people have been requesting for quite some time now.

@ValentinH
Copy link

ValentinH commented May 12, 2023

Do you have an ETA please? 🙂

@kettanaito
Copy link
Member

@ValentinH, as soon as it's sufficiently tested. You can contribute to that by giving the next version of MSW a try. I'd aim for sometime in late summer as I won't have much availability in the upcoming months.

You can adopt msw@next even now. The release candidate is stable and this is very similar, if not identical, to what we will ship in v2.0. You don't have to wait.

@ValentinH
Copy link

Thanks for the answer. I missed this info, I'll give the next version a try next week then 🤞

@henkkasoft
Copy link

henkkasoft commented May 22, 2023

Thank you for the great msw library and thank you @yairkukielkazunzunegui for solving my problem.
If someone is having the same problems this is what happened for me.

Upgrated our cra react project to vite and vitest.
We have msw used for integration tests. Those tests render the app and msw is serving the entire API and different user operations are simulated in the tests.
Fetch was not available anymore after the upgrade but adding
import 'whatwg-fetch';
to setupIntegrationTests.ts file fixed this for my local environment. (cra had internally made fetch available in tests but vite/vitest did not have that anymore)

But these tests continue to fail on Jenkins. This issue here helped me to figure the problem:
I had node 16 in my local environment and at Jenkins the tests were executed in node:20-alpine image. So the new node fetch was the problem.

The issue was fixed by adding the --no-experimental-fetch like this for integration test command in package.json:
"integrationTestCI": "cross-env NODE_OPTIONS='--no-experimental-fetch' vitest run --config ./vitest.integrationtest.config.js",
When node fetch start to work in msw, I will remove whatwg-fetch and --no-experimental-fetch, but for now this solution worked for me.

Here are my vitest.integrationtest.config.js and setupIntegrationTests.ts. Just in case someone is wondering.

vitest.integrationtest.config.js

import { defineConfig } from 'vite';
import react from '@vitejs/plugin-react';
import tsconfigPaths from 'vite-tsconfig-paths';
import eslint from 'vite-plugin-eslint';

export default () => {
  return defineConfig({
    plugins: [tsconfigPaths(), react(), eslint()],
    test: {
      maxConcurrency: 1,
      environment: 'jsdom',
      setupFiles: './src/setupIntegrationTests.ts',
      restoreMocks: true,
      include: ['./src/test/integration/*'],
    },
  });
};

setupIntegrationTests.ts

import 'whatwg-fetch';
import { expect, beforeEach, afterEach, afterAll } from 'vitest';
import { cleanup } from '@testing-library/react';
import matchers from '@testing-library/jest-dom/matchers';
import mockServer from 'mockapi/server';

expect.extend(matchers);

beforeEach(async () => {
  await mockServer.listen();
});
afterEach(async () => {
  await mockServer.resetHandlers();
  cleanup();
});
afterAll(async () => {
  await mockServer.close();
});

@kettanaito
Copy link
Member

Update

Node.js 18 support has been a gigantic effort and it's finally complete! 🎉

MSW officially supports Node.js 18+ in its next release tag. Please upgrade to npm i msw@next and read #1464 since it's a breaking change and will require adjustments to your existing usage of the library.

There's no intention of bringing Node.js 18 support to the 1.x version of the library due to the amount of refactoring required (which has been already done in next). Thanks.

@kettanaito
Copy link
Member

Released: v2.0.0 🎉

This has been released in v2.0.0!

Make sure to always update to the latest version (npm i msw@latest) to get the newest features and bug fixes.


Predictable release automation by @ossjs/release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature scope:node Related to MSW running in Node
Projects
None yet
Development

Successfully merging a pull request may close this issue.