nodejs/node

Tracking: Revert #5102 when possible

jasnell opened this issue · 9 comments

PR #5102 was landed as a temporary measure to get npm in master working again. This issue is intended to serve as a reminder that it needs to be reverted as planned.

/cc @ChALkeR @thealphanerd @nodejs/ctc

Can we revert after the next npm release or we are going to wait till the major release?

The plan is to release in v6 and remove again in v7 to give people time to upgrade their dependencies.

@jasnell npm is not the sole user of graceful-fs (and there might be other libs that do the same), my proposal was to keep this in 6.x branch and revert in master as soon as 6.x is branched.

@ChALkeR do you know how many modules rely on the pre fix version?

@thealphanerd Uh-oh. Not yet. For now, I can check only direct dependants.
Data from 2016-01-28, all direct dependants of graceful-fs: https://gist.github.com/ChALkeR/e51abea04b6facfb9bd0.

Note that v4.x are fixed, v3.x and below are affected, so I excluded everything that works with 4.x.

Update: less is fixed in current master, but no release has been published yet, less@2.6.0 still depends on graceful-fs v3.

@thealphanerd I will check indirect deps too, but I can't say when yet.

More modules that directly use fs source code in current versions (downloads/month on the left):

1272    guanlecoja-0.6.1.tgz/vendors.js:84525:var src = pre + process.binding('natives').fs + post
1021    bitballoon-0.2.2.tgz/browser/bitballoon.js:8462:var src = pre + process.binding('natives').fs + post
425 eris-db-0.14.1.tgz/test/browser/test_js/test_rpc_ws.js:3231:var src = pre + process.binding('natives').fs + post
223 openapi-node-3.0.3.tgz/pakmanaged.js:4167:    var src = pre + process.binding('natives').fs + post
178 gulp-display-help-1.3.0.tgz/pakmanaged.js:26140:    var src = pre + process.binding('natives').fs + post
36  uber-micro-0.0.0.tgz/pakmanaged.js:1901:    var src = pre + process.binding('natives').fs + post
32  yade-1.3.3.tgz/runtime.js:1601:var src = pre + process.binding('natives').fs + post
21  moduloteste-1.0.0.tgz/graceful-fs/fs.js:8:var src = pre + process.binding('natives').fs + post
20  guanlecoja_test-0.6.0.tgz/vendors.js:61174:var src = pre + process.binding('natives').fs + post
18  crowdjs-0.2.0.tgz/pakmanaged.js:1751:    var src = pre + process.binding('natives').fs + post

Update: All of the above looks like they are being polluted by graceful-fs source code, perhaps in the build process through the deps or so, and one directly copied graceful-fs inside the package.

@thealphanerd Indirect deps seem to be huge.
For example, current version of karma is 0.13.x (0.13.0 released last summer), and 0.12.x use broken graceful-fs. I will prepare a full checker in the next few days, I hope.

If npm tracked version downloads, you could measure impact instead of guessing.