how to refactor waterfall .then()

Question

This code here loads data using actions and would be series, but its going to be hard to edit this code add another API load, and the syntax is not clear.

this.props.loadNutMatrixes({perPage:'all'}).then(()=>{
      this.props.loadIngredients().then(()=>{
        this.props.getBadge().then(()=>{
          this.props.loadNutInfoItems({perPage:'all'}).then(()=>{
            this.props.getItemSize().then(()=>{
              this.props.getSingleMenuCategory(this.props.category_uid).then(()=>{
                this.props.loadAllStores(({per_page:'all'})).then(()=>{
                  if (this.props.selectedMenuItem ){
                    initialize("addNewMenuItem", {
                      ...this.props.selectedMenuItem
                    })
                  }
                })
              })
            })
          })
        })
      })
    })

nem035 · Accepted Answer · 2017-07-17T15:05:05.300

You can convert this to a vertical structure by chaining the promises instead of nesting:

this.props.loadNutMatrixes({perPage:'all'})
  .then(() => this.props.loadIngredients())
  .then(() => this.props.getBadge())
  .then(() => this.props.loadNutInfoItems({perPage:'all'}))
  .then(() => this.props.getItemSize())
  .then(() => this.props.getSingleMenuCategory(this.props.category_uid))
  .then(() => this.props.loadAllStores(({per_page:'all'})))
  .then(() => {
    if (this.props.selectedMenuItem) {
      initialize("addNewMenuItem", {
        ...this.props.selectedMenuItem
      })
    }
  });

A possible improvement could be to wrap all promise-creating functions that receive arguments into functions without arguments and pass those as props instead:

loadAllNutMatrixes() {
  return this.loadNutMatrixes({ perPage: 'all' });
}

loadAllNutInfoItems() {
  return this.loadNutInfoItems({ perPage: 'all' });
}

getSingleMenuCategoryFromId() {
  return this.getSingleMenuCategory(this.category_uid);
}

loadEveryStory() {
  return this.loadAllStores({ perPage: 'all' });
}

Then you could refactor the last step into its own method:

onChainFinished() {
  if (this.props.selectedMenuItem) {
    initialize("addNewMenuItem", {
      ...this.props.selectedMenuItem
    })
  }
}

And combine the two with some destructuring to achieve a cleaner chain:

const { props } = this;
props.loadAllNutMatrixes()
  .then(props.loadIngredients)
  .then(props.getBadge)
  .then(props.loadAllNutInfoItems)
  .then(props.getItemSize)
  .then(props.getSingleMenuCategoryFromId)
  .then(props.loadEveryStore)
  .then(this.onChainFinished);

EDIT based on your comment

using something like promise.all but in series way !

There is no native method to chain Promises but you can build a helper method suitable for your use case to do this. Here's a general example:

// `cp` is a function that creates a promise and 
// `args` is an array of arguments to pass into `cp`
chainPromises([
  { cp: this.props.loadNutMatrixes, args: [{perPage:'all'}] },
  { cp: this.props.loadIngredients },
  { cp: this.props.getBadge },
  { cp: this.props.loadNutInfoItems, args: [{perPage:'all'}] },
  { cp: this.props.getItemSize },
  { cp: this.props.getSingleMenuCategory, args: [this.props.category_uid] },
  { cp: this.props.loadAllStores, args: [{per_page:'all'}] }
]).then(() => {
  if (this.props.selectedMenuItem) {
    initialize("addNewMenuItem", {
      ...this.props.selectedMenuItem
    })
  }
});

function chainPromises(promises) {
  return promises.reduce(
    (chain, { cp, args = [] }) => {
      // append the promise creating function to the chain
      return chain.then(() => cp(...args));
    }, Promise.resolve() // start the promise chain from a resolved promise
  );
}

If you use the same approach as above to refactor methods with arguments, it would clean this code up as well:

const { props } = this;
chainPropsPromises([
  props.loadAllNutMatrixes,
  props.loadIngredients,
  props.getBadge,
  props.loadAllNutInfoItems,
  props.getItemSize,
  props.getSingleMenuCategoryFromId,
  props.loadEveryStory
])
.then(this.onChainFinished);

function chainPropsPromises(promises) {
  return promises.reduce(
    (chain, propsFunc) => (
      chain.then(() => propsFunc());
    ), Promise.resolve()
  );
}

they must be series so they can't be in promise.all but the vertical structure was cool , but it's just like indent refactoring , i want syntax refactoring ! — amir hosein ahmadi, Oct 08 '16 at 08:02
why `.then(() => this.props.loadIngredients())` and not simply `.then(this.props.loadIngredients)` ? — vsync, Oct 08 '16 at 08:50
@vsync no specific reason, just to be consistent (and keep cleaner code format) since some functions take arguments so they require to be wrapped in a callback. — nem035, Oct 08 '16 at 09:05
@nem035 - they can still "take" arguments. you just create tons of unneeded wrapper functions. wrapping is only used when you want to modify the response before passing it along (which is only by itself needed in complex situations where the callback function itself accepts a different response than the ajax supplies. — vsync, Oct 08 '16 at 10:38
@vsync the only reason I'm doing it is for vertical alignment of each function so the code is IMO easier to read. The point was how to build the promise chain, not to worry about performance of creating a couple extra function wrappers (which, unless this code runs millions of times, is insignificant anyways). — nem035, Oct 08 '16 at 18:00

score 0 · Answer 2 · answered Oct 08 '16 at 07:38

Return the promise to chain it to the outer promise.

this.props.loadNutMatrixes({perPage:'all'}).then(()=>{
  return this.props.loadIngredients()
})
.then(()=>{
  return this.props.getBadge()
})
.then(()=>{
  return this.props.loadNutInfoItems({perPage:'all'})
})
.then(()=>{
  return this.props.getItemSize()
})
.then(()=>{
  return this.props.getSingleMenuCategory(this.props.category_uid)
});

...

score 0 · Answer 3 · answered Oct 08 '16 at 07:39

If the promises aren't dependent on each other, I'd use Promise.all:

const {props} = this;
Promise.all([
    props.loadNutMatrixes({perPage: 'all'}),
    props.loadIngredients(),
    props.getBadge(),
    props.loadNutInfoItems({perPage: 'all'}),
    props.getItemSize(),
    props.getSingleMenuCategory(props.category_uid),
    props.loadAllStores(({per_page: 'all'})),
]).then(() => {
    if (props.selectedMenuItem) initialize("addNewMenuItem", {...props.selectedMenuItem});
});

they not depend on each other but must be series , in `Promise.all` they all called in parallel ! — amir hosein ahmadi, Oct 08 '16 at 07:58

how to refactor waterfall .then()

3 Answers3