Global fetch breaks source-map

[SimenB]

Version

v17.8.0

Platform

Darwin Simens-MacBook-Pro.local 21.4.0 Darwin Kernel Version 21.4.0: Fri Mar 18 00:45:05 PDT 2022; root:xnu-8020.101.4~15/RELEASE_X86_64 x86_64

Subsystem

No response

What steps will reproduce the bug?

npm init -y && npm install --save source-map (this gives 0.7.3 at the time of writing). source-map has more than 160M downloads a week, although the broken v7 has a mere 28M since it's async only. See https://www.runpkg.com/[email protected]/lib/read-wasm.js#1 for where it breaks.

// file.mjs
import {SourceMapConsumer} from 'source-map'

await new SourceMapConsumer({version: '3', sources: [], mappings: []})

$ node file.mjs
$ echo $?
0
$ node --experimental-fetch file.mjs
(node:21808) ExperimentalWarning: Fetch is an experimental feature. This feature could change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
/Users/simen/repos/source-map-boom/node_modules/source-map/lib/read-wasm.js:8
      throw new Error("You must provide the URL of lib/mappings.wasm by calling " +
            ^

Error: You must provide the URL of lib/mappings.wasm by calling SourceMapConsumer.initialize({ 'lib/mappings.wasm': ... }) before using SourceMapConsumer
    at readWasm (/Users/simen/repos/source-map-boom/node_modules/source-map/lib/read-wasm.js:8:13)
    at wasm (/Users/simen/repos/source-map-boom/node_modules/source-map/lib/wasm.js:25:16)
    at /Users/simen/repos/source-map-boom/node_modules/source-map/lib/source-map-consumer.js:264:14
    at async file:///Users/simen/repos/source-map-boom/file.mjs:3:1

Node.js v17.8.0
$ echo $?
1

How often does it reproduce? Is there a required condition?

Always.

What is the expected behavior?

It should not break since the code is not running in a browser environment.

What do you see instead?

If fetch is available as a global, source-map thinks it's in a browser setting, and breaks.

Additional information

I know this isn't Node's fault, but since source-map seems unmaintained, I hope maybe Node as a project can reach out the the old maintainers (or Mozilla?) so this is fixed.

Once v18 is stable (where fetch is a global by default), this'll break way more people. v8-to-istanbul, used by both C8 and Jest (to provide code coverage reports when using native V8 coverage) is affected by this, and probably lots of other projects.

[aduh95]

Is there an issue open for this on their repo?

[SimenB]

mozilla/source-map#349. It's apparently fixed in the beta releases

[SimenB]

Based on terser/terser#1164 (review) v8-to-istanbul might benefit from migrating, but there's still 28M weekly downloads of the broken version (jest has maybe half of those? unsure how npm counts)

[aduh95]

FWIW there's the --no-experimental-fetch CLI flag that can be used as a workaround.

Pinging @nodejs/npm in case they know how this could be mitigated.

[SimenB]

FWIW there's the --no-experimental-fetch CLI flag that can be used as a workaround.

Ah, nice! I can add that flag to Jest's tests at least

[targos]

Is there still something for Node.js to do?

[SimenB]

As a project it would be nice if node could try to get in touch with the maintainers so they make a new stable release before v18 is released. But code wise I doubt there's much to be done (beyond not unflagging fetch which I assume (and hope) is a non-starter).

[ljharb]

Node could also use a different name than fetch, or put it somewhere that’s not a global.

[SimenB]

C8 migrated away in a patch, and Jest in an alpha release. Still, that's only about half of the downloads of the broken version

[devsnek]

are these packages in our npm canary tests? obviously breaking changes are expected in major versions but it makes a good way to notify at least.

[SimenB]

Jest is (which is how I was made aware): nodejs/citgm#894. I don't think c8 or source-map is.

[ljharb]

Heads up that this continues to be broken for source-map in node 18, which is also breaking tsup, which is used by https://github.com/coinbase/cbpay-js/runs/6116903030?check_suite_focus=true. Given that mozilla/source-map#349 was last updated in 2019, there's not a strong implication that a fix will be published anytime soon.

[RaisinTen]

or put it somewhere that’s not a global.

I think that's being considered in #42814 (comment) - exposing fetch from a node:undici module.

[cspotcode]

Since source-map is pre-1.0, per semver rules a patch release wouldn't be picked up automatically by downstream projects, right?

[ljharb]

@cspotcode incorrect; in npm, with vX.Y.Z, v0.X.Y, and v0.0.X, X is the major, Y the minor, and Z the patch, and ^ behaves accordingly. So, anyone depending on ^0.7.3 will get anything >= 0.7.3 and < 0.8.

[mcollina]

@SimenB is this still a problem?

[ljharb]

@mcollina yes; see #42638 (comment) - jest may have worked around it but all the other source-map dependents likely haven't.

[mcollina]

Is there any fix we can do on the Node.js side?

[ljharb]

@mcollina the only fix i can think of is no longer having a global named fetch, since that's what this widely-deployed package interprets to mean "it's a browser" - either a global under another name, or a non-global, would ofc fix it.

[dnalborczyk]

the only fix i can think of is no longer having a global named fetch

please leave it a global named fetch.

node.js is not a browser and the source files are accessible and can be forked. I would not let an unmaintained package dictate the future of fetch on node.js.

one can also fix it by trying to contact someone at mozilla or just use a fork.

[ljharb]

@dnalborczyk it's not a browser which is why it shouldn't have a fetch global :-)

"use a fork" doesn't work because it's a widely used transitive dependency. Certainly fixing source-map itself is an option, but that's not under node's control.

[dnalborczyk]

it's not a browser which is why it shouldn't have a fetch global :-)

I'm sorry, not quite sure if I'm following your argument 😄 if you are not joking, your argument could be applied to any of the supported web api in node.js. why shouldn't it be supported just because it's not a browser?

"use a fork" doesn't work because it's a widely used transitive dependency. Certainly fixing source-map itself is an option, but that's not under node's control.

well, node.js 18 was released very recently. most production code use LTS versions. so there's still some time to correct the issue in any transitive packages, be it by a mozilla fix or a fork, until v18 reaches LTS.

changing the global name now would be a BREAKING change in itself.

the only fix i can think of is no longer having a global named fetch

I was thinking about this a bit. there's another possible solution by providing the fetch API to the user with an explicit fetch export, (e.g. import fetch, { Headers, Request } from 'node:fetch', in addition to the global.

if an app wants to use fetch as well as the source-map module (or any transitive dependencies using it) a developer could turn off the global module with the provided flag and import fetch explicitly if needed.

[ljharb]

@dnalborczyk the api is experimental so it wouldn’t require a semver-major to remove, but i understand that folks would be unlikely to agree to that.

My argument is that a web API should only exist in node - at all, or as a global - if it can 100% match the semantics. fetch can’t. Obviously node folks rejected that argument, or it wouldn’t have landed in node 18, but my opinion remains.

[SimenB]

Certainly fixing source-map itself is an option, but that's not under node's control.

Note that the alpha of source-map is fixed, so if somebody ™️ could get a hold of one of the maintainers, just promoting that to stable might help (or even backporting the fix to v0.7). At least that should let the fix trickle down over time. Main issue now (imo) is that source-map@latest is broken.

[mcollina]

@eemeli do you think you can ship an updated version of source-map?

[eemeli]

I'll see what I can do.

[eemeli]

[email protected] is now published as latest on npm, with no breaking changes to the previous version.

[mcollina]

Thanks @eemeli! I think we can close this now.