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

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

Open
7 tasks done
hi-ogawa opened this issue May 8, 2024 · 3 comments · May be fixed by #16647

Comments

@hi-ogawa
Copy link
Collaborator

hi-ogawa commented May 8, 2024

Describe the bug

I was investigating Vitest issue vitest-dev/vitest#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

Copy link

stackblitz bot commented May 8, 2024

Fix this issue in StackBlitz Codeflow Start a new pull request in StackBlitz Codeflow.

@sapphi-red
Copy link
Member

It might be related to #12957 (comment) (haven't checked the repro deeply)

@hi-ogawa hi-ogawa linked a pull request May 10, 2024 that will close this issue
4 tasks
@hi-ogawa
Copy link
Collaborator Author

hi-ogawa commented May 10, 2024

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 #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:

const additionalConditions = new Set(
options.overrideConditions || [
'production',
'development',
'module',
...options.conditions,
],
)

@hi-ogawa hi-ogawa changed the title pluginContainer.resolveId works differently before and after ssrLoadModule pluginContainer.resolveId(..., { ssr: true }) works differently before and after ssrLoadModule May 21, 2024
@hi-ogawa hi-ogawa changed the title pluginContainer.resolveId(..., { ssr: true }) works differently before and after ssrLoadModule pluginContainer.resolveId(..., { ssr: true }) works differently from ssrLoadModule resolution when "module" entry May 23, 2024
@hi-ogawa hi-ogawa changed the title pluginContainer.resolveId(..., { ssr: true }) works differently from ssrLoadModule resolution when "module" entry pluginContainer.resolveId(..., { ssr: true }) works differently from ssrLoadModule resolution for "module" entry/exports May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants