Pass `allItems` and `currentItems` to `_pagination.paginate()` #1100

jaulz · 2020-03-03T10:17:27Z

Sometimes you need alltems or the items from the last response in the paginate function (e.g. to compare if current length of items are less than requested size).

Checklist

I have read the documentation.
I have included a pull request description of my changes.
I have included some tests.
If it's a new feature, I have included documentation updates.

szmarczak · 2020-03-03T13:16:02Z

e.g. to compare if current length of items are less than requested size

You can do that in a shouldContinue function. I don't know why you duplicate the all array (it's named current in your code).

jaulz · 2020-03-03T13:36:50Z

No, I do not duplicate the all array but instead create a current array per iteration (i.e. per request). Maybe it's a bit misleading but with "current" I mean the items of the current request.

Think about this scenario:
I have an API where I can set a limit and offset but that does not return any information about whether the response contains all items or not. If I simply compare the passed limit with the returned items I can decide if a next request should be triggered or not.

szmarczak · 2020-03-03T14:04:20Z

If I simply compare the passed limit with the returned items I can decide if a next request should be triggered or not.

I don't get it. Why don't just modify the offset in _pagination.paginate and if the response contains no items, stop?

jaulz · 2020-03-03T14:14:38Z

But that would trigger an unnecessary request wouldn't it? If the last response contains only 10 of 20 items, it would anyway try another time with an updated offset.

szmarczak · 2020-03-03T14:15:31Z

Oh, I get you now! :D

source/types.ts

Co-Authored-By: Szymon Marczak <36894700+szmarczak@users.noreply.github.com>

jaulz · 2020-03-03T14:23:40Z

Sorry, maybe I was not clear enough before 😊

szmarczak · 2020-03-03T14:26:23Z

You have nothing to be sorry for, that's the point of a conversation - to make things clear :D

source/types.ts

test/pagination.ts

Co-Authored-By: Szymon Marczak <36894700+szmarczak@users.noreply.github.com>

test/pagination.ts

szmarczak · 2020-03-03T15:05:32Z

Don't forget to update the docs :)

jaulz · 2020-03-04T07:47:59Z

Here we go 😊

readme.md

jaulz · 2020-03-04T18:19:00Z

Good idea. I just added the example to the paginate function part.

szmarczak · 2020-03-04T19:59:30Z

readme.md

@@ -680,25 +680,27 @@ A function that transform [`Response`](#response) into an array of items. This i
 ###### \_pagination.paginate

 Type: `Function`\
-Default: [`Link` header logic](source/index.ts)
+Default: `(response, allItems, currentItems) => getLinkFromHeaders(response)` (find details [here](source/index.ts))


This change is unnecessary

Okay, I restored it but changed the sentence afterwards to make clear how the signature of the function should be. This should make it easier for users without diving into the code first.

szmarczak · 2020-03-04T20:00:10Z

readme.md


 A function that returns an object representing Got options pointing to the next page. If there are no more pages, `false` should be returned.

+For example, if you want to stop when the response contains less items that expected, you should use `(response, allItems, currentItems) => currentItems.length < LIMIT ? false : { url: getNextLink(response, allItems, currentItems) }`.


the LIMIT should be a response.request.options.searchParams.entriesPerPage

that expected -> than expected

Okay, it's updated.

szmarczak · 2020-03-04T20:00:58Z

tests are failing somehow

jaulz · 2020-03-05T07:59:27Z

I think the issue with the tests is independently since it first arose after documentation changes. Also, it also fails for Node 13 only...

readme.md

test/pagination.ts

jaulz · 2020-03-05T15:55:06Z

Thanks for pushing it forward! 😊

sindresorhus · 2020-03-07T05:18:53Z

readme.md

+For example, if you want to stop when the response contains less items than expected, you can use something like this:
+
+```js
+(response, allItems, currentItems) => {


I would prefer the example to be fully runnable, meaning that it includes the import statement and got(…).

I just extended the example.

feat: pass allItems and currentItems to paginate

59a2ea1

szmarczak reviewed Mar 3, 2020

View reviewed changes

source/types.ts Outdated Show resolved Hide resolved

szmarczak reviewed Mar 3, 2020

View reviewed changes

source/types.ts Outdated Show resolved Hide resolved

szmarczak changed the title ~~feat: pass allItems and currentItems to paginate~~ Pass allItems and currentItems to _pagination.paginate() Mar 3, 2020

jaulz and others added 3 commits March 3, 2020 15:22

fix: add currentItems to filter

791f297

Co-Authored-By: Szymon Marczak <36894700+szmarczak@users.noreply.github.com>

fix: add currentItems to shouldContinue

1186829

Co-Authored-By: Szymon Marczak <36894700+szmarczak@users.noreply.github.com>

fix: add current to filter and shouldContinue

d1d521b

szmarczak reviewed Mar 3, 2020

View reviewed changes

source/types.ts Show resolved Hide resolved

szmarczak reviewed Mar 3, 2020

View reviewed changes

source/types.ts Show resolved Hide resolved

Julian Hundeloh added 2 commits March 3, 2020 14:39

test: check if allItems and currentItems are arrays

4232b81

style: fix lint

a4b1a2b

szmarczak reviewed Mar 3, 2020

View reviewed changes

test/pagination.ts Outdated Show resolved Hide resolved

szmarczak reviewed Mar 3, 2020

View reviewed changes

test/pagination.ts Outdated Show resolved Hide resolved

jaulz and others added 3 commits March 3, 2020 15:54

fix: use t helper

7b01010

Co-Authored-By: Szymon Marczak <36894700+szmarczak@users.noreply.github.com>

fix: use t helper

bccd95e

Co-Authored-By: Szymon Marczak <36894700+szmarczak@users.noreply.github.com>

test: fix test

de3c26e

szmarczak reviewed Mar 3, 2020

View reviewed changes

test/pagination.ts Outdated Show resolved Hide resolved

Julian Hundeloh added 3 commits March 4, 2020 07:28

fix: remove typos

3f29cc8

docs: add currentItems to readme

a7c5c69

fix: lint

68cc75a

szmarczak reviewed Mar 4, 2020

View reviewed changes

readme.md Outdated Show resolved Hide resolved

docs: add example to readme

872e27b

szmarczak reviewed Mar 4, 2020

View reviewed changes

docs: update readme

b21be19

szmarczak reviewed Mar 5, 2020

View reviewed changes

readme.md Outdated Show resolved Hide resolved

docs: use block

3d7959e

szmarczak reviewed Mar 5, 2020

View reviewed changes

readme.md Outdated Show resolved Hide resolved

szmarczak added 5 commits March 5, 2020 15:19

Update readme.md

c31f2f9

Update readme.md

86d57b9

Update readme.md

b10be23

Update readme.md

09c41c0

Update create.ts

8054ab1

szmarczak reviewed Mar 5, 2020

View reviewed changes

test/pagination.ts Outdated Show resolved Hide resolved

Update test/pagination.ts

e49dec2

szmarczak approved these changes Mar 5, 2020

View reviewed changes

szmarczak requested a review from sindresorhus March 5, 2020 15:41

Update readme.md

4f2e21a

sindresorhus requested changes Mar 7, 2020

View reviewed changes

jaulz and others added 3 commits March 9, 2020 08:44

docs: add full example to readme

9850542

docs: add import and console.log to example

badc8be

Update readme.md

68f0c42

sindresorhus approved these changes Mar 9, 2020

View reviewed changes

sindresorhus requested a review from szmarczak March 9, 2020 07:57

szmarczak approved these changes Mar 9, 2020

View reviewed changes

sindresorhus merged commit 1cddd52 into sindresorhus:master Mar 9, 2020

jaulz deleted the patch-2 branch March 9, 2020 13:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GitHub Sponsors

Pass `allItems` and `currentItems` to `_pagination.paginate()` #1100

Pass `allItems` and `currentItems` to `_pagination.paginate()` #1100

jaulz commented Mar 3, 2020

szmarczak commented Mar 3, 2020

jaulz commented Mar 3, 2020

szmarczak commented Mar 3, 2020

jaulz commented Mar 3, 2020

szmarczak commented Mar 3, 2020

jaulz commented Mar 3, 2020

szmarczak commented Mar 3, 2020

szmarczak commented Mar 3, 2020

jaulz commented Mar 4, 2020

jaulz commented Mar 4, 2020

szmarczak Mar 4, 2020

jaulz Mar 5, 2020

szmarczak Mar 4, 2020

szmarczak Mar 4, 2020

jaulz Mar 5, 2020

szmarczak commented Mar 4, 2020

jaulz commented Mar 5, 2020

jaulz commented Mar 5, 2020

sindresorhus Mar 7, 2020

jaulz Mar 9, 2020


		A function that returns an object representing Got options pointing to the next page. If there are no more pages, `false` should be returned.

		For example, if you want to stop when the response contains less items that expected, you should use `(response, allItems, currentItems) => currentItems.length < LIMIT ? false : { url: getNextLink(response, allItems, currentItems) }`.

Pass allItems and currentItems to _pagination.paginate() #1100

Pass allItems and currentItems to _pagination.paginate() #1100

Conversation

jaulz commented Mar 3, 2020

Checklist

szmarczak commented Mar 3, 2020

jaulz commented Mar 3, 2020

szmarczak commented Mar 3, 2020

jaulz commented Mar 3, 2020

szmarczak commented Mar 3, 2020

jaulz commented Mar 3, 2020

szmarczak commented Mar 3, 2020

szmarczak commented Mar 3, 2020

jaulz commented Mar 4, 2020

jaulz commented Mar 4, 2020

szmarczak Mar 4, 2020

Choose a reason for hiding this comment

jaulz Mar 5, 2020

Choose a reason for hiding this comment

szmarczak Mar 4, 2020

Choose a reason for hiding this comment

szmarczak Mar 4, 2020

Choose a reason for hiding this comment

jaulz Mar 5, 2020

Choose a reason for hiding this comment

szmarczak commented Mar 4, 2020

jaulz commented Mar 5, 2020

jaulz commented Mar 5, 2020

sindresorhus Mar 7, 2020

Choose a reason for hiding this comment

jaulz Mar 9, 2020

Choose a reason for hiding this comment

Pass `allItems` and `currentItems` to `_pagination.paginate()` #1100

Pass `allItems` and `currentItems` to `_pagination.paginate()` #1100