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

Issues with ESM module import syntax and endless loop during load in SSR #3273

Closed
6 tasks done
aral opened this issue May 5, 2021 · 8 comments
Closed
6 tasks done

Comments

@aral
Copy link

aral commented May 5, 2021

Describe the bug

JSDB is a 100% JavaScript database. The latest version is written in ESM. Simply importing it in the SSR server script (in vite-plugin-ssr) causes errors that do not otherwise exist in other contexts.

(I am trying to use an ESM node module that otherwise works fine during SSR with Vite.)

Reproduction

  1. Create an instance of the vite-plugin-ssr vue starter app:
npm init vite-plugin-ssr
  1. Change the type to module in package.json:
{
  
  "type": "module",
  
}
  1. Install JSDB:
npm i @small-tech/jsdb
  1. Add the following line to pages/_default.page.server.js:
import JSDB from '@small-tech/jsdb'
  1. Run the dev server:
npm run dev

What should happen?

There should not be any errors.

What actually happens?

Initially, you get the following error:

Error: Failed to resolve import "fs/promises" from "pages/_default/_default.page.server.js". Does the file exist?
    at formatError (/home/aral/small-tech/small-web/sitekit/sandbox/vite-ssr-project/node_modules/vite/dist/node/chunks/dep-2c03f3f9.js:43582:46)

This is because the JSTable.js file in JSDB has the following import:

import fsPromises from 'fs/promises'

I don’t know if this is an issue with vite-plugin-ssr or vite itself but I’m leaning towards the latter. The workaround is to change the import to:

import fs from 'fs'
const fsPromises = fs.promises

Needless to say, while I can do this in a module I control, this would mean I’d have to update all third-party modules that might use these sorts of imports in the future to make them compatible.

Once that error is out of the way, you get several related errors:

1:53:15 PM [vite] error while updating dependencies:
Error: Build failed with 3 errors:
node_modules/@small-tech/jsdb/lib/JSTable.js:26:9: error: No matching export in "browser-external:perf_hooks" for import "performance"
node_modules/@small-tech/jsdb/lib/Time.js:15:9: error: No matching export in "browser-external:perf_hooks" for import "performance"
node_modules/@small-tech/jsdb/lib/Util.js:15:9: error: No matching export in "browser-external:util" for import "types"
    at failureErrorWithLog (/home/aral/small-tech/small-web/sitekit/sandbox/vite-ssr-project/node_modules/esbuild/lib/main.js:1224:15)
    at buildResponseToResult (/home/aral/small-tech/small-web/sitekit/sandbox/vite-ssr-project/node_modules/esbuild/lib/main.js:936:32)
    at /home/aral/small-tech/small-web/sitekit/sandbox/vite-ssr-project/node_modules/esbuild/lib/main.js:1035:20
    at /home/aral/small-tech/small-web/sitekit/sandbox/vite-ssr-project/node_modules/esbuild/lib/main.js:568:9
    at handleIncomingPacket (/home/aral/small-tech/small-web/sitekit/sandbox/vite-ssr-project/node_modules/esbuild/lib/main.js:657:9)
    at Socket.readFromStdout (/home/aral/small-tech/small-web/sitekit/sandbox/vite-ssr-project/node_modules/esbuild/lib/main.js:535:7)
    at Socket.emit (events.js:315:20)
    at addChunk (internal/streams/readable.js:309:12)
    at readableAddChunk (internal/streams/readable.js:284:9)
    at Socket.Readable.push (internal/streams/readable.js:223:10)

This time it’s clearly an esbuild error from the stack trace. The previous error did not have the same stack trace but it might also have to do with vite’s use of esbuild.

I don’t think the problem is with esbuild itself as I bundle Place using esbuild and it imports JSDB without issues. And I also just tried it with esbuild 0.9.3 – the same version used here – and it worked without issues.

Again, the fix is to rewrite the imports:

So import { performance } from 'perf_hooks' becomes:

import perfHooks from 'perf_hooks'
const performance = perfHooks.performance

etc.

Once those changes have been made, I no longer get errors but loading the index page causes the browser to enter an infinite loading loop with no warnings or errors either in the browser or on the console.

There is a chance that this is an issue with vite-plugin-ssr, so I’ve opened an issue there also also but it feels like it is more a general Vite issue to me.

System Info

Output of npx envinfo --system --npmPackages vite,@vitejs/plugin-vue --binaries --browsers:

  System:
    OS: Linux 5.4 elementary OS 5.1.7 Hera
    CPU: (12) x64 Intel(R) Core(TM) i7-10710U CPU @ 1.10GHz
    Memory: 6.37 GB / 15.51 GB
    Container: Yes
    Shell: 5.4.2 - /usr/bin/zsh
  Binaries:
    Node: 14.16.0 - ~/.nvm/versions/node/v14.16.0/bin/node
    Yarn: 1.22.5 - /usr/bin/yarn
    npm: 7.6.0 - ~/.nvm/versions/node/v14.16.0/bin/npm
  Browsers:
    Firefox: 87.0

Additional system info

  • Vite: 2.2.4
  • Esbuild (via Vite): 0.9.3
  • vite-plugin-ssr: 0.1.0-beta.40
  • JSDB: 2.0.2

Used package manager: npm

Before submitting the issue, please make sure you do the following

  • 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.
  • Provide a description in this issue that describes the bug.
  • 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 https://github.com/vuejs/vue-next instead. (I’m pretty sure it’s a Vite-related issue.)
  • Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
@brillout
Copy link
Contributor

brillout commented May 5, 2021

Vite should treat @small-tech/jsdb as an SSR external but doesn't.

In the meantime you can manually mark it as SSR external.

export default {
  ssr: {
    external: ['@small-tech/jsdb']
  },
  plugins: [vue(), ssr()]
}

I suspect the problem to be around the fact that @small-tech/jsdb ships as ESM; https://vitejs.dev/guide/ssr.html#ssr-externals.

@patak-js How about we treat all dependencies as SSR external? I don't see the benefit of processing Node.js dependencies.

@aral
Copy link
Author

aral commented May 5, 2021

@brillout Thanks, that got me a little further. But now I have the following error (my project is the vite-plugin-ssr vue starter template but with"type": "module" set in the package.json and using ESM throughout – just updated this issue to reflect that):

4:58:58 PM [vite] Error when evaluating SSR module /pages/_default/_default.page.server.js:
Error [ERR_REQUIRE_ESM]: Must use import to load ES Module: /home/aral/small-tech/small-web/sitekit/sandbox/vite-ssr-project/node_modules/@small-tech/jsdb/index.js
require() of ES modules is not supported.
require() of /home/aral/small-tech/small-web/sitekit/sandbox/vite-ssr-project/node_modules/@small-tech/jsdb/index.js from /home/aral/small-tech/small-web/sitekit/sandbox/vite-ssr-project/node_modules/vite/dist/node/chunks/dep-2c03f3f9.js is an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which defines all .js files in that package scope as ES modules.
Instead rename index.js to end in .cjs, change the requiring code to use import(), or remove "type": "module" from /home/aral/small-tech/small-web/sitekit/sandbox/vite-ssr-project/node_modules/@small-tech/jsdb/package.json.

    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1080:13)
    at Module.load (internal/modules/cjs/loader.js:928:32)
    at Function.Module._load (internal/modules/cjs/loader.js:769:14)
    at Module.require (internal/modules/cjs/loader.js:952:19)
    at require (internal/modules/cjs/helpers.js:88:18)
    at nodeRequire (/home/aral/small-tech/small-web/sitekit/sandbox/vite-ssr-project/node_modules/vite/dist/node/chunks/dep-2c03f3f9.js:69080:17)
    at ssrImport (/home/aral/small-tech/small-web/sitekit/sandbox/vite-ssr-project/node_modules/vite/dist/node/chunks/dep-2c03f3f9.js:69038:20)
    at eval (/pages/_default/_default.page.server.js:29:31)
    at instantiateModule (/home/aral/small-tech/small-web/sitekit/sandbox/vite-ssr-project/node_modules/vite/dist/node/chunks/dep-2c03f3f9.js:69066:166) (x2)

The JSDB index.js mentioned in the error contains:

export { default } from './lib/JSDB.js'

@brillout
Copy link
Contributor

brillout commented May 5, 2021

Doesn't @small-tech/jsdb have a CJS main field? Vite transpiles your code to CJS, so @small-tech/jsdb needs to support CJS.

@aral
Copy link
Author

aral commented May 5, 2021

@brillout No, it’s a pure ESM module optimised for ESM-only Node projects, it doesn’t support CJS. I can see the problem if Vite transpiles all code to CJS. (JSDB also saves its database tables in ESM format. There is the 1.x branch which is CJS and saves its tables in UMD/CJS format so there’s no reason to have a CJS build of it.)

Do you know if ESM-only node modules will be supported at some time or is this a fact of life for the foreseeable future?

PS. I did read the following on the link you provided earlier:

In the future, this heuristics will likely improve to detect if the project has type: "module" enabled, so that Vite can also externalize dependencies that ship Node-compatible ESM builds by importing them via dynamic import() during SSR.

And also:

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

https://github.com/vitejs/vite/blob/main/packages/vite/src/node/ssr/ssrExternal.ts#L11

@brillout
Copy link
Contributor

brillout commented May 5, 2021

Yes and currently transpiled Node.js code is evaluated by using new Function, see vite/packages/vite/src/node/ssr/ssrModuleLoader.ts. So you'll have to use CJS for the time being.

I guess we can close this ticket or replace it with a more precice one.

@aral
Copy link
Author

aral commented May 5, 2021

OK, I’m going to close this. It might be worth having an issue to track support for pure ESM Node modules and using using import to load ESM externals but it looks like this is already unofficially on the radar at least.

@aral aral closed this as completed May 5, 2021
@patak-dev
Copy link
Member

@aral this PR may be related #2157

@Niputi
Copy link
Contributor

Niputi commented May 5, 2021

that pr is for build and not dev so I guess it's not going to help

@github-actions github-actions bot locked and limited conversation to collaborators Jul 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants