Skip to content

please update source-map to 0.7 #34

Closed

Description

@pravi

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)

Activity

  1. alexander-akait commented on Jan 30, 2018

    @alexander-akait
    Member

    @pravi right now impossibly, look https://github.com/mozilla/source-map/blob/master/CHANGELOG.md#070

    Breaking change: Drop support for Node < 8. If you want to support older versions of node, please use v0.6 or below.

  2. andreicristianpetcu commented on Mar 4, 2018

    @andreicristianpetcu

    Relevant downstream issue firefox-devtools/debugger#5598

  3. lencioni commented on Apr 25, 2018

    @lencioni

    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?

  4. alexander-akait commented on Apr 25, 2018

    @alexander-akait
    Member

    @lencioni we work on this mozilla/source-map#326 (comment), can't do here, it should be solved in source-map package 😞

  5. lencioni commented on Jul 9, 2018

    @lencioni

    @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?

    1. Change webpack-sources to be asynchronous
    2. In webpack-sources, detect which version of source-map it has available and conditionally use the async one when that's the one that is installed, or fall back to the older synchronous one.
    3. Modify the source-map dependency 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?

  6. mlavina commented on Aug 7, 2018

    @mlavina

    Modify the source-map dependency range to allow 0.7+

    I think this is the issue here because something like yarn will fail to install if there is an engine mismatch. If someone is on Node 6 trying to install this package which is using sourcemap 0.7 it will fail.

    An option I have been thinking is either

    1. Have it as an optionalDependency and do some installing on runtime if the install failed
    2. Similar just do some kind of postinstall conditional install of source-map something like
    "postinstall":  sh -c \"if [ node -p -e 'process.version > 8'  ] then; yarn install source-map@^0.7.1 else yarn install source-map@^0.6.1 fi\"
    1. Make source-map a Peer Dependency, but this would be a simple change, but technically a breaking change. Then at runtime check the version with something like
    require('source-map/package.json').version

    and 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?

  7. alexander-akait commented on Aug 7, 2018

    @alexander-akait
    Member

    @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.js script and put logic here

  8. hedgepigdaniel commented on Sep 6, 2018

    @hedgepigdaniel

    What about having an intermediate package - e.g. webpack-sources-source-map

    It can have a peer dependency on source-map@^0.6.0 and an optional dependency on source-map@^0.7.0 (is that allowed...?). At runtime when it imports source-map it would end up with 0.7.0 only if it installed successfully. It could wrap both of them, and convert the synchronous API of 0.6.x into the asynchronous API of 0.7.x. That way webpack-sources can upgrade to the new API by using webpack-sources-source-map instead of source-map and still work on old node versions.

  9. mlavina commented on Sep 22, 2018

    @mlavina

    It can have a peer dependency on source-map@^0.6.0 and an optional dependency on source-map@^0.7.0 (is that allowed...?)

    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.

  10. marcins commented on Jul 15, 2019

    @marcins

    What's the status of updating source-map in webpack-sources now 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-map appears 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-package and 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-map 0.6.1 via the 2.0 beta of webpack-sources (at least from my reading of the dependencies).

  11. alexander-akait commented on Jul 16, 2019

    @alexander-akait
    Member

    In todo for webpack@5

  12. sokra commented on Oct 22, 2019

    @sokra
    Member

    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-map without that stuff.

  13. alexander-akait commented on Oct 22, 2019

    @alexander-akait
    Member

    I think will be great to look on benchmarks, maybe we can open an issue about sync ability for source-map, terser maintainers need sync version too


    Issue: mozilla/source-map#331

  14. marcins commented on Oct 22, 2019

    @marcins

    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-map 0.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.

  15. noppa commented on Jul 18, 2021

    @noppa

    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 of WebAssembly.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

      "resolutions": {
        "**/source-map": "npm:[email protected]"
      }

    and it cut about 15-20% of our total production build time.

  16. 7rulnik commented on Jul 18, 2021

    @7rulnik

    Yep, but I prefer source-map-js

  17. sokra commented on Jul 27, 2021

    @sokra
    Member

    Major version 3 of webpack-sources is now super fast, and dependency-free. (not yet fully bug-free, but I'm working on that).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Fields

    Priority

    None yet

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions