-
-
Notifications
You must be signed in to change notification settings - Fork 438
Update source-map -> 0.8.0-beta.0 for compatibility with global fetch
#1164
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
Conversation
jridgewell
left a comment
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.3has 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
fetchis often polyfilled, eg for testing or SSR. It's one of the last issues holding us back from makingterserthe 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-maprelease 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?