Investigate root cause for vite 4.3.x can't build date-fns/locale in ssr (temp. patched)

indeyets commented 1 year ago

Describe the bug

I'm importing locales from date-fns like this: import { enGB, es, de } from "date-fns/locale"; in my code.

It worked fine with vite 4.2.x, but both 4.3.0 and 4.3.1 replace the "from" part with … from "s"; which server, once it runs, can not find, for obvious reasons.

Reproduction

https://github.com/indeyets/vite-bug

Steps to reproduce

run npm install followed by npm test

System Info

I reproduced it both on macos and linux.

  System:
    OS: macOS 13.4
    CPU: (12) x64 Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
    Memory: 482.73 MB / 32.00 GB
    Shell: 3.6.1 - /usr/local/bin/fish
  Binaries:
    Node: 19.9.0 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 9.6.3 - /usr/local/bin/npm
  Browsers:
    Chrome: 112.0.5615.137
    Edge: 112.0.1722.58
    Firefox: 110.0
    Safari: 16.5
    Safari Technology Preview: 16.4

Used Package Manager

npm

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.

bluwy commented 1 year ago

Does it work in Vite 4.2? Using 4.2 and running your repro I get:

Error [ERR_UNSUPPORTED_DIR_IMPORT]: Directory import '/Users/bjorn/Work/repros/vite-3-date-fns/node_modules/date-fns/locale' is not supported resolving ES modules imported from /Users/bjorn/Work/repros/vite-3-date-fns/dist/server.js
Did you mean to import date-fns/node_modules/.pnpm/date-fns@2.29.3/node_modules/date-fns/locale/index.js?

indeyets commented 1 year ago

@bluwy good point. it works in my complex app, but not here. I reproduced bad behaviour without reproducing good.

I'll see what I can do 🤔

but it looks like @btea figured out the problem anyway?

bluwy commented 1 year ago

I wasn't sure if the PR was the right fix, and in any case we should fix to report the right error as before instead. But happy to look into it again if you manage to find out how it works before. FWIW I tried fiddling with ssr.external and ssr.noExternal in the repro but I can't repro that case where it works in 4.2 but not 4.3.

sun0day commented 1 year ago

btea's PR makes the Vite4.3's resolve behavior the same as Vite4.2's, but it seems like there is another potential bug as bluwy said. Could you provide your complex app or maybe vite.config.js? @indeyets

jaybe78 commented 1 year ago

Hello,

I'm also working on a SSR application where the server is built using webpack. Since I've upgrade to 4.3 version I'm also getting that issue

Module not found: Error: Can't resolve 's' in '/Users/jean-bricerougeot/Documents/bulbshare-web-app-react/websites/my.bulbshare.com/dist/server/src/renderer'
 @ ./dist/server/pageFiles.mjs
 @ ./dist/server/importBuild.cjs 4:21-46
 @ ../../node_modules/@brillout/vite-plugin-import-build/dist/autoImporter.js 2:34-110 8:37-121
 @ ../../node_modules/@brillout/vite-plugin-import-build/dist/loadServerBuild/index.js 9:21-47
 @ ../../node_modules/@brillout/vite-plugin-import-build/loadServerBuild.js 1:0-50
 @ ../../node_modules/vite-plugin-ssr/dist/cjs/node/runtime/globalContext/loadImportBuild.js 4:26-87
 @ ../../node_modules/vite-plugin-ssr/dist/cjs/node/runtime/globalContext.js 5:26-68
 @ ../../node_modules/vite-plugin-ssr/dist/cjs/node/runtime/renderPage.js 10:24-50

I confirm it works correctly When I downgrade to 4.2.0

sun0day commented 1 year ago

Could you provide a mini-repro? @jaybe78

bluwy commented 1 year ago

@DMOAbove that looks like a different issue and an expected one. And I don't think v4.2.2 could be regressing it, but if you found it so, please create a new issue with the repro. Thanks!

An update for this issue. It was fixed at https://github.com/vitejs/vite/pull/13010 but the fix can be improved further based on the root cause, so it's left opened. Updating the labels for now.

vitejs / vite