`pluginContainer.resolveId(..., { ssr: true })` works differently from `ssrLoadModule` resolution for `"module"` entry/exports

hi-ogawa commented 1 month ago

Describe the bug

I was investigating Vitest issue https://github.com/vitest-dev/vitest/issues/5664 and I came across this confusing behavior while checking the reproduction.

The package in question has following exports and it looks like Vite SSR resolution changes which to pick ...cjs.mjs or ...esm.js depending on ssrLoadModule / resolveId order.

(This is what I made as a simpler repro based on @emotion/react https://publint.dev/@emotion/react@11.11.4)

{
  "name": "test-dep",
  "version": "11.11.4",
  "exports": {
    ".": {
      "module": {
        "default": "./dist/emotion-react.esm.js"
      },
      "import": "./dist/emotion-react.cjs.mjs",
      "default": "./dist/emotion-react.cjs.js"
    }
  }
}

I'm not sure yet if this is directly relevant to the referenced Vitest issue, but I thought I'd share it here. I would appreciate if anyone knows whether this is a packaging issue, my incorrect usage, or Vite's resolution issue (or something else).

Reproduction

https://github.com/hi-ogawa/reproductions/blob/main/vitest-5664-mui-emotion-provider/repro-vite.mjs

Steps to reproduce

Running on stackblitz https://stackblitz.com/edit/github-htzuw3?file=repro-vite.mjs

$ node repro-vite.mjs
######
###### server.ssrLoadModule
######
dist/emotion-react.cjs.mjs
[Module: null prototype] {  }
######
###### server.pluginContainer.resolveId
######
{
  id: '/home/projects/github-yxzhzt/node_modules/.pnpm/file+fixtures+test-dep/node_modules/test-dep/dist/emotion-react.cjs.mjs'
}

$ node repro-vite.mjs reverse
######
###### server.pluginContainer.resolveId
######
{
  id: '/home/projects/github-yxzhzt/node_modules/.pnpm/file+fixtures+test-dep/node_modules/test-dep/dist/emotion-react.esm.js'
}
######
###### server.ssrLoadModule
######
dist/emotion-react.esm.js
[Module: null prototype] { default: {} }

Also comparing with NodeJS's resolution:

$ node repro-node.mjs
dist/emotion-react.cjs.mjs

System Info

(stackblitz)

  System:
    OS: Linux 5.0 undefined
    CPU: (8) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 0 Bytes / 0 Bytes
    Shell: 1.0 - /bin/jsh
  Binaries:
    Node: 18.18.0 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 10.2.3 - /usr/local/bin/npm
    pnpm: 8.15.6 - /usr/local/bin/pnpm
  npmPackages:
    vite: 5.2.10 => 5.2.10

Used Package Manager

pnpm

Logs

No response

Validations

[X] Follow our Code of Conduct
[X] Read the Contributing Guidelines.
[X] Read the docs.
[X] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
[X] Make sure this is a Vite issue and not a framework-specific issue. For example, if it's a Vue SFC related bug, it should likely be reported to vuejs/core instead.
[X] Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
[X] The provided reproduction is a minimal reproducible example of the bug.

stackblitz[bot] commented 1 month ago

Start a new pull request in StackBlitz Codeflow.

github-htzuw3 (Open reproduction)

sapphi-red commented 1 month ago

It might be related to https://github.com/vitejs/vite/issues/12957#issuecomment-1546664484 (haven't checked the repro deeply)

hi-ogawa commented 1 month ago

Thanks for the reference. Yeah, cache issue part of the issue might be related to that.

Another part of the issue is that it doesn't look like it's possible to exclude "module" condition when using resolveId("some-dep", { ssr: true }) regardless of "some-dep" is external or not.

Currently ssrLoadModule API has overrideConditions trick which happens to just exclude "module" condition and it behaves nicer in general (though it still gets "module" condition when directly doing ssrLoadModule("some-dep")).

I'm making a wip PR in https://github.com/vitejs/vite/pull/16647 to get overrideConditions into resolveId flow somehow. Please let me know if this approach isn't right.

On second thought, this didn't feel like a right problem to solve. Probably, what Vitest (or SSR in general) wants to do is to simply exclude 'module' condition from the default list when doing resolveExportsOrImports, but this will be probably too breaking:

https://github.com/vitejs/vite/blob/c3f8691b6da5973c25c3633920d1a94d075d71bc/packages/vite/src/node/plugins/resolve.ts#L1101-L1108

vitejs / vite