-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
fix(browser): improve source maps when vi.mock
is present
#6810
fix(browser): improve source maps when vi.mock
is present
#6810
Conversation
✅ Deploy Preview for vitest-dev ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@vitest/browser
@vitest/coverage-istanbul
@vitest/expect
@vitest/coverage-v8
@vitest/mocker
@vitest/pretty-format
@vitest/runner
@vitest/snapshot
@vitest/spy
@vitest/ui
@vitest/utils
vite-node
vitest
@vitest/web-worker
@vitest/ws-client
commit: |
Hm, this makes source maps for It is possibly related to the fact that we don't touch the import from
With |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 }), | |
} |
Doesn't seem to change anything |
Are you also testing with the latest Vite? If |
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 |
Ah, I missed that 🤦 then their change doesn't look relevant. |
To me it looks really good - what's wrong here? https://ariperkkio.github.io/vite-plugin-source-map-visualizer/#MTYyMQB2aS5tb2NrKCIuL2NvbnN0YW50cy... With Also these two showcase nicely the changes: |
The first source map looks different to me. The My source maps look like this 🤔 |
(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 |
Is there an easy way to notice when this was included in a release? |
@weilbith release changelogs are at https://github.com/vitest-dev/vitest/releases. This commit is mentioned on latest beta release. |
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:
pnpm-lock.yaml
unless you introduce a new test example.Tests
pnpm test:ci
.Documentation
pnpm run docs
command.Changesets
feat:
,fix:
,perf:
,docs:
, orchore:
.