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

ssr: resolve also .cjs and .mjs file extensions #4452

Closed
6 tasks done
ivanhofer opened this issue Jul 31, 2021 · 4 comments · Fixed by #4772
Closed
6 tasks done

ssr: resolve also .cjs and .mjs file extensions #4452

ivanhofer opened this issue Jul 31, 2021 · 4 comments · Fixed by #4772
Labels
feat: ssr p3-downstream-blocker Blocking the downstream ecosystem to work properly (priority)

Comments

@ivanhofer
Copy link
Contributor

Describe the bug

vite does not resolve .cjs extensions in ssr mode. I got it working by adding '.cjs' to ssrExtensions inside src/node/utils.ts.

const ssrExtensions = ['.js', '.cjs', '.json', '.node']

I'm not sure why the extension is missing. Maybe vite should also include the .mjs file extension.

The issue blocks users to add typesafe-i18n to their vite projects. That library exports only .cjs and .mjs files to support both CommonJS and ESM projects.

Reproduction

https://github.com/ivanhofer/typesafe-i18n-vite-ssr

run npm run dev and go to http://localhost:3000

System Info

System:
    OS: Linux 5.4 Ubuntu 20.04.2 LTS (Focal Fossa)
    CPU: (12) x64 Intel(R) Core(TM) i7-9850H CPU @ 2.60GHz
    Memory: 18.84 GB / 24.85 GB
    Container: Yes
    Shell: 5.0.17 - /bin/bash
  Binaries:
    Node: 16.5.0 - ~/.nvm/versions/node/v16.5.0/bin/node
    npm: 7.19.1 - ~/.nvm/versions/node/v16.5.0/bin/npm

Used Package Manager

npm

Logs

7:33:59 AM [vite] Error when evaluating SSR module /src/i18n/formatters.ts:
Error: Cannot find module 'typesafe-i18n/formatters' from '/home/ivanh/projects/typesafe-i18n-vite-ssr/src/i18n'
    at Function.resolveSync [as sync] (/home/ivanh/projects/typesafe-i18n-vite-ssr/node_modules/resolve/lib/sync.js:102:15)
    at resolveFrom$3 (/home/ivanh/projects/typesafe-i18n-vite-ssr/node_modules/vite/dist/node/chunks/dep-f2b4ca46.js:4076:29)
    at resolve (/home/ivanh/projects/typesafe-i18n-vite-ssr/node_modules/vite/dist/node/chunks/dep-f2b4ca46.js:73438:22)
    at nodeRequire (/home/ivanh/projects/typesafe-i18n-vite-ssr/node_modules/vite/dist/node/chunks/dep-f2b4ca46.js:73417:25)
    at ssrImport (/home/ivanh/projects/typesafe-i18n-vite-ssr/node_modules/vite/dist/node/chunks/dep-f2b4ca46.js:73369:20)
    at eval (/src/i18n/formatters.ts:3:31)
    at instantiateModule (/home/ivanh/projects/typesafe-i18n-vite-ssr/node_modules/vite/dist/node/chunks/dep-f2b4ca46.js:73402:166)
    at processTicksAndRejections (node:internal/process/task_queues:96:5) (x2)

Validations

@ygj6
Copy link
Member

ygj6 commented Aug 28, 2021

Currently vite uses require() to import module in ssr mode.

const mod = require(resolve(id, importer, root))

According to nodejs' doc, It is not possible to require() files that have the .mjs extension. Attempting to do so will throw an error.
So we can only include the .cjs file extension here.

@ivanhofer
Copy link
Contributor Author

I'm fine if . cjs gets added.
I already thought . mjs is a bit trickier. But does anything speak against using import when an esmmodule gets detected?

@ygj6
Copy link
Member

ygj6 commented Aug 30, 2021

Here I found a TODO note left by Evan You @ivanhofer

*
* TODO right now externals are imported using require(), we probably need to
* rework this when more libraries ship native ESM distributions for Node.
*/

@ivanhofer
Copy link
Contributor Author

@ygj6 thanks for taking a deeper look 😊
probably not needed right now but nice to have for the future.
Until then I'm happy with the resolving of . cjs files

@github-actions github-actions bot locked and limited conversation to collaborators Sep 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feat: ssr p3-downstream-blocker Blocking the downstream ecosystem to work properly (priority)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants