paulmillr / readdirp

Recursive version of fs.readdir with streaming api.
https://paulmillr.com
MIT License
381 stars 51 forks source link

[Feature request] Wait for callbacks completion #59

Closed ehmicky closed 5 years ago

ehmicky commented 5 years ago

allProcessed is fired when readdirp has fired all fileProcessed callbacks. However some fileProcessed callbacks might still be running if they are async (i.e. if they return a promise). It would be nice if readdirp could allow users to wait for all fileProcessed promises completion.

At the moment users can achieve this with some extra code, by pushing promises to a shared array during fileProcessed, then wait for all promises once allProcessed has been fired. But it would be simpler if readdirp could directly do this.

A few suggestions on possible implementations (but any others will do):

For information, this is what I have to do at the moment to achieve this:

  return new Promise((resolve, reject) => {
    const promises = []

    readdirp(
      options,
      file => {
        const promise = asyncFunction(file)
        promises.push(promise)
      },
      errors => {
        if (Array.isArray(errors)) {
          return reject(errors[errors.length - 1])
        }

        resolve(Promise.all(promises))
      },
    )
  })
paulmillr commented 5 years ago

We're dropping callbacks in v3. They are very inefficient. We're using real streams, and it's very simple to construct any data structure with them; see readme examples.

ehmicky commented 5 years ago

Can't callbacks use the same logic as the streams under the hood? I.e. they would have the same performance.

For example when the user wants a callback or promise, the library could iterate over the stream, then resolve the callback or promise.

Some users might not be able to operate on results incrementally (e.g. if only need to know total number of items). For those users, using either for await or a small utility function like get-stream is an option, but a callback or promise would be simpler.

paulmillr commented 5 years ago

We are exposing readdir.promise in v3, which should be enough for these cases:

const readdirp = require('readdirp').promise;
const entries = await readdirp('.');
ehmicky commented 5 years ago

Yes exactly that's perfect :+1: