Deleting rooms using nested promises I have a piece of code that has to deal with some nested promises and shared variables between them. This code is complicated by itself and because of promises syntax is even more difficult to read for those that haven't written it.

The code right now is something like this:

const _deleteOneToOneRooms = function(userId, teamMembers) {
  return function() {
    return User.findOne({ _id: userId })
    .then((userDeleted) =&gt; {
      return Promise.all(teamMembers.map((member) =&gt; {
        return User.findOne({ _id: member }).then((user) =&gt; {
          return {
            id: user._id,
            commonTeams: Utils.arrayIntersect(user.teams, userDeleted.teams).length !== 0
          };
        });
      }))
      .then((usersNoCommonTeam) =&gt; {
        return Promise.all(usersNoCommonTeam.map((user) =&gt; {
          if(user.commonTeams) {
            return Promise.resolve();
          }
          return Room.findOneAndRemove(
            {
              $and: [
                { type: 'one-to-one' },
                { owners: { $all: [user.id, userDeleted] } }
              ]
            });
        }));
      });
    })
    .then((roomsToDelete) =&gt; {
      return Promise.all(roomsToDelete.map((room) =&gt; {
        if(room) {
          return Membership.remove({ room: room._id });
        }
        return Promise.resolve();
      }));
    });
  };
};

someDatabaseLookups.then(_deleteOneToOneRooms(userId, teamMembers))


I have considered to split each then into functions so the overall code is simpler to read:

const _deleteOneToOneRooms = function(userId, teamMembers) {
  return function() {
     return User.findOne({ _id: userId })
     .then((deletedUser) =&gt; {
       return _getUsersWithNoCommonTeams(deletedUser, teamMembers);
     })
     .then((usersNoCommonTeam) =&gt; {
       return _getOneToOneRooms(userId, usersNoCommonTeam);
     })
     .then(_removeRooms);
  };
};


But I don't like having one-line functions only to give them names and at the end if you are reading the code for the first time, you will have to scroll up and down to know what is being done, which from my point of view makes it harder to understand.

So my question is, is there a convention on how to write the cleanest code when dealing with complicated nested promises that use shared variables?
If you have the possibility to use it, you could try the async / await pattern. It is however only part of the standard since ecmascript 2017.

The pattern allows you to wait for promise before continuing the execution path. This means you could rewrite your current code in the following syntax



// mock
const User = {
  findOne: async function( param ) {
    return new Promise(resolve =&gt; setTimeout(() =&gt; resolve( param ), 500));
  }
};

const _deleteOneToOneRooms = function(userId, teamMembers) {
  return async function() {
    let user = await User.findOne({ _id: userId });
    for (const member of teamMembers) {
      let foundMember = await User.findOne({ _id: member });
      // do something with that member
    }
  };
};

const removeUser = async function() {
  return _deleteOneToOneRooms( 5, [10, 15, 16] )().then( () =&gt; console.log('done') );
};

// an async function returns a promise in itself, so from outside an async function, you can use the `then` syntax
removeUser().then(() =&gt; console.log('async also done'));
// this will be logged first
console.log('remove user was called, rest is async');




This only gives you an idea from how you could reformat your code using this pattern.

The last piece of code can then also be rewritten like:

const _deleteOneToOneRooms = function(userId, teamMembers) {
  return async function() {
     let deletedUser = await User.findOne({ _id: userId });
     let usersNoCommonTeam = await _getUsersWithNoCommonTeams(deletedUser, teamMembers);
     return await _getOneToOneRooms(userId, usersNoCommonTeam);
  };
};