fix(browser): improve source maps when `vi.mock` is present #6810

sheremet-va · 2024-10-29T12:54:40Z

Description

This PR improves source map generation when vi.mock is detected. The mock is now present in the source map.

Fixes #6829

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-10-29T12:57:54Z

✅ Deploy Preview for vitest-dev ready!

Name	Link
🔨 Latest commit	`26c503d`
🔍 Latest deploy log	https://app.netlify.com/sites/vitest-dev/deploys/6720db12bc6a8b00086152ce
😎 Deploy Preview	https://deploy-preview-6810--vitest-dev.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.

pkg-pr-new · 2024-10-29T13:01:42Z

@vitest/browser

pnpm add https://pkg.pr.new/@vitest/browser@6810

@vitest/coverage-istanbul

pnpm add https://pkg.pr.new/@vitest/coverage-istanbul@6810

@vitest/expect

pnpm add https://pkg.pr.new/@vitest/expect@6810

@vitest/coverage-v8

pnpm add https://pkg.pr.new/@vitest/coverage-v8@6810

@vitest/mocker

pnpm add https://pkg.pr.new/@vitest/mocker@6810

@vitest/pretty-format

pnpm add https://pkg.pr.new/@vitest/pretty-format@6810

@vitest/runner

pnpm add https://pkg.pr.new/@vitest/runner@6810

@vitest/snapshot

pnpm add https://pkg.pr.new/@vitest/snapshot@6810

@vitest/spy

pnpm add https://pkg.pr.new/@vitest/spy@6810

@vitest/ui

pnpm add https://pkg.pr.new/@vitest/ui@6810

@vitest/utils

pnpm add https://pkg.pr.new/@vitest/utils@6810

vite-node

pnpm add https://pkg.pr.new/vite-node@6810

vitest

pnpm add https://pkg.pr.new/vitest@6810

@vitest/web-worker

pnpm add https://pkg.pr.new/@vitest/web-worker@6810

@vitest/ws-client

pnpm add https://pkg.pr.new/@vitest/ws-client@6810

commit: 33ca778

sheremet-va · 2024-10-29T13:10:41Z

Hm, this makes source maps for vi.mock better (it sees the hoisting now), but it is still partially broken 🤔 I am testing against this: https://github.com/vitest-tests/browser-examples/tree/main/examples/vue-msw-graphql (the stack trace is incorrect and source maps visualiser also goes crazy for some lines - primarily the first multi-line imports seems to be mapped weirdly). @AriPerkkio can you help me with this when you have the capacity? 🙏🏻

It is possibly related to the fact that we don't touch the import from vitest at all:

vitest/packages/mocker/src/node/hoistMocksPlugin.ts

Line 194 in bcc5908

if (hoistedModules.includes(node.source.value as string)) {

With globals: true we inject the if statement that actually breaks the source map - I am not sure how to fix that one 🤔

AriPerkkio

Is it correct that the original ast is returned as is? We are modifying the code a lot, but make no changes to ast. Does it help if it's not returned here? Leave it null?

vitest/packages/mocker/src/node/hoistMocksPlugin.ts

Lines 539 to 543 in 8878b04

    
           return { 
        
             ast, 
        
             code: s.toString(), 
        
             map: s.generateMap({ hires: 'boundary', source: id }), 
        
           }

sheremet-va · 2024-10-29T15:45:36Z

Is it correct that the original ast is returned as is?

Doesn't seem to change anything

hi-ogawa · 2024-10-30T00:43:21Z

It is possibly related to the fact that we don't touch the import from vitest at all:

Are you also testing with the latest Vite? If import "vitest" is still hoisted by Vite ssr transform, the fix vitejs/vite#16356 is only included in Vite 6.

sheremet-va · 2024-10-30T06:53:51Z

It is possibly related to the fact that we don't touch the import from vitest at all:

Are you also testing with the latest Vite? If import "vitest" is still hoisted by Vite ssr transform, the fix vitejs/vite#16356 is only included in Vite 6.

I’m testing the browser mode (I linked the example), it doesn't use Vite SSR transform. This PR actually includes changes from vitejs/vite#16356

hi-ogawa · 2024-10-31T03:14:08Z

I’m testing the browser mode (I linked the example)

Ah, I missed that 🤦 then their change doesn't look relevant.

AriPerkkio · 2024-10-31T10:17:44Z

Hm, this makes source maps for vi.mock better (it sees the hoisting now), but it is still partially broken 🤔 I am testing against this: https://github.com/vitest-tests/browser-examples/tree/main/examples/vue-msw-graphql (the stack trace is incorrect and source maps visualiser also goes crazy for some lines - primarily the first multi-line imports seems to be mapped weirdly).

To me it looks really good - what's wrong here? https://ariperkkio.github.io/vite-plugin-source-map-visualizer/#MTYyMQB2aS5tb2NrKCIuL2NvbnN0YW50cy...

With globals: true: https://ariperkkio.github.io/vite-plugin-source-map-visualizer/#MTg1NQAKaWY...

Also these two showcase nicely the changes:

Before link
After link

sheremet-va · 2024-10-31T10:23:55Z

To me it looks really good - what's wrong here?

The first source map looks different to me. The vitest import is mapped incorrectly ~~(I don't know how to share)~~. Which causes the error stack trace to point to the previous line instead of the correct one. I also get incorrect source maps when globals is turned on and vitest import is removed

My source maps look like this 🤔

sheremet-va · 2024-10-31T10:38:36Z

(Added a link to source maps, for some reason copying in Safari doesn't work, had to do it in Chrome)

I am also using the pkg.pr.new instead of linking

weilbith · 2024-11-18T09:12:56Z

Is there an easy way to notice when this was included in a release?

AriPerkkio · 2024-11-18T09:34:53Z

@weilbith release changelogs are at https://github.com/vitest-dev/vitest/releases. This commit is mentioned on latest beta release.

@Lordfirespeed

fix(browser): improve source maps when vi.mock is present

26c503d

sheremet-va added the cr-tracked label Oct 29, 2024

chore: cleanup

d727d14

sheremet-va added this to the 2.2.0 milestone Oct 29, 2024

AriPerkkio reviewed Oct 29, 2024

View reviewed changes

fix: inject import from "vitest" if "vi" is not imported when mocking

33ca778

sheremet-va merged commit 8d179af into vitest-dev:main Nov 13, 2024
18 checks passed

sheremet-va deleted the fix/source-maps-vi-mock branch November 13, 2024 16:02

sheremet-va modified the milestones: 2.2.0, 3.0.0 Dec 5, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(browser): improve source maps when `vi.mock` is present #6810

fix(browser): improve source maps when `vi.mock` is present #6810

sheremet-va commented Oct 29, 2024 •

edited

Loading

netlify bot commented Oct 29, 2024

pkg-pr-new bot commented Oct 29, 2024 •

edited

Loading

sheremet-va commented Oct 29, 2024 •

edited

Loading

AriPerkkio left a comment

sheremet-va commented Oct 29, 2024

hi-ogawa commented Oct 30, 2024

sheremet-va commented Oct 30, 2024 •

edited

Loading

hi-ogawa commented Oct 31, 2024

AriPerkkio commented Oct 31, 2024 •

edited

Loading

sheremet-va commented Oct 31, 2024 •

edited

Loading

sheremet-va commented Oct 31, 2024 •

edited

Loading

weilbith commented Nov 18, 2024

AriPerkkio commented Nov 18, 2024

	return {
	ast,
	code: s.toString(),
	map: s.generateMap({ hires: 'boundary', source: id }),
	}

fix(browser): improve source maps when vi.mock is present #6810

fix(browser): improve source maps when vi.mock is present #6810

Conversation

sheremet-va commented Oct 29, 2024 • edited Loading

Description

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

Tests

Documentation

Changesets

netlify bot commented Oct 29, 2024

✅ Deploy Preview for vitest-dev ready!

pkg-pr-new bot commented Oct 29, 2024 • edited Loading

sheremet-va commented Oct 29, 2024 • edited Loading

AriPerkkio left a comment

Choose a reason for hiding this comment

sheremet-va commented Oct 29, 2024

hi-ogawa commented Oct 30, 2024

sheremet-va commented Oct 30, 2024 • edited Loading

hi-ogawa commented Oct 31, 2024

AriPerkkio commented Oct 31, 2024 • edited Loading

sheremet-va commented Oct 31, 2024 • edited Loading

sheremet-va commented Oct 31, 2024 • edited Loading

weilbith commented Nov 18, 2024

AriPerkkio commented Nov 18, 2024

fix(browser): improve source maps when `vi.mock` is present #6810

fix(browser): improve source maps when `vi.mock` is present #6810

sheremet-va commented Oct 29, 2024 •

edited

Loading

pkg-pr-new bot commented Oct 29, 2024 •

edited

Loading

sheremet-va commented Oct 29, 2024 •

edited

Loading

sheremet-va commented Oct 30, 2024 •

edited

Loading

AriPerkkio commented Oct 31, 2024 •

edited

Loading

sheremet-va commented Oct 31, 2024 •

edited

Loading

sheremet-va commented Oct 31, 2024 •

edited

Loading