Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport graceful-fs update to Gulp 3 #2146

Closed
demurgos opened this issue Apr 8, 2018 · 4 comments
Closed

Backport graceful-fs update to Gulp 3 #2146

demurgos opened this issue Apr 8, 2018 · 4 comments

Comments

@demurgos
Copy link
Member

demurgos commented Apr 8, 2018

There is a Node issue reporting that Gulp 3 may not work properly with the upcoming Node 10 release. This is caused by the dependency on graceful-fs@3 because it monkey-patches some internal modules.
gulp@4 does not have this issue because it uses graceful-fs@4.

I haven't tried to reproduce the issue yet. According to the Node issue, they were able to patch a package (natives) to fix the problem but this solution may not be reliable.

Would it be possible to issue a patch update to gulp@3 to use graceful-fs@4? Is it a breaking change? Hard to implement?
An alternative would be to recommend the usage of gulp@4 (using engines.node?).

@phated
Copy link
Member

phated commented Apr 8, 2018

That's a semver breaking change. We're not doing anything about their issue.

@phated phated closed this as completed Apr 8, 2018
@yocontra
Copy link
Member

yocontra commented Apr 8, 2018

FYI This has been fixed already - the natives module used by graceful-fs has updated so we won't be breaking on node 10.

@demurgos
Copy link
Member Author

demurgos commented Apr 8, 2018

Thanks, I've read that it was fixed but it's only temporary according to this comment:

@addaleax has stated that natives is not a viable long-term solution, with a lifespan better measured in months than years. If I were gulp, I'd look into upgrading.

I suppose it means that it may break again in Node 11 or 12. But yes, it's already fixed in Gulp. graceful-fs explained that their changes tend to have large impacts and @phated confirmed that it's a breaking change. It just means that people may need to upgrade Gulp in the future to use the latest versions of Node: it's not that bad.

@bnoordhuis
Copy link

I suppose it means that it may break again in Node 11 or 12

Or in Node 10. Node 10 will be supported until 2021 (it's LTS) but you can't count on natives working for that long.

(In fact, you can probably count on it not working for that long.)

A way forward for gulp@3 is to back-port the amazing-graceful-fs changes from graceful-fs@4 to graceful-fs@3. I wrote amazing-graceful-fs, I'm willing to help review.

mbland pushed a commit to mbland/slush that referenced this issue May 26, 2018
Gulp 4.0.0 switched its task execution engine from `orchestrator` to
`undertaker`. As a result, certain methods and events from Gulp 3.9.1
upon which `slush` depended disappeared:

  gulpjs/gulp#755

Supporting Gulp 4.0.0 is important because Node 10 broke the
`graceful-fs` package upon which Gulp 3.9.1 depends. While there's a
workaround (updating the `natives` package), it places a burden on users
that still doesn't guarantee that Gulp 3.9.1 will remain future-proof:

  gulpjs/gulp#2146 (comment)
  gulpjs/gulp#2162 (comment)
  nodejs/node#19786 (comment)

As it turned out, the changes required to support both versions were
fairly straighforward, and should ensure that Slush remains future-proof
until the next major Gulp update.
mbland pushed a commit to mbland/slush that referenced this issue May 29, 2018
Gulp 4.0.0 switched its task execution engine from `orchestrator` to
`undertaker`. As a result, certain methods and events from Gulp 3.9.1 upon which
`slush` depended disappeared:

  gulpjs/gulp#755

Supporting Gulp 4.0.0 is important because Node 10 broke the `graceful-fs`
package upon which Gulp 3.9.1 depends. While there's a workaround (updating the
`natives` package), it places a burden on users that still doesn't guarantee
that Gulp 3.9.1 will remain future-proof:

  gulpjs/gulp#2146 (comment)
  gulpjs/gulp#2162 (comment)
  nodejs/node#19786 (comment)

As it turned out, the changes required to support both versions were fairly
straighforward, and should ensure that Slush remains future-proof until the next
major Gulp update.

NOTE: The test tasks are now all asynchronous via a `done` callback, since Gulp
4 doesn't support synchronous tasks. Any synchronous slushfile task will need to
be updated.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants