Skip to content

feat!: use module-runner instead of vite-node #8208

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

Merged
merged 86 commits into from
Jul 28, 2025

Conversation

sheremet-va
Copy link
Member

@sheremet-va sheremet-va commented Jun 22, 2025

Description

Fixes #7888
Fixes #6667

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

Copy link

netlify bot commented Jun 22, 2025

Deploy Preview for vitest-dev ready!

Name Link
🔨 Latest commit 88d21f4
🔍 Latest deploy log https://app.netlify.com/projects/vitest-dev/deploys/68875af520501c0008f99a9d
😎 Deploy Preview https://deploy-preview-8208--vitest-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

@sheremet-va
Copy link
Member Author

sheremet-va commented Jul 23, 2025

@AriPerkkio for some reason in the browser V8 marks the else branch as covered 🤔 (left one is happy-dom, right one is browser). Do you know how can this happen? Source maps? 🤔 It also runs the component twice it seems like 🤔

Screenshot 2025-07-23 at 16 04 25 Screenshot 2025-07-23 at 16 04 43

But the log prints only once and the else branch is not actually executed:

 RUN  v4.0.0-beta.3 /Users/vladimir/Projects/vitest/test/coverage-test
      Coverage enabled with v8

stdout | fixtures/test/vue-fixture.test.ts > define package in vm
Covered condition
 ✓ |chromium| fixtures/test/vue-fixture.test.ts > vue 3 coverage 10ms
 ✓ |chromium| fixtures/test/vue-fixture.test.ts > define package in vm 1ms
 ✓ |chromium| fixtures/test/vue-fixture.test.ts > vue non-SFC, uses query parameters in file imports 1ms

The coverage from v8 is also different:

// browser
    {
      scriptId: '77',
      url: '/fixtures/src/Vue/Defined.vue',
      functions: [
        {
          functionName: '',
          ranges: [ { startOffset: 0, endOffset: 2244, count: 1 } ],
          isBlockCoverage: true
        },
        {
          functionName: 'setup',
          ranges: [
            { startOffset: 247, endOffset: 584, count: 1 },
            { startOffset: 366, endOffset: 421, count: 0 }
          ],
          isBlockCoverage: true
        },
        {
          functionName: '_sfc_render',
          ranges: [ { startOffset: 735, endOffset: 849, count: 1 } ],
          isBlockCoverage: true
        }
      ]
    },

// happy-dom
    {
      scriptId: '866',
      url: 'file:///vitest/test/coverage-test/fixtures/src/Vue/Defined.vue',
      functions: [
        {
          functionName: '',
          ranges: [ { startOffset: 0, endOffset: 409, count: 1 } ],
          isBlockCoverage: true
        },
        {
          functionName: '',
          ranges: [ { startOffset: 13, endOffset: 409, count: 1 } ],
          isBlockCoverage: true
        },
        {
          functionName: 'get',
          ranges: [ { startOffset: 304, endOffset: 364, count: 0 } ],
          isBlockCoverage: false
        }
      ],
      startOffset: 209
    },

@AriPerkkio
Copy link
Member

AriPerkkio commented Jul 24, 2025

For some reason the Vue's CSS query is now again included in coverage. The failing test is especially for this:

<!-- Style block triggers a specific condition where sourcemaps used to break randomly -->
<style lang="css" scoped>
body {
background-color: #FFF;
}
</style>

Previously these were filtered out, not anymore. I did not check why this happens.

To reproduce, add following in packages/coverage-v8/src/browser.ts. Note that this is not proper fix, just a repro to show how these should be excluded from coverage.

function filterResult(coverage: Profiler.ScriptCoverage): boolean {
+  if (coverage.url.includes('.css')) {
+    return false
+  }

@sheremet-va
Copy link
Member Author

Previously these were filtered out, not anymore. I did not check why this happens.

Do you remember why they were filtered before? Is it some kind of a glob?

@AriPerkkio
Copy link
Member

Looking at history, it seems it was related to the order of file loading:

Relying on order seems unreliable. Let's actually do similar CSS request exclusion that we do for istanbul:

onFileTransform(sourceCode: string, id: string, pluginCtx: any): { code: string; map: any } | undefined {
// Istanbul/babel cannot instrument CSS - e.g. Vue imports end up here.
// File extension itself is .vue, but it contains CSS.
// e.g. "Example.vue?vue&type=style&index=0&scoped=f7f04e08&lang.css"
if (isCSSRequest(id)) {
return
}

@sheremet-va sheremet-va force-pushed the feat/use-module-runner branch from f65d582 to 53e4e10 Compare July 25, 2025 09:33
@sheremet-va
Copy link
Member Author

sheremet-va commented Jul 25, 2025

Looking at history, it seems it was related to the order of file loading:

* [c8 coverage seems broken when testing vue file with style tag #2539](https://github.com/vitest-dev/vitest/issues/2539)

Relying on order seems unreliable. Let's actually do similar CSS request exclusion that we do for istanbul:

onFileTransform(sourceCode: string, id: string, pluginCtx: any): { code: string; map: any } | undefined {
// Istanbul/babel cannot instrument CSS - e.g. Vue imports end up here.
// File extension itself is .vue, but it contains CSS.
// e.g. "Example.vue?vue&type=style&index=0&scoped=f7f04e08&lang.css"
if (isCSSRequest(id)) {
return
}

Now this breaks even more code in Vue 😄

The number of branches increased to 10 in every provider (weird, because I only changed v8), but v8 and v8-browser now have a difference:

// v8
   {
     "branches": "6/10 (60%)",
      "functions": "5/7 (71.42%)",
     "lines": "14/17 (82.35%)",
     "statements": "15/18 (83.33%)",
    }
// v8-browser
    {
      "branches": "6/10 (60%)",
      "functions": "5/7 (71.42%)",
      "lines": "13/16 (81.25%)",
      "statements": "14/17 (82.35%)",
    }

@AriPerkkio
Copy link
Member

All coverage-test/ were passing fine when I pushed 🤔

Taking a look

@sheremet-va
Copy link
Member Author

All coverage-test/ were passing fine when I pushed 🤔

Taking a look

I did a bit of a different change - I am filtering CSS requests here 60794d6

@AriPerkkio
Copy link
Member

Could it be related to the import.meta.env change you pushed too? This file is now showing too many branches unexpectedly:

uncoveredMethodUsingImportMeta() {
return `Source maps tend to break when import meta is used: ${import.meta.env.BASE_URL}`
},

@sheremet-va
Copy link
Member Author

sheremet-va commented Jul 25, 2025

Could it be related to the import.meta.env change you pushed too? This file is now showing too many branches unexpectedly:

uncoveredMethodUsingImportMeta() {
return `Source maps tend to break when import meta is used: ${import.meta.env.BASE_URL}`
},

Oh, yes, that makes sense. But why would v8 have a different result from others? (Even from v8-browser). It also has more lines and statements than the others.

The generated code now looks like this:

uncoveredMethodUsingImportMeta() { 
   return `Source maps tend to break when import meta is used: ${Object.assign(globalThis.__vitest_worker__?.metaEnv ?? import.meta.env).BASE_URL}` 
 }, 

Edit: v8 has more statements and lines than v8-browser

@sheremet-va
Copy link
Member Author

@AriPerkkio thank you for commit! All works now 🎉

@sheremet-va sheremet-va marked this pull request as ready for review July 25, 2025 12:49
@sheremet-va sheremet-va merged commit 9be01ba into vitest-dev:main Jul 28, 2025
32 of 34 checks passed
@sheremet-va sheremet-va deleted the feat/use-module-runner branch July 28, 2025 11:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants