fix(coverage): fix bundling of `v8-to-istanbul` by AriPerkkio · Pull Request #5549 · vitest-dev/vitest

AriPerkkio · 2024-04-15T14:18:37Z

Description

Fixes V8 Coverage fails #5532

Looks like Rollup doesn't transpile require('fs').promises.

Changes in bundle:

-const compatError = Error(`requires Node.js ${require$$9.engines.node}`);
-const { readFileSync } = require$$10;
-let readFile = () => { throw compatError };
-try {
-  readFile = require('fs').promises.readFile;
-} catch (_err) {
-  // most likely we're on an older version of Node.js.
-}
+import require$$9 from 'fs';
...
+const { readFileSync, promises } = require$$9;
+const readFile = promises.readFile;

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

It's really useful if your PR references an issue where it is discussed ahead of time. If the feature is substantial or introduces breaking changes without a discussion, PR might be closed.
Ideally, include a test that fails without this PR but passes with it.
Please, don't make changes to pnpm-lock.yaml unless you introduce a new test example.

Tests

Run the tests with pnpm test:ci.

Documentation

If you introduce new functionality, document it. You can run documentation with pnpm run docs command.

Changesets

Changes in changelog are generated from PR name. Please, make sure that it explains your changes in an understandable manner. Please, prefix changeset messages with feat:, fix:, perf:, docs:, or chore:.

netlify · 2024-04-15T14:18:55Z

✅ Deploy Preview for fastidious-cascaron-4ded94 ready!

Name	Link
🔨 Latest commit	`fb6c375`
🔍 Latest deploy log	https://app.netlify.com/sites/fastidious-cascaron-4ded94/deploys/661d373f19413600089cc1cc
😎 Deploy Preview	https://deploy-preview-5549--fastidious-cascaron-4ded94.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

hi-ogawa

Looks good to me!

Regarding this comment #5532 (comment)

This is working fine in Vitest's monorepo and in ecosystem-ci projects.

I think, for the tests inside this repo, Vitest (vite-node) see @vitest/coverage-v8 as non external, so it's probably injecting cjs compat including require:

vitest/packages/vite-node/src/client.ts

Lines 392 to 393 in c84113f

    
           // cjs compact 
        
           require: createRequire(href),

I thought ecosystem-ci would catch this, but maybe we don't test @vitest/coverage-v8? It looks like the one in coverage-large is using only istanbul https://github.com/vitest-tests/coverage-large/blob/6dc5a055b30556e78ee3a0b6fff3968fac222c07/vitest.config.ts#L29

AriPerkkio · 2024-04-16T14:06:36Z

Looks like v8-to-istanbul also calls this method on certain conditions so it's possible that Vitest monorepo is not running into that.

dschmidt · 2024-04-22T10:28:48Z

When can we expect a release with a fix?

Upstream fix in vitest-dev/vitest#5549

fix(coverage): fix bundling of v8-to-istanbul

fb6c375

AriPerkkio marked this pull request as ready for review April 15, 2024 14:56

hi-ogawa approved these changes Apr 16, 2024

View reviewed changes

AriPerkkio merged commit df6a432 into vitest-dev:main Apr 16, 2024

AriPerkkio deleted the fix/v8-to-istanbul-require branch April 16, 2024 14:15

dschmidt added a commit to owncloud/web that referenced this pull request Apr 22, 2024

fix: bundling error in @vitest/coverage-v8

eddc6ab

Upstream fix in vitest-dev/vitest#5549

dschmidt added a commit to owncloud/web that referenced this pull request Apr 22, 2024

fix: bundling error in @vitest/coverage-v8

cf1047f

Upstream fix in vitest-dev/vitest#5549

renovate bot referenced this pull request in ariakit/ariakit Apr 24, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Comments

fix(coverage): fix bundling of `v8-to-istanbul`#5549

fix(coverage): fix bundling of `v8-to-istanbul`#5549
AriPerkkio merged 1 commit intovitest-dev:mainfrom
AriPerkkio:fix/v8-to-istanbul-require

AriPerkkio commented Apr 15, 2024 •

edited

Loading

Uh oh!

netlify bot commented Apr 15, 2024 •

edited

Loading

Uh oh!

hi-ogawa left a comment

Uh oh!

AriPerkkio commented Apr 16, 2024

Uh oh!

dschmidt commented Apr 22, 2024

Uh oh!

Uh oh!

Comments

Conversation

AriPerkkio commented Apr 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

Tests

Documentation

Changesets

Uh oh!

netlify bot commented Apr 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for fastidious-cascaron-4ded94 ready!

Uh oh!

hi-ogawa left a comment

Choose a reason for hiding this comment

Uh oh!

AriPerkkio commented Apr 16, 2024

Uh oh!

dschmidt commented Apr 22, 2024

Uh oh!

AriPerkkio commented Apr 15, 2024 •

edited

Loading

netlify bot commented Apr 15, 2024 •

edited

Loading