Repository navigation
Closed
Labels
Description
With source-map 0.7, two tests are failing.
mocha -R spec
ConcatSource
✓ should concat two sources
✓ should be able to handle strings for all methods
PrefixSource
✓ should prefix a source
✓ should have consistent source/sourceAndMap behavior
ReplaceSource
1) should replace correctly
2) should replace multiple items correctly
package-entry
✓ should not throw SyntaxError
5 passing (68ms)
2 failing
1) ReplaceSource
should replace correctly:
TypeError: (intermediate value).originalPositionFor is not a function
at ReplaceSource._replacementToSourceNode (lib/ReplaceSource.js:156:54)
at ReplaceSource.<anonymous> (lib/ReplaceSource.js:83:37)
at Array.forEach (<anonymous>)
at ReplaceSource.node (lib/ReplaceSource.js:76:21)
at ReplaceSource.proto.sourceAndMap (lib/SourceAndMapMixin.js:30:18)
at Context.<anonymous> (test/ReplaceSource.js:31:26)
at callFn (/usr/lib/nodejs/mocha/lib/runnable.js:354:21)
at Test.Runnable.run (/usr/lib/nodejs/mocha/lib/runnable.js:346:7)
at Runner.runTest (/usr/lib/nodejs/mocha/lib/runner.js:442:10)
at /usr/lib/nodejs/mocha/lib/runner.js:560:12
at next (/usr/lib/nodejs/mocha/lib/runner.js:356:14)
at /usr/lib/nodejs/mocha/lib/runner.js:366:7
at next (/usr/lib/nodejs/mocha/lib/runner.js:290:14)
at Immediate.<anonymous> (/usr/lib/nodejs/mocha/lib/runner.js:334:5)
2) ReplaceSource
should replace multiple items correctly:
TypeError: (intermediate value).originalPositionFor is not a function
at ReplaceSource._replacementToSourceNode (lib/ReplaceSource.js:156:54)
at ReplaceSource.<anonymous> (lib/ReplaceSource.js:83:37)
at Array.forEach (<anonymous>)
at ReplaceSource.node (lib/ReplaceSource.js:76:21)
at ReplaceSource.proto.sourceAndMap (lib/SourceAndMapMixin.js:30:18)
at Context.<anonymous> (test/ReplaceSource.js:61:26)
at callFn (/usr/lib/nodejs/mocha/lib/runnable.js:354:21)
at Test.Runnable.run (/usr/lib/nodejs/mocha/lib/runnable.js:346:7)
at Runner.runTest (/usr/lib/nodejs/mocha/lib/runner.js:442:10)
at /usr/lib/nodejs/mocha/lib/runner.js:560:12
at next (/usr/lib/nodejs/mocha/lib/runner.js:356:14)
at /usr/lib/nodejs/mocha/lib/runner.js:366:7
at next (/usr/lib/nodejs/mocha/lib/runner.js:290:14)
at Immediate.<anonymous> (/usr/lib/nodejs/mocha/lib/runner.js:334:5)
Reacted by Ryan Elian, Andrei Petcu, PutziSan, Ed Morley, niko, Björn Brauer, Andrey Marchenko, Joe Lencioni, Andrii Oriekhov and Oskari Noppa
Metadata
Metadata
Assignees
Labels
Type
Fields
Priority
None yet
Activity
alexander-akait commentedon Jan 30, 2018
@pravi right now impossibly, look https://github.com/mozilla/source-map/blob/master/CHANGELOG.md#070
Revert source-map package flattening: webpack/webpack-sources#34
andreicristianpetcu commentedon Mar 4, 2018
Relevant downstream issue firefox-devtools/debugger#5598
lencioni commentedon Apr 25, 2018
Do you think webpack-sources could be updated in a way that supports both the old and new versions of source-map? That would allow folks running Node 8+ to get the performance improvements without hurting folks on older Node. Thoughts?
alexander-akait commentedon Apr 25, 2018
@lencioni we work on this mozilla/source-map#326 (comment), can't do here, it should be solved in
source-mappackage 😞lencioni commentedon Jul 9, 2018
@evilebottnawi I'm not sure I understand why this can't be resolved here, can you explain in a little more detail?
As far as the API is concerned, it seems that the main change is that source-map is now async instead of synchronous. Could we make this work by doing the following?
source-mapdependency range to allow 0.7+This would give consumers the option to update to a newer version of source-map, if they are on a new enough version of Node, or continue using the much slower 0.6 if they are on an older version of Node.
Or, is there something else missing that I don't understand?
mlavina commentedon Aug 7, 2018
I think this is the issue here because something like
yarnwill fail to install if there is an engine mismatch. If someone is on Node 6 trying to install this package which is usingsourcemap 0.7it will fail.An option I have been thinking is either
optionalDependencyand do some installing on runtime if the install failedpostinstallconditional install ofsource-mapsomething likeand have conditional code that way
@evilebottnawi I think this is related to our discussion on the uglify-js plugin webpack-contrib/uglifyjs-webpack-plugin#248
because i think a solution here will get translated to a solution there.
What are your thoughts on those 3 ideas?
alexander-akait commentedon Aug 7, 2018
@mlavina i think 2 point is looks good, let's do it first in plugin, example uglify/terser and then implement here, feel free to PR. Also some people use npm, better create
install.jsscript and put logic herehedgepigdaniel commentedon Sep 6, 2018
What about having an intermediate package - e.g.
webpack-sources-source-mapIt can have a peer dependency on
source-map@^0.6.0and an optional dependency onsource-map@^0.7.0(is that allowed...?). At runtime when it importssource-mapit would end up with0.7.0only if it installed successfully. It could wrap both of them, and convert the synchronous API of0.6.xinto the asynchronous API of0.7.x. That waywebpack-sourcescan upgrade to the new API by usingwebpack-sources-source-mapinstead ofsource-mapand still work on old node versions.mlavina commentedon Sep 22, 2018
Yeah that's the part that we need to figure out
At this point Node 6 is almost out of maintenance mode in April 2019 and I think at that point we can probably justify the breaking change to drop Node 6 support. If that's the case I am somewhat more inclined to wait then submit my proposed changes.
marcins commentedon Jul 15, 2019
What's the status of updating
source-mapinwebpack-sourcesnow that Node 6 is out of LTS and no longer supported? I believe this was the main blocker in the past.FYI @TheLarkInn - the performance fix in mozilla/source-map#308 which never made it to the 0.6.x branch, but only to the new async/WASM 0.7.x branch of
source-mapappears to significantly improve performance for production (minified) builds with source maps .As mentioned in mozilla/source-map#370 (comment) I applied the perf fix via
patch-packageand found a 70% decrease in our full Webpack 3 production build with sourcemaps (from 35 -> 12 minutes). I applied this to our smaller Webpack 4 build as well and saw an improvement there too, albeit smaller, from ~9 min to ~6.5 min. (that's still ~25% faster).I will be trying upgrading our main build to Webpack 4 later this week, which we previously couldn't do due to a combo of perf & memory pressure which I now suspect was due to this issue.
Also, I notice that even the Webpack 5 branch is still using
source-map0.6.1 via the 2.0 beta ofwebpack-sources(at least from my reading of the dependencies).alexander-akait commentedon Jul 16, 2019
In todo for webpack@5
sokra commentedon Oct 22, 2019
Sorry, but the
[email protected]version is nearly impossible to use without completely changing our API to Promises.In my opinion using WASM is not really needed for good performance in source-map. Or at least it's not worth the trade-off of additional complexity (async, managing wasm buffer initialization).
I really thinking about creating my own version of
source-mapwithout that stuff.alexander-akait commentedon Oct 22, 2019
I think will be great to look on benchmarks, maybe we can open an issue about
syncability forsource-map,tersermaintainers need sync version tooIssue: mozilla/source-map#331
marcins commentedon Oct 22, 2019
To be honest I'm less concerned about the WASM etc stuff that comes with the newer versions of
source-map, but the fix of a fundamental performance bug that has serious implications for CPU and memory usage when generating source maps for large compilations. We couldn't upgrade to Webpack 4 without applying this patch.Maybe it would be enough as a starting point to fork
source-map0.6.x, and apply the perf fix, and use that forked version in the Webpack ecosystem? Ideally Mozilla would merge the back-port PR and release a new 0.6.x release, but if it hasn't happened by now (almost two years) it seems unlikely to happen.noppa commentedon Jul 18, 2021
I just noticed 7rulnik/source-map-sync/pull/1 (source-map-sync on npm), which modifies source-map 0.8 to expose a synchronous API, by initializing the wasm module with
new WebAssembly.Module(buffer)instead ofWebAssembly.instantiate.The downside of this approach is that it won't work in browsers, since they apparently have a 4kb size limit for the synchronous initialization API. I know there are some projects that use Webpack in the browser too, but perhaps it could be configured somehow that on Node, Webpack would use the source-map-sync library or something else like it.
I tried it on our project with yarn resolutions
and it cut about 15-20% of our total production build time.
7rulnik commentedon Jul 18, 2021
Yep, but I prefer source-map-js
sokra commentedon Jul 27, 2021
Major version 3 of webpack-sources is now super fast, and dependency-free. (not yet fully bug-free, but I'm working on that).