vitest-dev / vitest

Next generation testing framework powered by Vite.
https://vitest.dev
MIT License
13.13k stars 1.18k forks source link

[ES Module but shipped in a CommonJS package] Lib importing another ESM-only library #6875

Open varugasu opened 2 weeks ago

varugasu commented 2 weeks ago

Describe the bug

I was creating a library with unbuild using rollup and generating dist/*.cjs and dist/*.mjs files. As I didn't want to import directly from lib/dist, I created the following package.json:

  "type": "module",
  "exports": {
    ".": {
      "require": "./dist/index.cjs",
      "node": "./dist/index.cjs",
      "import": "./dist/index.mjs",
      "types": "./dist/index.d.ts"
    },
    "./*": {
      "require": "./dist/*/index.cjs",
      "node": "./dist/*/index.cjs",
      "import": "./dist/*/index.mjs",
      "types": "./dist/*/index.d.ts"
    }
  },
  "files": [
    "dist"
  ],

This lib depends on another lib, esm-only.

esm-only is packaged as ESM but with .js extension and I can't control how this library bundled

My App project uses lib. When I ran vitest on my App project, I got this error:

 FAIL  src/lib.test.ts [ src/lib.test.ts ]
SyntaxError: Unexpected token 'export'
 ❯ ../lib/dist/utils/index.cjs:3:15
      1| 'use strict';
      2| 
      3| const utils = require('esm-only/dist/utils');
       |               ^
      4| 
      5| const multiply = (a, b) => {

Module node-packages/esm-only/dist/utils/index.js:1 seems to be an ES Module but shipped in a CommonJS package. To fix this issue, change the file extension to .mjs or add "type": "module" in your package.json.

The only thing I was able to make it work was to change lib/package.json to:

  "type": "module",
  "main": "dist/index.cjs",
  "module": "dist/index.mjs",
  "types": "dist/index.d.ts",
  "files": [
    "dist"
  ],

And then change src/lib.test.ts to:

import { multiply } from "lib/dist/utils";

But as you can see, I have lib/dist/utils, I want to be able to import lib/utils instead.

Another option is to change lib/build.config.ts to use mkdist:

import { defineBuildConfig } from "unbuild";

export default defineBuildConfig({
  entries: [
    {
      builder: "mkdist",
      input: "src/",
      distDir: "dist",
      ext: "js",
      format: "esm",
    },
  ],
  outDir: "dist",
  declaration: true,
});

And my lib/package.json to:

 "type": "module",
  "exports": {
    ".": {
      "require": "./dist/index.js",
      "node": "./dist/index.js",
      "import": "./dist/index.js",
      "types": "./dist/index.d.ts"
    },
    "./*": {
      "require": "./dist/*/index.js",
      "node": "./dist/*/index.js",
      "import": "./dist/*/index.js",
      "types": "./dist/*/index.d.ts"
    }
  },
  "files": [
    "dist"
  ],

Finally, this ONLY DOESN'T work on Vitest. On Vite I can normally run both in dev and production mode.

Related issues:

Reproduction

I created this repository to reproduce the issue I was having in another project: https://github.com/varugasu/node-packages

System Info

System:
    OS: macOS 15.1
    CPU: (10) arm64 Apple M1 Max
    Memory: 2.96 GB / 64.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.11.0 - ~/Library/Caches/fnm_multishells/50248_1730980063968/bin/node
    npm: 10.2.4 - ~/Library/Caches/fnm_multishells/50248_1730980063968/bin/npm
    pnpm: 9.9.0 - ~/Library/pnpm/pnpm
    bun: 1.1.26 - /opt/homebrew/bin/bun
    Watchman: 2024.10.21.00 - /opt/homebrew/bin/watchman
  Browsers:
    Safari: 18.1
  npmPackages:
    @vitejs/plugin-react: ^4.3.3 => 4.3.3 
    vite: ^5.4.10 => 5.4.10 
    vitest: ^2.1.4 => 2.1.4

Used Package Manager

pnpm

Validations

sheremet-va commented 2 weeks ago

Vitest cannot process ES modules that were imported using require because ES module transformation is asynchronous.

sheremet-va commented 2 weeks ago

To bypass that, you can enable the optimizer:

export default defineConfig({
  test: {
    deps: {
      web: {
        enabled: true,
        include: ['lib'],
      },
    },
  },
})
varugasu commented 2 weeks ago

Hey @sheremet-va , thank you for responding. I tried applying it and it also didn't work

hi-ogawa commented 2 weeks ago

esm-only is packaged as ESM but with .js extension and I can't control how this library bundled

@varugasu Assuming your esm-only is only for the purpose of repro, what is the actual npm package you use?

hi-ogawa commented 2 weeks ago

Also what is the reason you have node condition which directs to cjs? How about doing it like this?

  "exports": {
    ".": {
      "require": "./dist/index.cjs",
      "import": "./dist/index.mjs",
      "types": "./dist/index.d.ts"
    },
    "./*": {
      "require": "./dist/*/index.cjs",
      "import": "./dist/*/index.mjs",
      "types": "./dist/*/index.d.ts"
    }
  },
varugasu commented 2 weeks ago

@hi-ogawa , the actual package is an internal one, so I can't share it.

About your suggestion, it indeed solves my reproduction, but testing on my actual project, I have the following error:

Error: Directory import '.../node_modules/@mui/material/CircularProgress' is not supported resolving ES modules imported from .../node_modules/...index.mjs
Did you mean to import @mui/material/node/CircularProgress/index.js?

(I redacted the sensitive information with ...)

I added node because I was having this issue:

SyntaxError: Unexpected token 'export'

Module .../index.js:1 seems to be an ES Module but shipped in a CommonJS package. You might want to create an issue to the package "internal-lib-esm-only" asking them to ship the file in .mjs extension or add "type": "module" in their package.json.

As a temporary workaround you can try to inline the package by updating your config:

// vitest.config.js
export default {
  test: {
    server: {
      deps: {
        inline: [
          "internal-lib-esm-only"
        ]
      }
    }
  }
}
hi-ogawa commented 2 weeks ago

From what I can tell, both esm-only and @mui/material errors are due to the wrong usage and technically they should be fixed to follow a node module resolution even though many bundlers might tolerate this.

That said, the error from @mui/material import in your library is usually worked around by server.deps.inline. It would be nice to have a reproduction that to understand the issue though.