WIP - Update source-map to 0.7.2 (#5598) #995
Conversation
Some breaking changes that needed to be addressed: - new SourceMapConsumer now returns a Promise that needs to be awaited on - WebAssembly for encoding/decoding mappings needs to be loaded source-map loads wasm differently in web and Node contexts. Jest tests run in a Node environment so both need to be accounted for here.
| @@ -0,0 +1,5 @@ | |||
| // TODO: Remove this file after upgrading to Jest 22.4.1+ | |||
| // More info: https://github.com/facebook/jest/issues/687 | |||
| console.debug = console.log | |||
There was a problem hiding this comment.
In server environments, source-map calls console.debug, which does not exist in the version of Jest that this repo is currently running. Some other problems prevented me from simply upgrading Jest. I didn't think it was the right time to be going down that rabbithole.
| @@ -11,6 +11,9 @@ | |||
|
|
|||
| const { networkRequest } = require("devtools-utils"); | |||
| const { SourceMapConsumer, SourceMapGenerator } = require("source-map"); | |||
| SourceMapConsumer.initialize({ | |||
| "lib/mappings.wasm": "https://unpkg.com/source-map@0.7.2/lib/mappings.wasm" | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
I don't know the answer here either, but this is going to need to be configurable so that this can also work in Mozilla Central. There, we'll want to ship the wasm in Firefox and we'll need to use a resource URL or something.
There was a problem hiding this comment.
yep, we'll need to provide something when we start the worker...
|
CC @tromey could you take a look at this? |
|
I looked at the test failure a bit. I am not sure this is a valid test. The issue is that the source map contains two mappings for the I tend to think that the source map implementation has some freedom to decide which result it returns in this case. (Or perhaps it should return both.) I'm not 100% sure though, so CC @fitzgen on this as well. |
|
I forgot to mention - I read this patch and, aside from the URL configurability thing, it is pretty much what I thought this patch should look like. So assuming the webpack and test case issues are worked out, r+ from me. Thank you for doing this! |
|
Thanks @tromey Lets sort out the symmetry thing. I'm good with anything as long as I understand what is going on. I'm happy to fix this up so we can get the mappings.wasm in m-c and pulled in in the launchpad. |
If this is truly the case and debugger.html doesn't make any assumptions to the contrary, it should be OK to work around the failing tests like so: 1ed76bb |
|
I've posted an updated version of this in firefox-devtools/debugger#7071, so going to close this. |
This PR addresses firefox-devtools/debugger#5598 for updating
source-mapto0.7.2. The new version uses WebAssembly to encode/decode mappings. 馃挜This PR is marked as WIP for a couple reasons:
One of the source mapping Jest tests gives a surprising failure:
WebAssembly is loaded differently in Node as opposed to the web. (Jest tests run in a Node-like environment.) The way I handled that is almost certainly not how we want to do it. See my comments below for more details.