Skip to content
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: C8 sourcemaps #353

Merged
merged 1 commit into from Dec 29, 2021
Merged

fix: C8 sourcemaps #353

merged 1 commit into from Dec 29, 2021

Conversation

Monkatraz
Copy link
Contributor

Honestly, I'm not exactly sure what black magic is happening with C8 and the like, but as far as I understand this is a needed change. I don't think C8 was actually doing any source mapping prior to this, because it cached the coverage map when report.getCoverageMapFromAllCoverageFiles() was called.

Here is what Vitest is generating for source maps:
sourcemapthingy.zip

use this site to view it: https://sokra.github.io/source-map-visualization

You'll notice that the source mapping seems pretty much perfect, at least to me. I think any further issues with coverage are related to C8 itself.

@netlify
Copy link

netlify bot commented Dec 28, 2021

✔️ Deploy Preview for vitest-dev ready!

🔨 Explore the source changes: 309210d

🔍 Inspect the deploy log: https://app.netlify.com/sites/vitest-dev/deploys/61cb43373b537f00076cdca5

😎 Browse the preview: https://deploy-preview-353--vitest-dev.netlify.app

@Monkatraz
Copy link
Contributor Author

Monkatraz commented Dec 28, 2021

I think this fixes the same issue as #342

EDIT: I actually think both this and #342 are required because the source mapping isn't otherwise available in the modules (which borks breakpoints)

@Monkatraz
Copy link
Contributor Author

Monkatraz commented Dec 29, 2021

Further investigation:
This esbuild issue I think is relevant to some of the weird coverage issues.

If we take a look at this image:
image

You can see how the source is mapped pretty weirdly to the generated file.

That results in weird coverage that looks like this:
image

Really, the issue is two fold:

  1. The generated source mapping is not very dense
  2. Some of the mappings are bizarre and probably confuse the reporter

I see a couple "solutions" for this. One is to use tsc rather than esbuild when running with coverage, another is to use Istanbul directly. I don't like the latter, nor do I like the former, but out of the two, I think using tsc is probably better? If people are running with esbuild plugins I could see that causing issues though.

Another reason to use tsc is that esbuild actually rips any comment that isn't a legal comment, which means you can't actually use the /* c8 blah */ comments unless I'm missing something.

@Monkatraz
Copy link
Contributor Author

Okay, yeah, tsc emits a waaay more precise source map:
image

However, it breaks if I run it through the ssr mode. I presume that's because of the hacky tsc plugin I'm using.

@antfu antfu changed the title Fix C8 not using sourcemaps correctly (I think) fix: C8 sourcemaps Dec 29, 2021
@antfu antfu merged commit 436d280 into vitest-dev:main Dec 29, 2021
JakeGinnivan pushed a commit to JakeGinnivan/vitest that referenced this pull request Jan 8, 2022
chaii3 pushed a commit to chaii3/vitest that referenced this pull request May 13, 2022
…-dev#353)

* feat(useFetch): new feature createFetch

* Update packages/core/useFetch/index.ts

Co-authored-by: Anthony Fu <anthonyfu117@hotmail.com>

* removed unnecessary overloads

* Update packages/core/useFetch/index.ts

Co-authored-by: Anthony Fu <anthonyfu117@hotmail.com>

* baseUrl is now optional

* added function to check type of args

* removed console log from useFetch test

* Update packages/core/useFetch/index.ts

Co-authored-by: Anthony Fu <anthonyfu117@hotmail.com>

* merged imports in useFetch

* chore: update

Co-authored-by: Anthony Fu <anthonyfu117@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants