Skip to content

fix: revert OpenAPIHono generics from routes/index.ts (OOM)#2270

Closed
andrew-bierman wants to merge 2 commits into
developmentfrom
fix/routes-index-generics-oom
Closed

fix: revert OpenAPIHono generics from routes/index.ts (OOM)#2270
andrew-bierman wants to merge 2 commits into
developmentfrom
fix/routes-index-generics-oom

Conversation

@andrew-bierman
Copy link
Copy Markdown
Collaborator

Summary

  • Removes the <{ Bindings: Env; Variables: Variables }> generics that were added to the three OpenAPIHono instances in routes/index.ts in ✨ add Hono RPC foundation #2268

Adding those generics causes TypeScript to exhaust ~8 GB of heap and OOM. This is the same instantiation-depth explosion that motivated the separate rpcRoutes pattern — the runtime router's type is never exported as AppType, so the generics serve no purpose there and should not be present.

Test plan

  • bun run check-types in packages/api completes without OOM

Adding <Bindings/Variables> to the three OpenAPIHono instances in the
runtime router causes TypeScript to OOM (~8 GB heap) — the same
instantiation-depth issue that motivated the separate rpcRoutes pattern.
The runtime router doesn't need explicit generics since its type is not
exported as AppType.

https://claude.ai/code/session_01JtEqZjLSh3kkycRSrrF3f5
Copilot AI review requested due to automatic review settings April 22, 2026 12:29
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 22, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 87114971-f400-48c5-96b3-c50e580910fe

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/routes-index-generics-oom

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added the api label Apr 22, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 22, 2026

Coverage Report for Expo Unit Tests Coverage (./apps/expo)

Status Category Percentage Covered / Total
🔵 Lines 79.9% 517 / 647
🔵 Statements 79.9% (🎯 75%) 517 / 647
🔵 Functions 92.85% 52 / 56
🔵 Branches 92.55% 199 / 215
File CoverageNo changed files found.
Generated in workflow #644 for commit 6fffb8d by the Vitest Coverage Report Action

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 22, 2026

Coverage Report for API Unit Tests Coverage (./packages/api)

Status Category Percentage Covered / Total
🔵 Lines 85.86% 905 / 1054
🔵 Statements 85.86% (🎯 80%) 905 / 1054
🔵 Functions 94.11% 48 / 51
🔵 Branches 88.92% 281 / 316
File CoverageNo changed files found.
Generated in workflow #644 for commit 6fffb8d by the Vitest Coverage Report Action

@cloudflare-workers-and-pages
Copy link
Copy Markdown
Contributor

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
❌ Deployment failed
View logs
packrat-admin a2ef411 Apr 22 2026, 12:30 PM

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR reverts explicit OpenAPIHono<{ Bindings: Env; Variables: Variables }> generics in the top-level route composition to avoid TypeScript instantiation-depth/heap blowups that lead to OOM during type-checking, aligning with the existing rpcRoutes pattern used for hc<AppType> inference.

Changes:

  • Remove Env / Variables type imports and drop OpenAPIHono generic parameters in packages/api/src/routes/index.ts.
  • Simplify publicRoutes and routes construction while keeping the same routing structure.
Comments suppressed due to low confidence (1)

packages/api/src/routes/index.ts:45

  • The JSDoc here says this routes type is used by hc<AppRoutes>(), but the repo’s api client currently uses hc<AppType> exported from packages/api/src/index.ts. Please update/remove this comment (and consider dropping the AppRoutes export if it’s no longer used) to avoid confusion and reduce TS type surface area.
const routes = $(new OpenAPIHono().route('/', publicRoutes).route('/', protectedRoutes));

export { routes };

/** Full type of the PackRat Hono app — used by `hc<AppRoutes>()` in api-client. */

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

.route('/auth', authRoutes)
.route('/admin', adminRoutes),
);
const publicRoutes = $(new OpenAPIHono().route('/auth', authRoutes).route('/admin', adminRoutes));
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

This line is >100 chars (repo Biome lineWidth is 100) and will likely fail biome check formatting. Please reformat back to a multi-line chain (or run biome format) to keep it within the formatter’s wrapping rules.

Suggested change
const publicRoutes = $(new OpenAPIHono().route('/auth', authRoutes).route('/admin', adminRoutes));
const publicRoutes = $(
new OpenAPIHono()
.route('/auth', authRoutes)
.route('/admin', adminRoutes),
);

Copilot uses AI. Check for mistakes.
.route('/', publicRoutes)
.route('/', protectedRoutes),
);
const routes = $(new OpenAPIHono().route('/', publicRoutes).route('/', protectedRoutes));
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

This chained call is also over the 100-char formatter line width and is likely to trigger a formatting failure in biome check. Reformat to the multi-line style used elsewhere in this file (or run biome format).

Suggested change
const routes = $(new OpenAPIHono().route('/', publicRoutes).route('/', protectedRoutes));
const routes = $(
new OpenAPIHono().route('/', publicRoutes).route('/', protectedRoutes),
);

Copilot uses AI. Check for mistakes.
AppRoutes was superseded by AppType in packages/api/src/index.ts.
Lines kept single-line as Biome prefers (both fit within 100 chars).

https://claude.ai/code/session_01JtEqZjLSh3kkycRSrrF3f5
@andrew-bierman andrew-bierman deleted the fix/routes-index-generics-oom branch April 24, 2026 23:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants