Remove walk() and walkSync()

[jprichardson]

@RyanZim and I have decided to remove walk() and walkSync().

This was not an easy decision, but the long-term vision of this library is to provide Node.js programmers with an easy to use, batteries-included way to do file system operations. While we've succeeded up to this point, @RyanZim and I must be judicious about our resources.

I think there's a strong case to be made for walk() to be in the library, but the Streams interface may not belong - perhaps generators are a better way to accomplish this?

ES2015 and ES2017 bring generators and async/await. This ushers in a new era of writing JavaScript code. I believe that we'll see a decline in using sync() methods over the years. However, before too many people establish dependencies upon the synchronous code, it should be removed sooner rather than later. We will start with walkSync(). The other sync methods will stay with us for awhile (at least one year).

walkSync() joined fs-extra for the 1.0.0 release (two months ago) and with 2.0.0 coming soon, it's going to be removed along with walk(). Users of walk()have a drop-in replacement with klaw. But we don't want to leave users of walkSync() in the dust (I don't think there are many at this point), but @mawni / @agnivade would you be interested in creating a klaw-sync or walk-sync library for us to point to users?

[manidlou]

That sounds reasonable. I am interested in creating a klaw-sync or walk-sync library. However, since @agnivade is the original auther of walkSync, that'd be great to know what he thinks about it.

[agnivade]

I can understand about removing the sync functions, but why is walk being removed ? If the long-term vision is to allow users

with an easy to use, batteries-included way to do file system operations.

why move walk into a separate library ?

To me, it doesn't make sense to import a library just to use a walk operation. I would just copy-paste a SO answer and be done with it.

Anyways, that was my 2 cents. I am currently involved in other projects too, which will take my time. So I won't be able to actively spend my time in creating/maintaining a library. @mawni - please go ahead and create it. I would be happy to review it if you would like.

[RyanZim]

@agnivade walk is currently contained in https://github.com/jprichardson/node-klaw. fs-extra simply requires it and exports it as fs.walk().

We hope that in the future, fs-extra will have a walk method; however, (as @jprichardson said) we are not sure if klaw's readable stream interface is the best way to do this moving forward. We would like to reevaluate our approach and allow klaw to stabilize a bit more before re-exposing it as part of fs-extra's public API.

Also, as @jprichardson said, we must be judicious about our resources. Currently, @jprichardson & I have very little time to work on klaw. I would like to focus on cleaning up copy (it currently contains quite a bit of legacy code) and adding support for promises to fs-extra. Cutting out walk for the time being will hopefully allow us to focus on these issues/features.

Anyhow, thanks for the quick response @agnivade.

---

@mawni Go ahead and extract walkSync; when you have it in its own repo, please comment here. I would like to link to it from the README and/or the release notes for v2. Thanks in advance!

[RyanZim]

Actual removal done on the v2 branch (master) via #339; going to leave this open to track walkSync progress.

[manidlou]

Thanks @RyanZim. I will comment here as soon as the module is ready.

[manidlou]

@jprichardson, @RyanZim, @agnivade would you please look at https://github.com/mawni/node-klaw-sync? I'd like to know your ideas about it before publishing it.

[agnivade]

Sure, give me some time though.

[RyanZim]

@mawni LGTM, though I haven't actually tried it. (I never used walk or walkSync personally)

PR welcome here and at klaw linking to your library.

[agnivade]

LGTM overall. Couple of minor points -

1. Your minimum node version support seem to be 4. So, it would be good to use the latest ES6 standard. Keeps the code cleaner and easy to maintain.

2. Please add a .eslintrc file and run the linter on travis.

[jprichardson]

@mawni - looks good to me!

Please add a .eslintrc file and run the linter on travis.

It's already there :p standard (which uses eslint) is being called on npm test.

[agnivade]

Ah I see. In that case, I would suggest creating an extra "lint" section in package.json so that developers can run the linter separately through "npm run lint". Its just a small thing that gives extra flexibility.

[manidlou]

@jprichardson, @RyanZim, @agnivade thanks a lot. I truly appreciate it.

I would suggest creating an extra "lint" section in package.json so that developers can run the linter separately through "npm run lint".

Great point. For sure, I will add it to package.json.

@RyanZim I will add the klaw-sync link to docs, then submit PR.

[RyanZim]

SGTM

[RyanZim]

Fully resolved with #343. @mawni PR for klaw welcome anytime.