Changes to the .pagination API

[PopGoesTheWza]

What problem are you trying to solve?

Improve .pagination API (and possibly performance and stability)

Describe the feature

I would humbly suggest the following 2 breaking changes:

• Switch order of allItems and currentItems parameters of pagination.paginate, pagination.filter and pagination.shouldContinue

My reasoning about parameters order is that probably most use cases don't need the allItems parameters, with currentItems typically being more used. To be confirmed by other users, but if indeed currentItems is used more than allItems then it should reflect in the order of parameters.

It doesn't change how the Got code works but could save custom pagination.paginate, pagination.filter and pagination.shouldContinue from having an unused and unaesthetic _ as the second parameter... ;)

• Default value for stackAllItem to false

As per the default of allItems, it implies a waste of CPU and RAM resources when unused. This is especially noticeable when iterating large datasets (and can results in a node.js crash in worst case scenarios)

Checklist

• I have read the documentation and made sure this feature doesn't already exist.

[szmarczak]

unaesthetic _ as the second parameter...

You can use a rule that only forces you to have _ only at the beginning, so you can have _allItems. I cannot decide on this on either.

Regarding the stackAllItems option, it mainly depends on what most people do. We need more feedback.

[fiznool]

FWIW, when there is more than one value to be passed in to a function I prefer to pass in an object so it can be destructured. This permits the access of any/all of the properties in the object and also allows further properties to be easily added at a later date.

So the full signature for pagination.paginate would resemble:

paginate({ response, allItems, currentItem })

If you only wanted currentItem:

paginate({ currentItem })

[PopGoesTheWza]

I too usually prefer object parameters when possible/sensible. I don't think it is common with callback signatures, though...

[szmarczak]

@fiznool Good idea. We already did this with retry.calculateDelay.

[PopGoesTheWza]

This brought me into checking this.

[sindresorhus]

I agree. We should make it an object instead in Got v12 (not soon, unfortunately).

[sindresorhus]

As for:

Default value for stackAllItem to false

I haven't yet made up my mind. If you use the pagination API, please chime in on this.

[sindresorhus]

If anyone wants to see this happen in Got 12, now would be a time to submit a pull request, otherwise, we'll defer it to Got 13.

[PopGoesTheWza]

@szmarczak sorry for the late PRs. Looks like they won't be needed after all.