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
Update source-map -> 0.8.0-beta.0 for compatibility with global fetch
#1164
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is due to SourceMapConsumer
using WASM, which uses fetch
or fs.readFile
to get the WASM bytes to instantiate. We could instead switch to @jridgewell/trace-mapping
which provides a near drop-in API (and is faster, less memory intensive, and doesn't depend on WASM).
🤔 this sounds tempting! The memory usage of source-map is definitely a problem. Though Terser also needs to handle the production of source maps. This library could help https://www.npmjs.com/package/vlq#-buffer |
It is only the (Specifically: |
I tried creating a PR for |
fetch
@jridgewell - did you get anywhere with adding Would love to move towards |
Implemented as |
Sounds good to me! |
Thanks for coming up with this @robhogan ! |
This also closes #1030, who also deserves some credit for suggesting the fix ages ago :) |
Summary: This update includes the removal of a direct dependency on `source-map@0.7.x`, in particular the problematic (especially for React Native) `fetch`-based browser/node environment detection described in terser/terser#1164. This was the last item blocking us from deprecating and replacing `metro-minify-uglify`, which uses the long-depreceted [`uglify-es`](https://www.npmjs.com/package/uglify-es). ### Follow-up I'll follow up with adding `metro-minify-terser` to `metro`'s dependencies and communicating the deprecation of `metro-minify-uglify` in OSS. Then we'll be able to switch the default and remove `metro-minify-uglify` from the deps in a release cycle or two. Reviewed By: motiz88 Differential Revision: D36098090 fbshipit-source-id: 415431f17e8048bcd1eccd80c3faa6bbc2283cfb
source-map@0.7.3
has an issue with buggy identification of node vs browser environment (it usestypeof fetch === 'function'
to spot browsers) - See mozilla/source-map#349, mozilla/source-map#432 and fixes mozilla/source-map#350, mozilla/source-map#363.This is an issue for React (Native) environments where
fetch
is often polyfilled, eg for testing or SSR. It's one of the last issues holding us back from makingterser
the default minifier for https://github.com/facebook/metro.Importantly, this is also probably going to be an issue for all NodeJS users soon, as NodeJS moves forward with a native global
fetch
: nodejs/node#41749Unfortunately, the only
source-map
release since that bug was fixed is 0.8.0-beta.0, dated Nov 2018.I've suggested they publish a release but I'm not optimistic (Release 0.8, or 0.7.4? mozilla/source-map#452).
Would you consider a dependency on a "prerelease", or is this a non-starter?