Make source maps plain objects for use with t.valueToNode

[thebanjomatic]

When the sourcemaps were switched to use "@ampproject/remapping", the type
of the inputSourceMap that gets passed around changed from being a plain js
object to a class SourceMap from the remapping package. This causes issues
with the @babel/types valueToNode function because that function is defined
to throw for objects that aren't plain-objects.

In practice I was seeing this error after upgrading babel when running code-
coverage, and I saw a similar error reported here: vuejs/vue-jest#450 before
deciding to try to track the error down myself.

This appears to be broken since 7.17.0 (pinning @babel/core to ~7.16.0 prevents the issue from occurring).

This is my first time contributing to babel and I had issues getting the project to build locally with an error:

[22:14:45] Compiling 'packages/babel-standalone/src/index.ts' with rollup ...
{
  code: 'MISSING_EXPORT',
  exporter: 'packages\\babel-core\\src\\index.ts',
  importer: 'packages\\babel-plugin-transform-runtime\\node_modules\\@babel\\helper-define-polyfill-provider\\esm\\index.browser.mjs',
  message: "'default' is not exported by 'packages\\babel-core\\src\\index.ts'",
  missing: 'default',
...

Q	A
Fixed Issues?
Patch: Bug Fix?	Yes
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass?	Yes
Documentation PR Link
Any Dependency Changes?
License	MIT

[babel-bot]

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/51317/

[jridgewell]

This is a strange usecase, but LGTM.

[nicolo-ribaudo]

This feels hacky/strange, but I'm ok with it since it's to restore backward compatibility. However, we might consider reverting this in Babel 8 since it's an unnecessary clone for 99.9% of the possible use cases, and you will then need to do t.valueToNode({ ...sourceMap }).

May I ask why you pass source maps to valueToNode?

[nicolo-ribaudo]

This is my first time contributing to babel and I had issues getting the project to build locally with an error:

I think no one in the team uses Windows, so our configuration might need to be fixed to work with \ instead of / in paths 😅

[thebanjomatic]

May I ask why you pass source maps to valueToNode?

@nicolo-ribaudo I agree this maybe isn't ideal.

I just tried to track that down. I don't own/maintain any of the projects that were involved in this error, this just came up when I was running jest for code coverage.

It looks like the sourcemap is being grabbed from file.inputMap.sourcemap in babel-plugin-istanbul here:
https://github.com/istanbuljs/babel-plugin-istanbul/blob/99bd2a27493b7552ae75877496c3b2ad93f5fc46/src/index.js#L114-L116

And istanbul-lib-instrument is then calling valueFromNode on the coverage data which includes the inputSourceMap here I think under the assumption that this.cov.toJSON() actually converts to json:
https://github.com/istanbuljs/istanbuljs/blob/a743b8442e977f0c77ffa282eed7ac84ca200d1f/packages/istanbul-lib-instrument/src/visitor.js#L688-L700

It looks like there is already an issue now in istanbul-lib-instrument. It would probably be better to move the source-map conversion into babel-plugin-istanbul when it grabs the inputMap in the first place.

[thebanjomatic]

@nicolo-ribaudo Assuming this babel-plugin-istanbuil PR gets merged and released, we can probably revert this PR.

Sorry for not finding that sooner as I think its definitely the more appropriate fix, it was much easier when dealing with unfamiliar code to find the immediate cause of the error, and where the sourcemap was created, but less obvious tracking down everything that happened to the sourcemap along the way.

[jridgewell]

No problem, I think it's still appropriate to have the fix here because we caused the regression. We can revert in Babel 8, but I don't think it's terrible.