coverage-istanbul does not work with workspaces and SvelteKit

Ernxst commented 3 months ago

Describe the bug

In a monorepo with a simple SvelteKit app, running coverage with @vitest/coverage-istanbul does not work. Tests pass but collecting coverage fails with the following error:

SyntaxError: /Users/ernest/Projects/Repros/vitest-workspaces-istanbul/packages/web/src/routes/+page.svelte: Support for the experimental syntax 'jsx' isn't currently enabled (1:1):

> 1 | <h1>Welcome to SvelteKit</h1>
    | ^
  2 | <p>Visit <a href="https://kit.svelte.dev">kit.svelte.dev</a> to read the documentation</p>
  3 |

Add @babel/preset-react (https://github.com/babel/babel/tree/main/packages/babel-preset-react) to the 'presets' section of your Babel config to enable transformation.
If you want to leave it as-is, add @babel/plugin-syntax-jsx (https://github.com/babel/babel/tree/main/packages/babel-plugin-syntax-jsx) to the 'plugins' section to enable parsing.

If you already added the plugin for this syntax to your config, it's possible that your config isn't being loaded.
You can re-run Babel with the BABEL_SHOW_CONFIG_FOR environment variable to show the loaded configuration:
        npx cross-env BABEL_SHOW_CONFIG_FOR=/Users/ernest/Projects/Repros/vitest-workspaces-istanbul/packages/web/src/routes/+page.svelte <your build command>
See https://babeljs.io/docs/configuration#print-effective-configs for more info.

1  |  <h1>Welcome to SvelteKit</h1>
   |  ^
2  |  <p>Visit <a href="https://kit.svelte.dev">kit.svelte.dev</a> to read the documentation</p>
3  |

Works fine with @vitest/coverage-v8 but I need istanbul (required by @cloudflare/vitest-pool/workers)

Reproduction

Clone repo
Install dependencies (bun install)
Run test script (bun run test) - everything passes
Run coverage script (bun run coverage)

System Info

System:
    OS: macOS 14.5
    CPU: (8) arm64 Apple M1
    Memory: 40.45 MB / 16.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.12.2 - ~/Library/Caches/fnm_multishells/86163_1717766938803/bin/node
    npm: 10.5.0 - ~/Library/Caches/fnm_multishells/86163_1717766938803/bin/npm
    pnpm: 8.9.0 - ~/Library/pnpm/pnpm
    bun: 1.1.12 - ~/.bun/bin/bun
  Browsers:
    Chrome: 125.0.6422.77
    Edge: 125.0.2535.92
    Safari: 17.5
  npmPackages:
    @vitest/coverage-istanbul: 1.5.0 => 1.5.0 
    vitest: 1.5.0 => 1.5.0

Used Package Manager

npm

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] 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.

AriPerkkio commented 3 months ago

As work-around for now you can set coverage.all: false or configure Svelte plugin in the root vitest.config.ts.

I think this is caused by the root vitest.config.ts not having Svelte Vite plugin, and it fails when transforming uncovered Svelte files here:

https://github.com/vitest-dev/vitest/blob/7cbd943c7bd37ad666dce31537037a53bfdec7cd/packages/coverage-istanbul/src/provider.ts#L304

Ernxst commented 3 months ago

I'd rather have coverage for all my source files so I went with adding the Svelte plugin to the root vitest.config.ts. It worked in the repro but in my actual project it doesn't - I have an api package which uses @cloudflare/vitest-pool-workers and the api tests fail with:

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Unhandled Errors ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯

Vitest caught 1 unhandled error during the test run.
This might cause false positive tests. Resolve unhandled errors to make sure your tests are not affected.

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Unhandled Error ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
TypeError: Cannot use require() to import an ES Module.
 ❯ node_modules/chai/lib/chai/utils/inspect.js node_modules/chai/index.mjs%3Fmf_vitest_no_cjs_esm_shim:217:17
 ❯ __require node_modules/chai/index.mjs%3Fmf_vitest_no_cjs_esm_shim:8:50
 ❯ node_modules/chai/lib/chai/utils/objDisplay.js node_modules/chai/index.mjs%3Fmf_vitest_no_cjs_esm_shim:235:19
 ❯ __require node_modules/chai/index.mjs%3Fmf_vitest_no_cjs_esm_shim:8:50
 ❯ node_modules/chai/lib/chai/utils/getMessage.js node_modules/chai/index.mjs%3Fmf_vitest_no_cjs_esm_shim:262:22
 ❯ __require node_modules/chai/index.mjs%3Fmf_vitest_no_cjs_esm_shim:8:50
 ❯ node_modules/chai/lib/chai/utils/index.js node_modules/chai/index.mjs%3Fmf_vitest_no_cjs_esm_shim:749:26
 ❯ __require node_modules/chai/index.mjs%3Fmf_vitest_no_cjs_esm_shim:8:50
 ❯ node_modules/chai/lib/chai.js node_modules/chai/index.mjs%3Fmf_vitest_no_cjs_esm_shim:2725:17
 ❯ __require node_modules/chai/index.mjs%3Fmf_vitest_no_cjs_esm_shim:8:50

When running vitest from the monorepo root - the tests pass when I run it from the packages/api directory. I created a feature/api branch in the repro and found the same issue there with a minimal setup. After some investigation, it seems that the [loupe]() package chai imports has invalid ESM exports (see this issue and the PR). Removing the module field in its package.json fixes everything so I'm not sure whether this is a vitest issue, a chai issue, a loupe issue or a vitest-pool-workers issue as everything works perfectly when I don't add the Svelte plugin to the root vitest.config.ts.

Ernxst commented 3 months ago

After some more testing, it seems adding the plugin to the root breaks anything that doesn't have exactly the correct package exports. For example, I have a package using unplugin-icons and running coverage on it now fails with:

Error: No known conditions for "./types/svelte" specifier in "unplugin-icons" package

Looking in the package.json of the module, I see it has the following export:

"./types/svelte": {
  "types": "./types/svelte.d.ts"
}

With no import condition which sounds reasonable since it's for types only. However, it breaks because of this so the fix is to add "import": "./types/svelte.d.ts". Next, I have a package which uses msw and coverage now fails with:

Error: No known conditions for "./browser" specifier in "msw" package

Which has the following package.json export:

 "./browser": {
    "types": "./lib/browser/index.d.ts",
    "browser": {
      "require": "./lib/browser/index.js",
      "import": "./lib/browser/index.mjs"
    },
    "node": null,
    "require": "./lib/browser/index.js",
    "import": "./lib/browser/index.mjs",
    "default": "./lib/browser/index.js"
}

And this is fixed by simply moving the import condition up, above the browser condition. I doubt patching every failing package.json is the solution here

sheremet-va commented 3 months ago

I just want to add that this is documented: https://vitest.dev/guide/workspace.html#coverage

vitest-dev / vitest