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

Unable to use msw/node for testing solid-js due to resolve.conditions set to browser #2092

Closed
4 tasks done
nknapp opened this issue Mar 16, 2024 · 7 comments
Closed
4 tasks done
Labels
bug Something isn't working needs:triage Issues that have not been investigated yet. scope:node Related to MSW running in Node

Comments

@nknapp
Copy link

nknapp commented Mar 16, 2024

Prerequisites

Environment check

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

Node.js version

Tested in v18.15.0 and v20.11.1

Reproduction repository

https://github.com/nknapp/msw-solid-testing-library-bug-reproduction

Reproduction steps

Run

npm install
npm test

Current behavior

The test output for the loadValue.test.ts is

 FAIL  src/loadValue.test.ts [ src/loadValue.test.ts ]
Error: No known conditions for "./node" specifier in "msw" package
 ❯ e node_modules/vite/dist/node/chunks/dep-jvB8WLp9.js:47465:25
 ❯ n node_modules/vite/dist/node/chunks/dep-jvB8WLp9.js:47465:646
 ❯ o node_modules/vite/dist/node/chunks/dep-jvB8WLp9.js:47465:1297
 ❯ resolveExportsOrImports node_modules/vite/dist/node/chunks/dep-jvB8WLp9.js:48144:20
 ❯ resolveDeepImport node_modules/vite/dist/node/chunks/dep-jvB8WLp9.js:48163:31
 ❯ tryNodeResolve node_modules/vite/dist/node/chunks/dep-jvB8WLp9.js:47888:20
 ❯ Context.resolveId node_modules/vite/dist/node/chunks/dep-jvB8WLp9.js:47649:28
 ❯ Object.resolveId node_modules/vite/dist/node/chunks/dep-jvB8WLp9.js:50880:64
 ❯ TransformContext.resolve node_modules/vite/dist/node/chunks/dep-jvB8WLp9.js:50571:23

The error occurs, because the vite-plugin-solid sets the vite configuration option resolve.conditions to ['browser'] and because the package.json of both msw and @mswjs/interceptors specify browser: null for imports being used.

This is the extract of the msw package.json

  "exports": {
    "./node": {
      "browser": null,
-----------------^
      "types": "./lib/node/index.d.ts",
      "require": "./lib/node/index.js",
      "import": "./lib/node/index.mjs",
      "default": "./lib/node/index.mjs"
    }
  }

On the other hand, if the plugin would not specify this condition, vite would load the ssr version of solid-js, providing and empty render function.

Expected behavior

The loadValue test should pass.

In fact, I can make it pass, by copying the default key to the browser key in the package.json exports.

In msw

  "exports": {
    "./node": {
      "browser": "./lib/node/index.mjs",
      "types": "./lib/node/index.d.ts",
      "require": "./lib/node/index.js",
      "import": "./lib/node/index.mjs",
      "default": "./lib/node/index.mjs"
    }
  }

In @mswjs/interceptors:

    "./ClientRequest": {
      "browser": "./lib/node/interceptors/ClientRequest/index.js",
      "types": "./lib/node/interceptors/ClientRequest/index.d.ts",
      "require": "./lib/node/interceptors/ClientRequest/index.js",
      "import": "./lib/node/interceptors/ClientRequest/index.mjs",
      "default": "./lib/node/interceptors/ClientRequest/index.js"
    },
@nknapp nknapp added bug Something isn't working needs:triage Issues that have not been investigated yet. scope:node Related to MSW running in Node labels Mar 16, 2024
nknapp added a commit to nknapp/aikido-exam that referenced this issue Mar 16, 2024
@kettanaito
Copy link
Member

kettanaito commented Mar 17, 2024

Hi, @nknapp. Thanks for reporting this.

You must configure your tooling to use the node resolve condition in a Node.js process, which is correct. Resolving dependencies using a browser condition while running in Node.js is a bad idea. I understand why tools try that but it's a shortcoming of the design. It should at least be ['browser', 'node']. If you can control this, set the array of resolve conditions to include node as well. If you can't control this, raise and issue in the respective tooling. You can link this comment there, and I'd be more than happy to explain why people should not use browser export condition in Node.js.

MSW uses the conditional exports correctly: you must not import msw/node in the browser. We cannot point that browser condition to a Node.js build, that'd blow things up on correct usage.

@nknapp
Copy link
Author

nknapp commented Mar 19, 2024

Thanks, I have opened an issue with the plugin and will try your workaround.

As I said there. I think it is debatable. Since the user has to explicitly import msw/node, I don't see the necessity to set browser to null. The import makes it already clear that it is supposed to be used in Node.

@nknapp
Copy link
Author

nknapp commented Mar 19, 2024

@kettanaito I have tried your suggestion and modified vite-plugin-solid to set ['browser', 'node'] as conditions.

This still does not work, because the resolve.exports code iterates the keys of the export definition and returns the first matching key, which is browser. If browser were the last key in the exports definition, or if instead of browser: null, you would omit the browser key completely, then at least msw would work with ['browser', 'node'].

I haven't tried though, if testing solidjs components would work as well....

@nknapp
Copy link
Author

nknapp commented Mar 23, 2024

@kettanaito I got some more insight and a better solution.

The resolution algorithm will iterate the keys of the exports object and take the first match. The order or keys is significant (see this answer in the resolve.exports repo.

For msw/node this means that even if node is in the conditions, as long as browser is in there as well, the resolution will fail, because browser comes before node in the exports object.

If you intend resolution to fail for browser builds and succeeds for Node.js, it should look like this:

    "./node": {
      "types": "./lib/node/index.d.ts",
      "node": {
        "require": "./lib/node/index.js",
        "import": "./lib/node/index.mjs"
      },
      "browser": null,
      "default": "./lib/node/index.mjs"
    },

First look for 'node" and succeed. Then look for "browser" and fail.

I have updated my reproduction example https://github.com/nknapp/msw-solid-testing-library-bug-reproduction to
patch msw with these updated objects.

  • vite build fails with Error: [commonjs--resolver] No known conditions for "./node" specifier in "msw" package
  • vitest works fine.

Would be nice if you could reopen this issue.

@MarkLeMerise
Copy link

Thanks for all the work you did narrowing down this issue, @nknapp. As it happens, I just finished a migration from msw 1.x -> 2.x.

// server.ts
import { setupServer } from "msw/node";
import { handlers } from "./handlers";
export const server = setupServer(...handlers);

But, I am seeing a similar error when running vitest:

No known conditions for "./ClientRequest" specifier in "@mswjs/interceptors" package

It looks like the key order in the main msw/package.json now follows your recommendation, but msw/node_modules/@mswjs/interceptors/package.json is still in its original order. I applied this patch by hand and my tests passed:

"./ClientRequest": {
+    "node": {
+       "require": "./lib/node/interceptors/ClientRequest/index.js",
+        "import": "./lib/node/interceptors/ClientRequest/index.mjs"
+     },
      "browser": null,
      "types": "./lib/node/interceptors/ClientRequest/index.d.ts",
      "require": "./lib/node/interceptors/ClientRequest/index.js",
      "import": "./lib/node/interceptors/ClientRequest/index.mjs",
      "default": "./lib/node/interceptors/ClientRequest/index.js"
},

The main difference in my setup is the Vite ecosystem library versions are a bit older:

{
  "vitest": "0.34.1",
  "vite": "4.4.10"
  "vite-plugin-solid": "2.7.0",
  "solid-js": "1.7.12",
  "msw": "2.2.14"
}

@EresDev
Copy link

EresDev commented Oct 28, 2024

I am facing the same problem with vitest.

@nknapp
Copy link
Author

nknapp commented Oct 28, 2024

@MarkLeMerise I haven't looked at the issue anymore, since I was occupied with other things. But if you think your patch is correct and if it matches the change in the main module, would you mind opening a PR?

You will get my upvote. I think, since this issue is closed already, the information might get lost here.

@github-actions github-actions bot locked and limited conversation to collaborators Nov 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working needs:triage Issues that have not been investigated yet. scope:node Related to MSW running in Node
Projects
None yet
Development

No branches or pull requests

4 participants