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

chore(sourcemap): convert inputSourceMap to plain js object #272

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

thebanjomatic
Copy link

@thebanjomatic thebanjomatic commented Feb 17, 2022

@babel/core recently made a change where the sourcemap is no longer a plain js
object, but instead is a class SourceMap. This breaks when the coverage data
gets passed into valueToNode by istanbul-lib-instrument because the coverage
data is no longer entirely plain json objects (this.cov.toJSON() just returns
the data and doesn't do any conversion).

In this PR we are checking if the input sourcemap is a plain js object, and
converting it if it is not.

Note that this does seem like the more appropriate place to make the fix since it
is istanbul that has the requirement that the coverage data is a plain javascript object
and the patch applied here babel/babel#14283 is more of a
temporary workaround.

istanbuljs/istanbuljs#672

@babel/core recently made a change where the sourcemap is no longer a plain js
object, but instead is a class SourceMap. This breaks when the coverage data
gets passed into valueToNode by istanbul-lib-instrument because the coverage
data is no longer entirely plain json objects (this.cov.toJSON() just returns
the data and doesn't do any conversion).

In this PR we are checking if the input sourcemap is a plain js object, and
converting it if it is not.
@bcoe
Copy link
Member

bcoe commented Feb 21, 2022

@thebanjomatic I think this problem is better fixed here, if I'm following:

istanbuljs/istanbuljs#662

The only thing blocking #662 from landing is that someone needs to write a test. I would happy accept a PR for a test, if you'd like to contribute (you can branch from #662).

@SimenB
Copy link
Member

SimenB commented Jul 5, 2024

Should we close this since it was fixed upstream?

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.

3 participants