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

remove async lib #1089

Closed
jimmywarting opened this issue Aug 16, 2018 · 3 comments
Closed

remove async lib #1089

jimmywarting opened this issue Aug 16, 2018 · 3 comments
Labels

Comments

@jimmywarting
Copy link

jimmywarting commented Aug 16, 2018

The async lib are huge and you only use one function (series) at one place

const async = require('async');

resolve(cb) {
cb = cb || (() => {});
const resolveConflicts = conflict => {
return next => {
if (!conflict) {
next();
return;
}
this.collision(conflict.file, status => {
// Remove the resolved conflict from the queue
_.pull(this.conflicts, conflict);
conflict.callback(null, status);
next();
});
};
};
async.series(this.conflicts.map(resolveConflicts), cb.bind(this));
}

I believe this dose the exact same thing, (I have not tested the code - it's a proposal, that i think i got right)

  resolve(cb) {
    cb = cb || (() => {});
    const iterator = this.conflicts.values();
    
    const resolveConflicts = iterator => {
      const {value:conflict, done} = iterator.next();

      // use this line if this.conflicts.entries() is being used instead
      // const {value:[i, conflict], done} = iterator.next();

      if (done) cb();
      else if (!conflict) resolveConflicts();
      else this.collision(conflict.file, status => {
        // Remove the resolved conflict from the queue
        _.pull(this.conflicts, conflict);
        conflict.callback(null, status);
        resolveConflicts(iterator);
      });
    };

    resolveConflicts(iterator);
  }

i saw over at node.green that array.values() was not so widely compatible but .entries() was so i also showed how to destruct using entries (also possible to remove destructing all together...)

@SBoudrias
Copy link
Member

Might be able to also reuse something like https://www.npmjs.com/package/async-series ?

Either way, PR is welcomed as long as we support current Node LTS (>= 6)

@jimmywarting
Copy link
Author

I don't think you need a library for that, just shown you how it could be done without any dependencies

@github-actions
Copy link
Contributor

This issue is stale because it has been open 15 days with no activity. Remove stale label or comment or this will be closed in 5 days

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants