instanceof checks fail when using peerDeps?

[skrivle]

Describe the bug

Hi there!

First of all, thank you for all the hard work on vitest! Very much appreciated 🙏 !

When upgrading from 1.4.0 to 1.5.0. we are seeing some of our tests fail.

When debugging we noticed this seems to be related with a failing instanceof check.

More specifically, we have an Apollo GraphQL server and we are relying on instanceof to verify whether an error is an instance of GraphQLError in some places. With 1.5.0 error instanceof GraphQLError fails whereas with 1.4.0 this succeeds.

In this specific case the package @apollo/server is throwing the GraphQLError instance. Apollo server imports GraphQLError from the graphql package on which it has a peerDependency. Our own project then also specifies a dependency on the graphql packages and imports GraphQLError to do the instanceof check.

It feels like somehow the graphql package gets duplicated during bundling? This might explain why instanceof fails?

To be clear we are not seeing this issue during runtime. Only when running tests with vitest.

Reproduction

I have tried to come up with a minimal reproduction:
github: https://github.com/skrivle/vitest-deps-repro.
stackblitz: https://stackblitz.com/~/github.com/skrivle/vitest-deps-repro

• run yarn test to see failing unit test
• run yarn start to see a similar check with node succeed.

I have created a test-package that has a peerDependency on graphql just as apollo/server does in our case. This package then throws a GraphQLError. In app.spec.ts we are then verifying whether the error thrown is an instanceof GraphQLError. This fails in the test. However when running a similar script with node (run yarn start) the check seems to succeed just fine.

❗ In our graphql server project our tests do pass with 1.4.0 but fail with 1.5.0. However with the repro I've made, it seems to also be failing with 1.4.0. This is a bit suspicious. Though, I thought it might already be worth sharing...

System Info

System:
    OS: macOS 14.4.1
    CPU: (10) arm64 Apple M1 Pro
    Memory: 1.23 GB / 32.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.11.1 - /opt/homebrew/opt/nvm/versions/node/v20.11.1/bin/node
    Yarn: 3.3.0 - /opt/homebrew/opt/nvm/versions/node/v20.11.1/bin/yarn
    npm: 10.2.4 - /opt/homebrew/opt/nvm/versions/node/v20.11.1/bin/npm
    pnpm: 9.0.5 - /opt/homebrew/opt/nvm/versions/node/v20.11.1/bin/pnpm
  Browsers:
    Chrome: 124.0.6367.62
    Safari: 17.4.1
  npmPackages:
    @vitest/coverage-v8: 1.5.0 => 1.5.0
    vitest: 1.5.0 => 1.5.0

Used Package Manager

yarn

Validations

• Follow our Code of Conduct
• Read the Contributing Guidelines.
• Read the docs.
• Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
• The provided reproduction is a minimal reproducible example of the bug.

[hi-ogawa]

This seems like yet-another instance of dist/esm/... path issue:

• fix(vite-node): fix isValidNodeImport to check "type": "module" first #5416
• When upgrading from 1.4.0 to 1.5.0, an error related to CommonJS modules appears. #5555 (comment)

@apollo/server seems like a legitimate esm package https://publint.dev/@apollo/[email protected], but Vitest (vite-node) has been incorrectly treating it as a "bad" package and transforming the entire code and that made @apollo/server to import graphql's module entry https://publint.dev/[email protected] which is graphql/index.mjs.

Your source code is always transformed, so both were seeing graphql/index.mjs on Vitest v1.4.0, but after the change #5416, the legitimate @apollo/server are not transformed and it ends up loading graphql/index.js since we rely on Node's behavior there, thus dual package happens.

Actually, I don't know why Vitest/Vite is not reading "main": "index" entry (which they probably mean index.js), but this might be partly because of graphql side's issue or Vite resolution issue.

You can try server.deps.inline: ["@apollo/server"] (like I mentioned in #5555 (comment)) to get back to the previous behavior and that should fix the issue, but potentially there's other issue involved with this.

[skrivle]

Hi!

Thanks for the quick response! I've tried using deps.inline and it does work for the repro I have created (adding 'test-package'). Though for our real project it ends up throwing this error at me:

It seems like the graphql package might actually be the one (partially) causing the problem as described here: graphql/graphql-js#4062

[hi-ogawa]

Hmm, I expected server.deps.inline should be the only factor for your case. Maybe you have other package importing graphql. Can you try something like yarn why graphql?

---

Also the other thing I was wondering about is rather unusual "main": "index" entry and indeed it looks like this is tripping off Vitest's normal resolution. Patching it to "main": "index.js" seems to make Vitest to import graphql/index.js instead of graphql/index.mjs.

[hi-ogawa]

One more thing I forgot to mention is that you can force Vitest's resolution using alias.
For example, one idea would be to borrow NodeJs's require by something like:

import { createRequire } from "node:module";
const require = createRequire(import.meta.url);

export default defineConfig({
  test: {
      alias: {
          // or maybe simply __dirname + "/node_modules/graphql/index.js"
          "graphql": require.resolve("graphql"),
      }
  }
});

[skrivle]

Ah, yes you're right. I forgot that there were other packages with a peer dependency on graphql. Adding those fixes it.

I can also confirm that the second solution work as well. We might prefer that for now since it seems like that's the package causing the issue.

Thanks again 🙏