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 a breakage when using the plugin with --source-map on Node 14+. #41

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

Conversation

chipx86
Copy link

@chipx86 chipx86 commented Jan 9, 2021

A Node.js 14.x added strict type checking when writing files to disk, preventing methods with their own .toString() method from being written to disk and generating a ERR_INVALID_ARG_TYPE error in the process. This affected using this plugin in combination with --source-map.

The behavioral change was introduced in nodejs/node#31030 and recently fixed in nodejs/node#34993. That fix was not comprehensive, and did not resolve the issue for the plugin.

To avoid this issue for all versions of Node, we no longer assume there will be an implicit call to SourceMapGenerator.toString(). Instead, it's now explicitly called when setting the data to write for the source map, fixing source map generation.

This was tested on the latest releases of Node 12 through 15.

A Node.js 14.x added strict type checking when writing files to disk,
preventing methods with their own `.toString()` method from being
written to disk and generating a `ERR_INVALID_ARG_TYPE` error in the
process. This affected using this plugin in combination with
`--source-map`.

The behavioral change was introduced in
nodejs/node#31030 and recently fixed in
nodejs/node#34993. That fix was not
comprehensive, and did not resolve the issue for the plugin.

To avoid this issue for all versions of Node, we no longer assume there
will be an implicit call to `SourceMapGenerator.toString()`. Instead,
it's now explicitly called when setting the data to write for the source
map, fixing source map generation.

This was tested on the latest releases of Node 12 through 15.
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.

2 participants