Node-slack-sdk: Method for paginated API calls

[aoberoi]

Description

Automatic pagination is implemented in the SDK, but its not recommended to depend on this functionality. That is because the SDK will paginate until the end of the collection, and then merge the contents from the individual pages in a destructive manner. However, manually paginating takes a lot of effort to do right - recursion isn't easy.

It would be useful to for the WebClient to provide a method that "did pagination right". It would be better than automatic pagination because it should "yield" the full results for each page (not destructive), and provide a means to
stop pagination at any point (not needlessly exhaustive).

Here's the current proposal:

const { WebClient } = require('@slack/client');

const web = new WebClient(process.env.SLACK_TOKEN);

// I'd like to find a public channel with a certain name, but this requires paging
// through potentially all the channels.

const channelName = 'team-awesome';

const done = web.paginate(
  // the first argument is the method name of a cursor-paginated method
  'conversations.list',

  // the second argument is the options to pass to that method
  { types: 'public_channel' }

  // the third argument is a predicate which returns `true` when pagination
  // should stop (so that not returning anything is treated as continue).
  // it is called with the individual page result
  (res) => res.channels.some(c => c.name === channelName),

  // the fourth (and optional) argument is a reducer function
  // it is called with an accumulator, the individual page result, and the page number
  // acc is initialized to undefined
  (acc, res, idx) => {
    // the accumulated value will be what the Promise returned (done) resolves to
    for (channel of res.channels) {
      if (channel.name === channelName) { return channel.id }
    }
  },
});

(async () => {
  // The method returns a Promise when pagination is complete. The resolved
  // value could be undefined (when there's no reducer defined) or the
  // accumulated value (when there is a reducer defined).
  const channelId = await done;
  console.log(`Channel name: ${channelName}, ID: ${channelId}`);
})()

Requirements (place an x in each of the [ ])

• [x] I've read and understood the Contributing guidelines and have done my best effort to follow them.
• [x] I've read and agree to the Code of Conduct.
• [x] I've searched for any related issues and avoided creating a duplicate issue.

[aoberoi]

Problem with this design: there's no way to know what method to call for the next page, and with which options. Should we assume the options are static (the same for every page) except for the cursor?

[Lemmmy]

I think it would be reasonable to assume the options are static - changing the options mid-pagination is going to change the cursor in most cases anyway. As for the method - perhaps pass the method and the options separately, like this:

const done = WebClient.paginate(
  web.conversations.list, // pass the method itself
  { types: 'public_channel' }, // then the options
  (res) => // predicate as before
});

[aoberoi]

Thanks for the feedback!

When you say pass the method itself, I think that would work, but that's just an alias to WebClient.apiCall() with the first argument bound to a specific string method name. Do you think it would be any more or less helpful if instead it was like this:

const done = web.paginate(
  'conversations.list', // method name
  { types: 'public_channel' }, // then the options
  (res) => // predicate as before
});

Notice that I changed it from a static helper method on the WebClient class to an instance method, and then changed the function into a string.

I think the advantage to this approach is we could do all the pre-request work that happens in WebClient.apiCall() only once, where as if the argument was the function and the helper was a static function, we'd end up calling all the pre-request work before every page.

[Lemmmy]

That would work too, and seems much more efficient as you pointed out.

[aoberoi]

Labeling as good first issue because we have some convergence on the API design (minus probably bike-shedding on the names). However, I do think this is a pretty advanced contribution, so its probably best for someone who understands async iteration and recursion pretty well.

[aoberoi]

One more idea: an optional fourth argument that is a reducer function (as specified for Array.prototype.reduce).

If its supplied, then the returned Promise (done in the example above) would resolve to the accumulator value when the predicate indicated that pagination should stop.

[aoberoi]

Updated initial post to reflect current consensus.

[aoberoi]

Updating the title to reflect the fact that if this is implemented, we should also log deprecation warnings for any usage of automatic pagination found at runtime.

If we get an implementation of this feature (before v4 is EOL), we should create another issue to track the removal of automatic pagination in v5.

[aoberoi]

Other thoughts:

• How can I choose the page size? By inspecting the options (second argument), a limit value will be captured and used as the page size for all subsequent requests. If there isn't one specified, we'll use a default (200 was the default from automatic pagination).

• Is there an API that will return an AsyncIterator so that I might use the for-await-of loop in my esnext code? 🤔

[aoberoi]

An idea about the AsyncIterator: If the third argument (predicate) is optional, and its not passed in, the method can return an AsyncIterator that is suitable for a for-await-of loop.

The implementation would be pretty gnarly. There would probably be two diverging code paths, and one of those might be recursive.

[aoberoi]

let channelId = undefined;
for await (page of web.paginate('conversations.list', {  })) {
    for (channel of page.channels) {
      if (channel.name === channelName) { channelId = channel.id; break; }
    }
}

const channelId = undefined;
await web.paginate('conversations.list', { types: 'public_channel' }, (res) => 
  return res.channels.some((c) => {
    if (c.name === channelName) {
      channelId = c.id;
      return true;
    }
  }),
});

[aoberoi]

👆 that was just an accidental click