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 crash with node.js that has or polyfills fetch() #6

Merged
merged 1 commit into from
Mar 7, 2022

Conversation

trentm
Copy link
Contributor

@trentm trentm commented Feb 28, 2022

"source-map" v0.7.3 uses if (typeof fetch === "function") to
differentiate between browser and node.js environments. If a node.js
environment has fetch() -- will be the default in node v18 -- or
polyfills it, then it throws:
You must provide the URL of lib/mappings.wasm by calling SourceMapConsumer.initialize({ 'lib/mappings.wasm': ... }) before using SourceMapConsumer


Hi. Thanks for this library!

The "source-map" issue is mozilla/source-map#349 It was fixed in Aug 2018, but there hasn't been a release of "source-map" which includes the fix, except for a "0.8.0-beta.0" beta release. It doesn't appear a non-beta source-map release will happen soon. If it helps give some stability comfort, GoogleChrome/workbox#2712 hit the same issue last year and update to source-map@0.8.0-beta.0.

There are some "breaking changes" listed in the source-map changelog so I'm not sure if you'd want to bump the major ver of this package before releasing again.

This has come up now because the node.js v18.0.0 nightly builds enabled a Node.js implementation of fetch() by default as of last week (nodejs/node#41811). I work on a project that tests with recent Node.js nightly builds. Our tracking issue is: elastic/apm-agent-nodejs#2589

"source-map" v0.7.3 uses `if (typeof fetch === "function")` to
differentiate between browser and node.js environments. If a node.js
environment has `fetch()` -- will be the default in node v18 -- or
polyfills it, then it throws:
    You must provide the URL of lib/mappings.wasm by calling SourceMapConsumer.initialize({ 'lib/mappings.wasm': ... }) before using SourceMapConsumer
@trentm
Copy link
Contributor Author

trentm commented Feb 28, 2022

This shows the test failure with a recent node v18.0.0 nightly build:

% node --version
v18.0.0-nightly20220222c071bd581a

% npm test

> load-source-map@2.0.0 test
> tape test/test.js && standard

TAP version 13
# should give back undefined as result if no sourcemap is referenced
ok 1 should not error
ok 2 sourcemap should be undefined
# should handle inline sourcemaps
not ok 3 should not error
  ---
    operator: error
    at: <anonymous> (/Users/trentm/tm/load-source-map/test/test.js:19:7)
    stack: |-
      Error: Error parsing sourcemap for file "/Users/trentm/tm/load-source-map/test/fixtures/lib-inline/example.js":
      You must provide the URL of lib/mappings.wasm by calling SourceMapConsumer.initialize({ 'lib/mappings.wasm': ... }) before using SourceMapConsumer
          at readWasm (/Users/trentm/tm/load-source-map/node_modules/source-map/lib/read-wasm.js:8:13)
          at wasm (/Users/trentm/tm/load-source-map/node_modules/source-map/lib/wasm.js:25:16)
          at /Users/trentm/tm/load-source-map/node_modules/source-map/lib/source-map-consumer.js:264:14
  ...
/Users/trentm/tm/load-source-map/test/test.js:23
    t.deepEqual(sourcemap.originalPositionFor(generated), expected, 'should have correct source mapping')
                          ^

TypeError: Cannot read properties of undefined (reading 'originalPositionFor')
    at /Users/trentm/tm/load-source-map/test/test.js:23:27
    at onConsumerError (/Users/trentm/tm/load-source-map/lib/index.js:49:14)

Node.js v18.0.0-nightly20220222c071bd581a

@rexxars
Copy link
Owner

rexxars commented Mar 7, 2022

Thanks! I've published v3.0.1 which includes your fix. Note that it now requires node 12 or higher.

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