Gulp: Old graceful-fs version in gulp 3.x

[ChALkeR]

Opening this as a separate issue, as #1571 got comments closed (and describes a slightly different issue), and my comment in #1604 got moderated without a notice.

graceful-fs below version 4.0 (well, technically only 2.x and 3.x) is going to break on Node.js 7.x (which is going to happen in about 5 months from now).

_If you are sure that everything is covered and understood correctly — just close this, please :-) (and excuse me for opening a new issue), I am just making sure that nothing is missed here._

Your assessment of this issue in #1571 is not precise:

There is nothing wrong with the dependency, especially since it is only used in development. We will be updating or removing it in gulp 4 and the message will go away.

There are two warnings there — a warning on installation of graceful-fs lower than 3.x and a warning in runtime when an old graceful-fs version does unsupported stuff (which it does upon require).

I am only talking about the runtime warning here that comes from Node.js. That runtime warning is emitted only in those situations which would be turned into throws on Node.js 7.0 and above.

Now, just running gulp even with an empty gulpfile.js file emits this runtime warning:

(node:25661) fs: re-evaluating native module sources is not supported. If you are using the graceful-fs module, please update it to a more recent version.

which is going to turn into a throw in Node.js 7.0.

The dependency tree to graceful-fs looks like this:

└─┬ [email protected] 
  └─┬ [email protected] 
    ├─┬ [email protected] 
    │ └─┬ [email protected] 
    │   └─┬ [email protected] 
    │     └─┬ [email protected] 
    │       └── [email protected] 
    └── [email protected] 

[email protected] is not reevalating native module sources, so it doesn't trigger that warning and would not be affected by that throw in Node.js 7.0.

Only [email protected] causes that warning here, which is required only by [email protected] (apart from being a devDependency). [email protected] is loaded both by orchestrator (which is loaded only by gulp) and directly by gulp.

To my knowledge, there shouldn't be breaking changes in graceful-fs update between 3.x and 4.0 and that should be a clean update.

A straightforward fix for to this would be to release a vinyl-fs 0.3.x with graceful-fs@4, perhaps.

[phated]

Thank you for opening a new issue about this. I understand what you are saying about an error in node 7, but gulp 4 will be out before that time which has a new version of vinyl-fs with a new version of graceful-fs. We aren't going to upgrade to graceful-fs 4 in vinyl-fs 0.3.x because that is a breaking version number in graceful-fs and we don't want to propagate those assumptions to end users. Vinyl-fs has had an insane amount of versions released since 0.3.x that is included with gulp 3.x and we are not focusing efforts on it.

[ChALkeR]

@phated Ok, thanks! I was just making sure that nothing is misunderstood here.

[hgwood]

@phated Would I be correct in saying that all gulp users will have to transition to v4, which means making significant gulpfile modifications, between the moment it is released and the moment Node 7 is released, otherwise they will be stuck in Node <7?

May I also ask what do you mean be "we don't want to propagate those assumptions to end users"? Are you referring to the assumption that "there shouldn't be breaking changes in graceful-fs update between 3.0 and 4.0"? Do you mean that you don't want gulp ^3 users to be automatically upgraded to graceful-fs 4 because that would be automatically propagating a potential breaking change?

Simply trying to understand. :)

[ChALkeR]

@phated

but gulp 4 will be out before that time

Just to confirm — that's still the case, correct? I'm not hurrying or anything, just asking, as three months have passed and we have less than one month to land that change. So any additional data points would be useful here.

[phated]

I confirmed with a member of the node foundation that node 7 will never reach LTS, so it's not really a problem. People should not be using odd releases (they are insanely buggy) and if they are comfortable with that, they should be comfortable enough using our branch. This project is worked on in everyone's spare time, so it'll take however long it takes.

[ChALkeR]

@phated Thanks for the response! I had no intention in causing any pressure here, and I understand the spare time argument perfectly well, everything I do here is also done in my spare time.

I just wanted some clarifications to better plan my further actions, that's why I asked you about the plans on your side again today, as my previous info was from https://github.com/gulpjs/gulp/issues/1640#issuecomment-217247072.

Thanks! :smile:

[ChALkeR]

Note: gulp could be even fixed in a way that would not affect existing vinyl-fs users, except for gulp — fork vinyl-fs@0 as vinyl-fs-0 (a separate module), fix it there, and make gulp use that module. Not sure if that makes sense, though.

[phated]

@ChALkeR fwiw, I think that the scenario described in https://github.com/nodejs/node/issues/8149#issuecomment-240547322 likely exists in >0 user's gulpfles. These are the types of unquantifiable problems we must avoid. graceful-fs even recommends doing a major bump when changing versions. This problem is not going to be solvable on our end.

[ChALkeR]

@phated

fwiw, I think that the scenario described in nodejs/node#8149 (comment) likely exists in >0 user's gulpfles.

that could be true, but I can't come up with a combination of re-evaluating module (graceful-fs) + a monkey-patching module that would actually break in a way that is worse than they already are. Could you?

Btw, I could quickly look up the list of packages that depends on any module combinations (including nested dependencies), if you name me some.

Perhaps running the tests for various ecosystem modules with this fix could help?
/cc @TheAlphaNerd

[ChALkeR]

For example, exactly one module depends on both gulp and mock-fs, it's http://npmjs.com/live with 365 downloads/month. And that doesn't mean that they are actually used together there.

I could look up all the other combinations that you suppose could explode together.

Upd: I did not check devDependencies, though, but I could use those as the entry point, if that would be of any help.

[phated]

@ChALkeR looking up dependent modules doesn't help because I know companies have private 3000+ line gulpfiles. This is why I've already stated that it's more than a gulp ecosystem problem and trickles down to consumers (people that are making gulpfiles) and cannot be quantified. Gulp 3.x is frozen because every tiny change I've tried to make in the past has broken people so we stopped. We cannot safely change this.

[MylesBorins]

@phated if we did a call for open projects and could smoke test a non trivial number of complicated projects would that ease your concern at all?

[phated]

@TheAlphaNerd somewhat, but probably not enough (especially given the amount of feedback I usually receive)

[MylesBorins]

@phated we are going to discuss a number of potential solutions, including a new externally managed fs module similar to readable-stream

One thing that I would like to examine is if there is a way that we can create a smoke test environment you can use to make more informed decisions about breaking changes on the various architectures that node supports. Considering the amount of users that are relying on gulp, and that many of them are not writing node full time, I think it could be a very valuable testing tool for gulp.

Perhaps the results show that we can't do this, or perhaps we cannot get enough information. Even in that situation the tool could potentially help you to make better decisions regarding gulp 4.

Would you be interested in collaborating on this?

[phated]

@TheAlphaNerd yeah, definitely. I want to utilize the time I have to finish gulp 4 but the tool you describe is something I've wanted for a long time.

[ChALkeR]

@phated, ok, another proposal, definitely semver-minor this time.

What about adding an option to gulp@3, which should be explicitly specified when requiring gulp, and which instructs gulp@3 to use graceful-fs@4?

E.g. const gulp = require('gulp/compat');, which if and only if it's the first require of gulp (i.e. if gulp is not loaded by nested deps) would force this and all the following require('gulp') to use graceful-fs@4?

The restriction for that to be the first require('gulp') is required so that deps won't misuse it and change the behavour of everything, — this option would be available only to the top-level gulpfile itself.

Another way — add a command-line flag, but that's less ideal for some cases as it could require changes in the tooling. We could do both, though =).

This way, nothing could possibly break without the topmost user consent, and gulp@3 will still be compatible with everything through and opt-in.

If we don't do this, then even when gulp@4 would be released, users of gulp@3 would have issues becase they would not update immediately.

@TheAlphaNerd, thoughts?

[ChALkeR]

@phated

looking up dependent modules doesn't help because I know companies have private 3000+ line gulpfiles.

Btw, point taken. This makes the devdeps checks not needed at this point, I suppose.

[ChALkeR]

POC: https://github.com/gulpjs/gulp/pull/1760