refactor: use zod-openapi to serve openapi.json route#1687
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThis PR transitions the ENS API from runtime-generated OpenAPI schemas to pre-generated static documents. It removes OpenAPI validation dependencies, adds operationId metadata and response schemas across route handlers, deletes the custom validation middleware, and introduces a new document generation function while maintaining existing route functionality. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Pull request overview
Refactors ENSApi’s OpenAPI spec endpoint to generate and serve openapi.json directly from the existing @hono/zod-openapi route definitions, completing the migration away from hono-openapi and removing now-dead validation glue.
Changes:
- Serve
/openapi.jsonviaOpenAPIHono#getOpenAPI31Document()using existing stub route registration plus sharedopenapiMeta. - Centralize OpenAPI metadata in
openapi-meta.ts, including the dynamicpackage.jsonversion. - Remove
hono-openapi,@hono/standard-validator, and the unusedvalidate.tshandler.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Drops hono-openapi and related dependency graph from the lockfile. |
| apps/ensapi/src/openapi-meta.ts | Adds packageJson.version to OpenAPI metadata (and keeps servers/tags in one place). |
| apps/ensapi/src/lib/handlers/validate.ts | Removes unused validation middleware that depended on hono-openapi. |
| apps/ensapi/src/index.ts | Replaces openAPIRouteHandler with getOpenAPI31Document() generated from stub-registered zod-openapi routes; appends localhost server dynamically. |
| apps/ensapi/package.json | Removes unused hono-openapi and @hono/standard-validator dependencies. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Greptile SummaryCompleted the migration from
All changes are consistent with the established migration pattern from recent PRs (#1673, #1686, #1685, #1684, #1672, #1661). The refactoring is clean, well-tested (type-checks pass), and maintains the same functionality while consolidating on a single OpenAPI generation system. Confidence Score: 5/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[GET /openapi.json Request] --> B[createStubRoutesForSpec]
B --> C[Create OpenAPIHono App]
C --> D[Register All Routes with Stub Handlers]
D --> E[amIRealtimeRoutes]
D --> F[ensnodeRoutes]
D --> G[ensanalyticsRoutes]
D --> H[nameTokensRoutes]
D --> I[registrarActionsRoutes]
D --> J[resolutionRoutes]
E & F & G & H & I & J --> K[stubApp.getOpenAPI31Document]
K --> L[Merge openapiMeta + localhost server]
L --> M[Return OpenAPI 3.1 JSON]
Last reviewed commit: e3f8a94 |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 12 changed files in this pull request and generated 5 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/ensapi/src/handlers/name-tokens-api.routes.ts`:
- Line 34: The operationId "getApiName-tokens" in name-tokens-api.routes.ts
should follow the project's canonical operationId convention rather than being
changed in isolation; pick the standard pattern (either the existing
hyphen/colon style or an alphanumeric-only style) and apply it consistently
across all route handler files (update the operationId strings in this file and
the other route handlers), then regenerate any codegen/artifacts and update
tests/docs to match the chosen convention; specifically ensure the operationId
value "getApiName-tokens" is aligned with the decided pattern across the
codebase.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
apps/ensapi/src/handlers/amirealtime-api.routes.tsapps/ensapi/src/handlers/ensanalytics-api-v1.routes.tsapps/ensapi/src/handlers/ensanalytics-api.routes.tsapps/ensapi/src/handlers/ensnode-api.routes.tsapps/ensapi/src/handlers/name-tokens-api.routes.tsapps/ensapi/src/handlers/registrar-actions-api.routes.tsapps/ensapi/src/handlers/resolution-api.routes.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/ensapi/src/handlers/resolution-api.routes.ts`:
- Line 17: The operationId values use inconsistent punctuation and casing which
can break code generators; update the operationIds to a consistent camelCase
pattern (no hyphens or colons). Replace getApiResolveRecords:name with something
like getApiResolveRecordsByName, getApiResolvePrimary-name:address:chainId with
getApiResolvePrimaryByNameAddressChainId, and getApiResolvePrimary-names:address
with getApiResolvePrimaryByNamesAddress (apply same convention at the other
occurrences referenced in the comment).
In `@packages/ensnode-sdk/src/ensapi/api/resolution/zod-schemas.ts`:
- Around line 47-49: The JSON Schema for the names field is invalid because
propertyNames: { type: "number" } is incorrect; update the withOpenApi call for
the names schema (the names variable using withOpenApi and z.record) to replace
propertyNames: { type: "number" } with a pattern-based constraint that matches
numeric-looking keys (for example propertyNames: { pattern: "^[0-9]+$" } or
another regex you need) so propertyNames validates the string property names
correctly instead of using type:"number".
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
apps/ensapi/src/handlers/resolution-api.routes.tspackages/ensnode-sdk/src/ensapi/api/resolution/zod-schemas.tspackages/ensnode-sdk/src/indexing-status/zod-schema/omnichain-indexing-status-snapshot.tspackages/ensnode-sdk/src/shared/zod-types.ts
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 13 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 13 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
lightwalker-eth
left a comment
There was a problem hiding this comment.
@notrab Really happy how this is advancing 😄 Looks good!
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 13 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "@ensnode/ponder-subgraph": "workspace:*", | ||
| "@hono/node-server": "^1.19.5", | ||
| "@hono/otel": "^0.2.2", | ||
| "@hono/standard-validator": "^0.2.2", | ||
| "@hono/zod-openapi": "^1.2.2", | ||
| "@namehash/ens-referrals": "workspace:*", |
There was a problem hiding this comment.
pnpm-lock.yaml still lists @hono/standard-validator and hono-openapi under the apps/ensapi importer (so a CI run with --frozen-lockfile will fail). Please regenerate and commit the lockfile after removing these deps from apps/ensapi/package.json.
Lite PR
Tip: Review docs on the ENSNode PR process
Summary
hono-openapi'sopenAPIRouteHandlerwith@hono/zod-openapi'sgetOpenAPI31Document()for serving the/openapi.jsonroute, using the existing stub routes andopenapiMeta.hono-openapiand@hono/standard-validatordependenciesvalidate.tsfile (was the only consumer ofhono-openapi, not imported anywhere).Why
zod-openapifor the ensapi app. All API routes have already been migrated to use@hono/zod-openapi'screateRoute, so the spec endpoint should use the same system rather than relying on a separatehono-openapilibrary.Testing
tsc --noEmit— passes cleanly.hono-openapiand@hono/standard-validatorhave no remaining imports in the source.Notes for Reviewer (Optional)
openapi-meta.tswas updated to use the dynamicpackageJson.versioninstead of the previously hardcoded"0.0.0".index.tssince it depends on config (whichopenapi-meta.tsintentionally avoids importing to keep stub route generation config-free).Pre-Review Checklist (Blocking)