Skip to content

fix(typescript): await getNextPage() in generated pagination snippet - #17563

Open
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1787932321-ts-readme-pagination-await
Open

fix(typescript): await getNextPage() in generated pagination snippet#17563
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1787932321-ts-readme-pagination-await

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Description

Linear ticket: Refs

The manual page-by-page pagination example emitted into generated TypeScript SDK README.md / reference.md / snippet.json did not type-check (reproduced on generator 3.88.3):

let page = await client.item.listActivity();
while (page.hasNextPage()) {
    page = page.getNextPage(); // TS2740: Type 'Promise<Page<...>>' is missing ... from type 'Page<...>'
}

Root cause: the snippet is not a text template — it is built as a TypeScript AST in maybeLeverageInvocation in GeneratedDefaultEndpointImplementation.ts, which created the assignment RHS as a plain createCallExpression for page.getNextPage(). The pagination runtime (Page.getNextPage() and CustomPager.getNextPage()) is async and returns Promise<this>, so assigning its result back into the Page-typed page variable is a type error. The neighbouring for await (const item of pageableResponse) snippet is generated separately and was always correct, which is why only the manual loop was broken. The fix wraps the call in ts.factory.createAwaitExpression, emitting page = await page.getNextPage(); (the enclosing snippet is already an async context, since the initial await client...() call is emitted there too).

Changes Made

  • Emit page = await page.getNextPage(); from the pagination leverage-code AST in GeneratedDefaultEndpointImplementation.maybeLeverageInvocation.
  • Updated the client-class-generator snapshot for the leverage-code test.
  • Regenerated 83 ts-sdk seed outputs (README, reference, snippet.json) across all pagination fixtures.
  • Added an unreleased TypeScript SDK changelog entry.
  • Updated README.md generator (if applicable)

Audit of sibling snippets / other languages

  • Binary-response snippets check out against BinaryResponse: the active stream() assignment is synchronous, and the arrayBuffer() / blob() / bytes() alternatives are shown as awaited comments.
  • The async-iterator pagination snippet is correct as-is.
  • Other generators do not have the analogous defect: Java's SyncPage.nextPage() returns SyncPage<T> synchronously; Go's example already uses page, err = page.GetNextPage(ctx) with error handling; Python (iter_pages()), PHP (getPages()), C#, Rust and Swift pagination sections don't use this direct-async-assignment shape.

Testing

  • Unit tests added/updated — client-class-generator (612 tests) and TS generator (186 tests) suites pass; leverage-code snapshot now asserts the awaited form.
  • Manual testing completed — pnpm compile, pnpm seed:build, pnpm lint:biome, pnpm format:fix clean; pnpm seed test --generator ts-sdk --skip-scripts 251/251 passing; no page = page.getNextPage(); remains anywhere under seed/.

Gap: there is no existing test that extracts fenced TypeScript blocks from a generated README.md/reference.md and runs tsc over them. Seed's TS project verification (.fern/verify.shpnpm build && pnpm test) compiles the generated SDK and dynamic-snippet tests, but never the documentation snippets, so this class of bug (README-only, non-compiling example code) is currently uncaught by CI. The snapshot test added here pins the exact generated text, but a real README compile check would need new infrastructure.

Link to Devin session: https://app.devin.ai/sessions/79649fbe4aa54feb8b07597bbf1de100
Open in Devin Desktop: https://app.devin.ai/desktop/session/79649fbe4aa54feb8b07597bbf1de100?variant=devin


Devin Review

…on snippet

Co-Authored-By: bot_apk <apk@cognition.ai>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@nitpickybot nitpickybot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Single-line generator fix wrapping page.getNextPage() in an await expression, plus regenerated seed fixtures and a changelog entry. The AST change is correct and the enclosing snippet is already async (it opens with await client...). No issues found.


To request another review, comment /ai-review on this pull request.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Devin Review

@github-actions

Copy link
Copy Markdown
Contributor

SDK Generation Benchmark Results

Comparing PR branch against median of 5 nightly run(s) on main (latest: 2026-08-28T06:31:59Z).

Full benchmark table (click to expand)
Generator Spec main (generator) main (E2E) PR (generator) Delta
ts-sdk square 160s (n=5) 165s (n=5) 150s -10s (-6.2%)

main (generator): generator-only time via --skip-scripts (includes Docker image build, container startup, IR parsing, and code generation — this is the same Docker-based flow customers use via fern generate). main (E2E): full customer-observable time including build/test scripts (nightly baseline, informational). Delta is computed against generator-only baseline.
⚠️ = generation exited with a non-zero exit code (timing may not reflect a successful run).
Baseline from nightly runs on main (latest: 2026-08-28T06:31:59Z). Trigger benchmark-baseline to refresh.
Last updated: 2026-08-28 16:31 UTC

@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

Review — no breaking changes found.

  • getNextPage() is public async getNextPage(): Promise<this> in both Page.ts and CustomPager.ts, so the previous snippet page = page.getNextPage() assigned a Promise to a Page variable and did not type-check. The emitted snippet is documentation/reference only, so this changes no generated SDK runtime code and no public API.
  • The enclosing generated example is already async, so await is valid there.
  • Snapshot and the 83 regenerated seed README/reference/snippet.json outputs are consistent, and no page = page.getNextPage(); remains under seed/.
  • Changelog present under generators/typescript/sdk/changes/unreleased/, type fix.

Approving.

Written by Devin

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants