From 4773842dbf93161d34826da6d64f387cca50e034 Mon Sep 17 00:00:00 2001 From: "Garen J. Torikian" Date: Mon, 2 Nov 2026 18:09:27 -0500 Subject: [PATCH] fix: Reuse the first page in autoPagination Every list method already makes the first request before handing the result to AutoPaginatable, but autoPagination() ignored that page and requested the list from the start again with limit=100. Every autoPagination() call therefore cost at least two round trips, even when the first response already held the whole list. Continuing from the first page's cursor drops the redundant request. Follow-up pages still walk forward from that page, which also makes the before-cursor special case from #1720 unnecessary. Fixes #1746 --- src/common/utils/pagination.spec.ts | 41 ++++++++++++++++--------- src/common/utils/pagination.ts | 23 +++++--------- src/feature-flags/feature-flags.spec.ts | 11 +++---- src/pipes/pipes-requests.spec.ts | 2 -- 4 files changed, 38 insertions(+), 39 deletions(-) diff --git a/src/common/utils/pagination.spec.ts b/src/common/utils/pagination.spec.ts index 128432b5c..fd8366c7a 100644 --- a/src/common/utils/pagination.spec.ts +++ b/src/common/utils/pagination.spec.ts @@ -56,6 +56,24 @@ describe('AutoPaginatable', () => { expect(mockApiCall).not.toHaveBeenCalled(); }); + it('returns initial data without a request when there is no next page', async () => { + const initialData: List = { + object: 'list', + data: [{ id: '1' }, { id: '2' }], + listMetadata: { before: null, after: null }, + }; + + const paginatable = new AutoPaginatable( + initialData, + mockApiCall, + ); + + const result = await paginatable.autoPagination(); + + expect(result).toEqual(initialData.data); + expect(mockApiCall).not.toHaveBeenCalled(); + }); + it('paginates through all pages', async () => { const initialData: List = { object: 'list', @@ -88,6 +106,8 @@ describe('AutoPaginatable', () => { const result = await resultPromise; expect(result).toEqual([ + { id: '1' }, + { id: '2' }, { id: '3' }, { id: '4' }, { id: '5' }, @@ -97,7 +117,7 @@ describe('AutoPaginatable', () => { expect(mockApiCall).toHaveBeenCalledTimes(2); expect(mockApiCall).toHaveBeenNthCalledWith(1, { limit: 100, - after: undefined, + after: 'cursor1', }); expect(mockApiCall).toHaveBeenNthCalledWith(2, { limit: 100, @@ -116,11 +136,6 @@ describe('AutoPaginatable', () => { }; mockApiCall - .mockResolvedValueOnce({ - object: 'list', - data: [{ id: 'u11' }, { id: 'u12' }], - listMetadata: { before: 'u11', after: 'u12' }, - }) .mockResolvedValueOnce({ object: 'list', data: [{ id: 'u13' }, { id: 'u14' }], @@ -140,7 +155,7 @@ describe('AutoPaginatable', () => { const resultPromise = paginatable.autoPagination(); - await jest.advanceTimersByTimeAsync(700); + await jest.advanceTimersByTimeAsync(350); const result = await resultPromise; @@ -152,20 +167,16 @@ describe('AutoPaginatable', () => { { id: 'u15' }, ]); - expect(mockApiCall).toHaveBeenCalledTimes(3); + expect(mockApiCall).toHaveBeenCalledTimes(2); expect(mockApiCall).toHaveBeenNthCalledWith(1, { - limit: 100, - before: 'u21', - }); - expect(mockApiCall).toHaveBeenNthCalledWith(2, { limit: 100, after: 'u12', }); - expect(mockApiCall).toHaveBeenNthCalledWith(3, { + expect(mockApiCall).toHaveBeenNthCalledWith(2, { limit: 100, after: 'u14', }); - for (const call of mockApiCall.mock.calls.slice(1)) { + for (const call of mockApiCall.mock.calls) { expect(call[0]).not.toHaveProperty('before'); } }); @@ -233,7 +244,7 @@ describe('AutoPaginatable', () => { await resultPromise; expect(mockApiCall).toHaveBeenNthCalledWith(1, { - after: undefined, + after: 'cursor1', status: 'active', limit: 100, }); diff --git a/src/common/utils/pagination.ts b/src/common/utils/pagination.ts index dbb21c22b..2bd77c3df 100644 --- a/src/common/utils/pagination.ts +++ b/src/common/utils/pagination.ts @@ -24,23 +24,17 @@ export class AutoPaginatable< return this.list.listMetadata; } - private async *generatePages( - params: PaginationOptions, - ): AsyncGenerator { + private async *generatePages(after: string): AsyncGenerator { + // Follow-up pages always walk forward from the first page, so `before` is dropped const { before, ...options } = this.options; - const result = await this.apiCall({ - ...options, - limit: 100, - ...(params.before ? { before } : {}), - after: params.after, - }); + const result = await this.apiCall({ ...options, limit: 100, after }); yield result.data; if (result.listMetadata.after) { // Delay of 4rps to respect list users rate limits await new Promise((resolve) => setTimeout(resolve, 350)); - yield* this.generatePages({ after: result.listMetadata.after }); + yield* this.generatePages(result.listMetadata.after); } } @@ -49,16 +43,13 @@ export class AutoPaginatable< * Returns the first result if `options.limit` is passed to the first request. */ async autoPagination(): Promise { - if (this.options.limit) { + if (this.options.limit || !this.listMetadata.after) { return this.data; } - const results: ResourceType[] = []; + const results = [...this.data]; - for await (const page of this.generatePages({ - before: this.options.before, - after: this.options.after, - })) { + for await (const page of this.generatePages(this.listMetadata.after)) { results.push(...page); } diff --git a/src/feature-flags/feature-flags.spec.ts b/src/feature-flags/feature-flags.spec.ts index bd368d6ba..b810c00f4 100644 --- a/src/feature-flags/feature-flags.spec.ts +++ b/src/feature-flags/feature-flags.spec.ts @@ -353,7 +353,6 @@ describe('FeatureFlags', () => { }, }; fetchOnce(fixtureWithAfter); - fetchOnce(fixtureWithAfter); fetchOnce(listOrganizationFeatureFlagsFixture); const result = await workos.featureFlags.listOrganizationFeatureFlags({ @@ -362,12 +361,12 @@ describe('FeatureFlags', () => { await result.autoPagination(); - const thirdCallUrl = fetch.mock.calls[2][0]; - const thirdCallParams = Object.fromEntries( - new URL(String(thirdCallUrl)).searchParams, + const secondCallUrl = fetch.mock.calls[1][0]; + const secondCallParams = Object.fromEntries( + new URL(String(secondCallUrl)).searchParams, ); - expect(thirdCallParams).not.toHaveProperty('organizationId'); - expect(thirdCallParams).toHaveProperty('after', 'flag_next_page'); + expect(secondCallParams).not.toHaveProperty('organizationId'); + expect(secondCallParams).toHaveProperty('after', 'flag_next_page'); }); }); }); diff --git a/src/pipes/pipes-requests.spec.ts b/src/pipes/pipes-requests.spec.ts index 99a7910c6..1102b8bbc 100644 --- a/src/pipes/pipes-requests.spec.ts +++ b/src/pipes/pipes-requests.spec.ts @@ -42,7 +42,6 @@ beforeEach(() => fetch.resetMocks()); describe('Pipes request contracts', () => { it('preserves the ownership filter across automatic pagination', async () => { - fetchOnce(listDataIntegrationFixture); fetchOnce({ ...listDataIntegrationFixture, list_metadata: { before: null, after: 'next' }, @@ -60,7 +59,6 @@ describe('Pipes request contracts', () => { ), ).toEqual([ { ownership: 'organization', order: 'desc' }, - { ownership: 'organization', order: 'desc', limit: '100' }, { ownership: 'organization', order: 'desc', limit: '100', after: 'next' }, ]); });