msw icon indicating copy to clipboard operation
msw copied to clipboard

Can't resolve '@mswjs/interceptors/lib/interceptors/ClientRequest'

Open ntucker opened this issue 2 years ago • 21 comments

Prerequisites

Environment check

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

Browsers

NODE 16/18

Reproduction repository

https://github.com/coinbase/rest-hooks/tree/a3963b2618981992bab115a8e8543abdb9cdc655/website

Gonna revert MSW upgrade to make this work so master will no longer have this problem.

Reproduction steps

  1. clone repo above
  2. git checkout a3963b2618981992bab115a8e8543abdb9cdc655
  3. navigate to website folder
  4. yarn install
  5. yarn start
  6. observe error in cli as well as opened browser tab

Current behavior

ERROR in ./node_modules/msw/lib/node/index.mjs 45:0-94

Module not found: Error: Can't resolve '@mswjs/interceptors/lib/interceptors/ClientRequest' in '/home/ntucker/src/rest-hooks/website/node_modules/msw/lib/node'
Did you mean 'index.js'?
BREAKING CHANGE: The request '@mswjs/interceptors/lib/interceptors/ClientRequest' failed to resolve only because it was resolved as fully specified
(probably because the origin is strict EcmaScript Module, e. g. a module with javascript mimetype, a '*.mjs' file, or a '*.js' file where the package.json contains '"type": "module"').
The extension in the request is mandatory for it to be fully specified.
Add the extension to the request.

https://github.com/mswjs/msw/pull/1247 breaks in node versions that support ESM

Expected behavior

To fix this - the ESM version of MSW must be explicit about file extensions. Otherwise it is by definition incorrectly built. If this cannot be done you should not ship a custom ESM version as this is incorrect. The only reason it would work is if it is used by a system that doesn't enforce fully (webpack). However this will break in fully-spec compliant things like node.

More simply:

import '@mswjs/interceptors/lib/interceptors/ClientRequest' => import '@mswjs/interceptors/lib/interceptors/ClientRequest.mjs'

ntucker avatar Jun 01 '22 21:06 ntucker

Hey, @ntucker. Thanks for reporting this.

To fix this - the ESM version of MSW must be explicit about file extensions.

That's an interesting part: MSW doesn't publish an ESM version as of now. The /lib/ import you see is CommonJS. We don't have official support for ESM, I need to look into proper ESM bundling with tsup because the factory esm build target produced a module that breaks for consumers (see #1252).

Can you try this on 0.41.0 and 0.40.x? Does it work on the latter version?

kettanaito avatar Jun 02 '22 10:06 kettanaito

node_modules/msw/lib/node/index.mjs is commonjs? Mjs extensions should only be used for esm

ntucker avatar Jun 02 '22 14:06 ntucker

node_modules/msw/lib/node/index.mjs is commonest? Mjs extensions should only be used for esmwhat is https://github.com/mswjs/msw/pull/1247 about then? It adds mjs extensions

ntucker avatar Jun 02 '22 14:06 ntucker

Oh, you're right, we do publish ESM but only for msw/node:

https://github.com/mswjs/msw/blob/1b9dde0193bf170c9ed0b89dee6226021e067f76/tsup.config.ts#L40

🤦 I'm tried of dancing around with builds. We should remove all ESM build targets. The Interceptors library is (always was) CommonJS. So if the parent module (msw/node) is ESM it won't be able to use it.

I have a suspicion that ESM mean different things in different bundles. We used to have ESM build with Rollup for years but that shouldn't have worked (I've described why above). So confused.

kettanaito avatar Jun 03 '22 12:06 kettanaito

I believe importing is fine... you just have to specify the file extension anytime you're in an esm.

ntucker avatar Jun 03 '22 14:06 ntucker

@ntucker, yeah, that's the problem: how to do that in an ambiguous context?

The source code compiles to multiple targets, so I don't see specifying file extensions all the time as a sensible strategy. Moreover, it raises a compilation error in TS, which forces you not to include extensions. It seems like this should be done by the bundler automatically since it's the bundle that's aware of the build target (ESM) and should provide all the necessary transformations for the source code to produce a valid target code.

I'm looking for help with the proper library bundling, so anybody experienced with this is welcome to join the discussion or open a pull request. Thanks.

kettanaito avatar Jun 09 '22 13:06 kettanaito

Ts doesn't force you to not use extensions. If you add .js it will still look for .d.ts etc. See my project for example bundling: https://github.com/coinbase/rest-hooks

ntucker avatar Jun 09 '22 15:06 ntucker

For instance, you can see in https://github.com/coinbase/rest-hooks/blob/master/packages/core/src/index.ts all the relative imports specify .js in the extension.

If you use tsc for compiling your js: https://devblogs.microsoft.com/typescript/announcing-typescript-4-7/#ecmascript-module-support-in-node-js

If you use babel - you can see an example of how I do it https://github.com/coinbase/rest-hooks/blob/master/packages/core/package.json#L55 with and config https://github.com/coinbase/rest-hooks/blob/master/babel.config.js (using @anansi/babel-preset)

ntucker avatar Jun 09 '22 17:06 ntucker

Again, this implementation is against spec so it will break in all environments that implement spec correctly: https://nodejs.org/api/esm.html#mandatory-file-extensions

ntucker avatar Aug 01 '22 16:08 ntucker

simplest way to reproduce it is

👉 test.mjs

import { setupServer } from 'msw/node';

execute in the console

node test.mjs

will result in

$ node test.mjs
node:internal/process/esm_loader:97
    internalBinding('errors').triggerUncaughtException(
                              ^

Error [ERR_UNSUPPORTED_DIR_IMPORT]: Directory import '/html/rocket/node_modules/msw/node' is not supported resolving ES modules imported from /html/rocket/test.mjs
Did you mean to import msw/lib/node/index.js?
    at new NodeError (node:internal/errors:387:5)
    at finalizeResolution (node:internal/modules/esm/resolve:400:17)
    at moduleResolve (node:internal/modules/esm/resolve:965:10)
    at defaultResolve (node:internal/modules/esm/resolve:1173:11)
    at nextResolve (node:internal/modules/esm/loader:173:28)
    at ESMLoader.resolve (node:internal/modules/esm/loader:852:30)
    at ESMLoader.getModuleJob (node:internal/modules/esm/loader:439:18)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/module_job:76:40)
    at link (node:internal/modules/esm/module_job:75:36) {
  code: 'ERR_UNSUPPORTED_DIR_IMPORT',
  url: 'file:///html/rocket/node_modules/msw/node'
}

Node.js v18.7.0

Is there a plan to go to esm? or to support esm entrypoints?

See node docs for conditional entry points for esm & cjs

https://nodejs.org/api/packages.html#packages_conditional_exports

daKmoR avatar Aug 28 '22 09:08 daKmoR

Is there a plan to go to esm? or to support esm entrypoints?

Yes, there is a plan. Contributions are extremely welcome to make this plan a reality. This particular support doesn't interest me at the moment so I'm unlikely to spend time on it. But I'm always open to providing code reviews and making this support happen.

kettanaito avatar Aug 30 '22 11:08 kettanaito

I would personally be happy with ESM targets being removed until properly impelemented

ntucker avatar Aug 30 '22 17:08 ntucker

just started having this issue using [email protected] + [email protected] + [email protected].

Is there a workaround til this is fixed but scripting the node_modules?

raulfdm avatar Sep 07 '22 13:09 raulfdm

Workaround: downgrade to 0.46.1

raulfdm avatar Sep 07 '22 13:09 raulfdm

just started having this issue using [email protected] + [email protected] + [email protected].

Is there a workaround til this is fixed but scripting the node_modules?

where did you start noticing the issue on the msw/node side? curious if you would mind trying some steps to triage

in node_modules/msw/node/package.json remove the module entry - and in node_modules/msw/package.json

 "./node": {
      "require": "./lib/node/index.js",
      "default": "./lib/node/index.mjs"
    }

make the default the index.js build - removing the require option

then restart and see if that allows a build to succeed. if so, we might consider dropping the esm node build for now, until we can handle that directly

mattcosta7 avatar Sep 07 '22 15:09 mattcosta7

just started having this issue using [email protected] + [email protected] + [email protected]. Is there a workaround til this is fixed but scripting the node_modules?

where did you start noticing the issue on the msw/node side or the msw direct side?

msw/node. In the client it was working fine but the server part was broken

raulfdm avatar Sep 07 '22 15:09 raulfdm

just started having this issue using [email protected] + [email protected] + [email protected]. Is there a workaround til this is fixed but scripting the node_modules?

where did you start noticing the issue on the msw/node side or the msw direct side?

msw/node. In the client it was working fine but the server part was broken

I added a few triage steps to my last message, if you wouldn't mind giving them a shot, and describing output. it might help with getting that fixed a bit

I think we probably never used the module import from msw/node directly, since the package.json wasn't type: module there, and in https://github.com/mswjs/msw/pull/1383/files#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519R15-R16 that might not be compat.

@ivanhofer curious if you've seen that working on your end properly

mattcosta7 avatar Sep 07 '22 15:09 mattcosta7

just started having this issue using [email protected] + [email protected] + [email protected]. Is there a workaround til this is fixed but scripting the node_modules?

where did you start noticing the issue on the msw/node side or the msw direct side?

msw/node. In the client it was working fine but the server part was broken

I added a few triage steps to my last message, if you wouldn't mind giving them a shot, and describing output. it might help with getting that fixed a bit

I think we probably never used the module import from msw/node directly, since the package.json wasn't type: module there, and in #1383 (files) that might not be compat.

@ivanhofer curious if you've seen that working on your end properly

It is working for me, because I'm using it in a cjs context. I would assume this fixes it for esm: https://github.com/mswjs/msw/pull/1395 I didn't spot that the esm file didn't have a d.ts file in the last PR.

ivanhofer avatar Sep 07 '22 16:09 ivanhofer

@mattcosta7 I'm also only experiencing this problem since that upgrade (see the build error) and just tried your triage steps in my project. I can confirm that after those steps, the build completed successfully, so it might indeed be worth dropping esm from the Node build for now.

(Alternatively, only using default exports when importing from CJS modules in an ES module might also fix actual ES module support.)

Vinnl avatar Sep 08 '22 11:09 Vinnl

I have investigated this issue a bit. There are a few road blocks to make msw/node work in a .mjs file:

  1. reference lib/index.js instead of 'lib' in a few files (easy to achive)
  2. tsup does not support import assertions (needed here https://github.com/mswjs/msw/blob/main/src/context/status.ts#L1) https://github.com/egoist/tsup/issues/714
  3. dynamic import of node-fetch (https://github.com/mswjs/msw/blob/main/src/context/fetch.ts#L6) is not supported in esm context. https://github.com/evanw/esbuild/issues/1921

ivanhofer avatar Sep 10 '22 21:09 ivanhofer

I have created a draft PR that fixes point nr. 1. If anyone knows a solution to the other points. Feel free to contribute.

ivanhofer avatar Sep 11 '22 18:09 ivanhofer

White we are waiting for the PR to be merged, here is my temporary fix with yarn patch (available only with Yarn 2, if you don't use yarn 2, try patch-package).

Export of the msw-npm-0.47.4-c11a8d2944.patch file

diff --git a/lib/node/index.mjs b/lib/node/index.mjs
index 2cce41056acf9178a73d624db6319e8b7c2f1536..bbe0f4c42cd6cd3cec114c0ac76c5eec2b00ad22 100644
--- a/lib/node/index.mjs
+++ b/lib/node/index.mjs
@@ -42,11 +42,13 @@ import { setTimeout as nodeSetTimeout } from "timers";
 var setTimeout = nodeSetTimeout;
 
 // src/node/setupServer.ts
-import { ClientRequestInterceptor } from "@mswjs/interceptors/lib/interceptors/ClientRequest";
-import { XMLHttpRequestInterceptor } from "@mswjs/interceptors/lib/interceptors/XMLHttpRequest";
+import { ClientRequestInterceptor } from "@mswjs/interceptors/lib/interceptors/ClientRequest/index.js";
+import { XMLHttpRequestInterceptor } from "@mswjs/interceptors/lib/interceptors/XMLHttpRequest/index.js";
 
 // src/node/createSetupServer.ts
-import { bold } from "chalk";
+import chalk from 'chalk';
+const { bold } = chalk;
+
 import { isNodeProcess as isNodeProcess3 } from "is-node-process";
 import { StrictEventEmitter } from "strict-event-emitter";
 import {
@@ -256,7 +258,7 @@ function isStringEqual(actual, expected) {
 }
 
 // src/context/status.ts
-import statuses from "statuses/codes.json";
+import statuses from "statuses/codes.json" assert {type: "json"};;
 var status = (statusCode, statusText) => {
   return (res) => {
     res.status = statusCode;
@@ -409,7 +411,9 @@ var errors = (errorsList) => {
 // src/context/fetch.ts
 import { isNodeProcess as isNodeProcess2 } from "is-node-process";
 import { Headers } from "headers-polyfill";
-var useFetch = isNodeProcess2() ? __require("node-fetch") : window.fetch;
+import nodeFetch from 'node-fetch';
+
+var useFetch = isNodeProcess2() ? nodeFetch : window.fetch;
 var augmentRequestInit = (requestInit) => {
   const headers = new Headers(requestInit.headers);
   headers.set("x-msw-bypass", "true");
@@ -575,7 +579,7 @@ function prepareResponse(res) {
 
 // src/utils/matching/matchRequestUrl.ts
 import { match } from "path-to-regexp";
-import { getCleanUrl } from "@mswjs/interceptors/lib/utils/getCleanUrl";
+import { getCleanUrl } from "@mswjs/interceptors/lib/utils/getCleanUrl.js";
 
 // src/utils/url/cleanUrl.ts
 var REDUNDANT_CHARACTERS_EXP = /[\?|#].*$/g;
@@ -638,7 +642,7 @@ function matchRequestUrl(url, path, baseUrl) {
 import * as cookieUtils3 from "cookie";
 import { store } from "@mswjs/cookies";
 import { IsomorphicRequest } from "@mswjs/interceptors";
-import { decodeBuffer } from "@mswjs/interceptors/lib/utils/bufferUtils";
+import { decodeBuffer } from "@mswjs/interceptors/lib/utils/bufferUtils.js";
 import { Headers as Headers2 } from "headers-polyfill";
 
 // src/utils/request/getRequestCookies.ts

t18n avatar Oct 31 '22 22:10 t18n

@turboninh Can you please export the commit you patched and post it here?

morrisonbrett avatar Oct 31 '22 23:10 morrisonbrett

@morrisonbrett Updated my comment :)

t18n avatar Nov 01 '22 09:11 t18n

Released: v0.48.0 🎉

This has been released in v0.48.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.

kettanaito avatar Nov 08 '22 12:11 kettanaito

https://github.com/mswjs/msw/issues/1267#issuecomment-1297776977

FYI the issue with bufferUtils still occurs in 0.48.0

Error [ERR_MODULE_NOT_FOUND]: Cannot find module 'node_modules/@mswjs/interceptors/lib/utils/bufferUtils' imported from node_modules/msw/lib/node/index.mjs
Did you mean to import @mswjs/interceptors/lib/utils/bufferUtils.js

grzegorz-zadora avatar Nov 08 '22 13:11 grzegorz-zadora

Me too.. FYI the issue with bufferUtils still occurs in ^0.48.0

Did you mean to import @mswjs/interceptors/lib/utils/bufferUtils.js?
    at new NodeError (internal/errors.js:322:7)

cksal0805 avatar Nov 08 '22 14:11 cksal0805

I wonder if the error on 0.48.0 is still there because of the interceptors? Looking at this comment https://github.com/mswjs/msw/pull/1399#pullrequestreview-1169508489

jorgepvenegas avatar Nov 09 '22 00:11 jorgepvenegas

I was getting the same bufferUtils error after upgrading to 0.48.0 but it's now working for me after upgrading to 1.2.1

brleinad avatar Jun 07 '23 19:06 brleinad