Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
📝 WalkthroughWalkthroughThis PR refactors OpenAPI route metadata across multiple API handlers by extracting inline route descriptions into separate, centralized Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 route metadata so it can be shared between the runtime server and spec-generation contexts without pulling in config/env-dependent modules.
Changes:
- Extracted
describeRoutemetadata into dedicated*.routes.tsmodules per handler. - Centralized OpenAPI
documentation(info/servers/tags) intosrc/openapi.tsand reused it in the runtime/openapi.jsonendpoint. - Added a lightweight
createRoutesForSpec()route composer (src/routes.ts) that mounts only metadata + stub handlers for spec generation.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| apps/ensapi/src/routes.ts | Adds spec-only Hono app composition using stub handlers and imported route metadata. |
| apps/ensapi/src/openapi.ts | Centralizes OpenAPI documentation metadata (info/servers/tags). |
| apps/ensapi/src/index.ts | Uses openapiDocumentation for /openapi.json, overriding version and adding localhost server. |
| apps/ensapi/src/handlers/resolution-api.ts | Replaces inline describeRoute objects with imports from resolution-api.routes.ts. |
| apps/ensapi/src/handlers/resolution-api.routes.ts | New module containing OpenAPI metadata + response schemas for resolution endpoints. |
| apps/ensapi/src/handlers/registrar-actions-api.ts | Replaces inline describeRoute objects with imports from registrar-actions-api.routes.ts. |
| apps/ensapi/src/handlers/registrar-actions-api.routes.ts | New module containing OpenAPI metadata for registrar-actions endpoints. |
| apps/ensapi/src/handlers/name-tokens-api.ts | Replaces inline describeRoute objects with import from name-tokens-api.routes.ts. |
| apps/ensapi/src/handlers/name-tokens-api.routes.ts | New module containing OpenAPI metadata + response schemas for name-tokens endpoint. |
| apps/ensapi/src/handlers/ensnode-api.ts | Replaces inline describeRoute objects with imports from ensnode-api.routes.ts. |
| apps/ensapi/src/handlers/ensnode-api.routes.ts | New module containing OpenAPI metadata + response schemas for config/indexing-status endpoints. |
| apps/ensapi/src/handlers/ensanalytics-api.ts | Replaces inline describeRoute objects with imports from ensanalytics-api.routes.ts. |
| apps/ensapi/src/handlers/ensanalytics-api.routes.ts | New module containing OpenAPI metadata for ENSAnalytics v0 endpoints. |
| apps/ensapi/src/handlers/ensanalytics-api-v1.ts | Replaces inline describeRoute objects with imports from ensanalytics-api-v1.routes.ts. |
| apps/ensapi/src/handlers/ensanalytics-api-v1.routes.ts | New module containing OpenAPI metadata for ENSAnalytics v1 endpoints. |
| apps/ensapi/src/handlers/amirealtime-api.ts | Replaces inline describeRoute object with import from amirealtime-api.routes.ts. |
| apps/ensapi/src/handlers/amirealtime-api.routes.ts | New module containing OpenAPI metadata for /amirealtime. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@apps/ensapi/src/handlers/ensanalytics-api-v1.routes.ts`:
- Around line 4-63: The route descriptors getReferralLeaderboardV1Route,
getReferrerDetailV1Route, and getEditionConfigSetRoute currently only list
response descriptions—update each responses object to include content entries
that attach JSON schemas via validationResolver (same pattern used in
name-tokens-api.routes.ts) so the OpenAPI spec exposes response body schemas;
include the appropriate status codes (e.g., 200 success schemas and
400/404/500/503 error schemas where applicable) and reference the existing
response schema validators for leaderboard pages, referrer detail, edition
config set, and shared error shapes.
In `@apps/ensapi/src/handlers/ensnode-api.routes.ts`:
- Line 9: Replace the explicit type annotation on your route exports with a
"satisfies DescribeRouteOptions" clause to preserve literal type inference
(e.g., tag strings); specifically update getConfigRoute (and the other route
export at the second occurrence) to use "satisfies DescribeRouteOptions" instead
of ": DescribeRouteOptions" so the routes match the style used in
amirealtime-api.routes.ts, registrar-actions-api.routes.ts, and
ensanalytics-api-v1.routes.ts.
In `@apps/ensapi/src/handlers/resolution-api.routes.ts`:
- Around line 10-56: Add documented error responses to each route metadata
(getResolveRecordsRoute, getResolvePrimaryNameRoute,
getResolvePrimaryNamesRoute): include common error status entries such as 400
(invalid input), 404 (not found), and 500 (internal error) under the responses
object and reference the appropriate error response schema (e.g., a shared
validationErrorSchema or genericErrorSchema via validationResolver) so the
OpenAPI spec describes possible failures as well as the 200 success case.
In `@apps/ensapi/src/handlers/resolution-api.ts`:
- Around line 21-25: The route descriptors returned by
getResolvePrimaryNameRoute, getResolvePrimaryNamesRoute, and
getResolveRecordsRoute only document a 200 response; update each descriptor to
include standard error responses (e.g., 400, 404, 500, 503) matching the style
used in name-tokens-api.routes.ts so the OpenAPI spec covers middleware
invariant failures and thrown errors; modify the response objects in those
functions to add descriptive schemas/messages for each error status and ensure
any shared error schema is reused if present.
In `@apps/ensapi/src/routes.ts`:
- Line 40: The stub handler currently uses an untyped parameter (const stub =
(c: any) => c.text("")); replace the any with a minimal Hono type: import {
Context } from "hono" and change the signature to use Context (e.g., const stub
= (c: Context) => c.text("") or declare the return as Response/Promise<Response>
if async); update the parameter name if desired and ensure the import for
Context is added so the handler is properly typed.
- Around line 42-85: The route strings in createRoutesForSpec are hard-coded and
can drift from the handler definitions; refactor by exporting path constants
from each handler module (e.g., export const CONFIG_ROUTE = "/api/config" in the
module that defines getConfigRoute, REGISTRAR_ACTIONS_ROUTE for
getRegistrarActionsRoute, AM_I_REALTIME_ROUTE for getAmIRealtimeRoute, etc.) and
import those constants into routes.ts to use in createRoutesForSpec instead of
literal strings, ensuring each handler (getConfigRoute, getIndexingStatusRoute,
getNameTokensRoute, getRegistrarActionsRoute,
getRegistrarActionsByParentNodeRoute, getResolveRecordsRoute,
getResolvePrimaryNameRoute, getResolvePrimaryNamesRoute,
getReferrerLeaderboardRoute, getReferrerDetailRoute,
getReferralLeaderboardV1Route, getReferrerDetailV1Route,
getEditionConfigSetRoute, getAmIRealtimeRoute) exports its path constant so both
the handler and the router share the single source of truth.
| export const getReferralLeaderboardV1Route = { | ||
| tags: ["ENSAwards"], | ||
| summary: "Get Referrer Leaderboard (v1)", | ||
| description: "Returns a paginated page from the referrer leaderboard for a specific edition", | ||
| responses: { | ||
| 200: { | ||
| description: "Successfully retrieved referrer leaderboard page", | ||
| }, | ||
| 404: { | ||
| description: "Unknown edition slug", | ||
| }, | ||
| 500: { | ||
| description: "Internal server error", | ||
| }, | ||
| 503: { | ||
| description: "Service unavailable", | ||
| }, | ||
| }, | ||
| } satisfies DescribeRouteOptions; | ||
|
|
||
| export const getReferrerDetailV1Route = { | ||
| tags: ["ENSAwards"], | ||
| summary: "Get Referrer Detail for Editions (v1)", | ||
| description: `Returns detailed information for a specific referrer for the requested editions. Requires 1-${MAX_EDITIONS_PER_REQUEST} distinct edition slugs. All requested editions must be recognized and have cached data, or the request fails.`, | ||
| responses: { | ||
| 200: { | ||
| description: "Successfully retrieved referrer detail for requested editions", | ||
| }, | ||
| 400: { | ||
| description: "Invalid request", | ||
| }, | ||
| 404: { | ||
| description: "Unknown edition slug", | ||
| }, | ||
| 500: { | ||
| description: "Internal server error", | ||
| }, | ||
| 503: { | ||
| description: "Service unavailable", | ||
| }, | ||
| }, | ||
| } satisfies DescribeRouteOptions; | ||
|
|
||
| export const getEditionConfigSetRoute = { | ||
| tags: ["ENSAwards"], | ||
| summary: "Get Edition Config Set (v1)", | ||
| description: | ||
| "Returns the currently configured referral program edition config set. Editions are sorted in descending order by start timestamp (most recent first).", | ||
| responses: { | ||
| 200: { | ||
| description: "Successfully retrieved edition config set", | ||
| }, | ||
| 500: { | ||
| description: "Internal server error", | ||
| }, | ||
| 503: { | ||
| description: "Service unavailable", | ||
| }, | ||
| }, | ||
| } satisfies DescribeRouteOptions; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Route descriptors lack response content/schema definitions.
These three descriptors define only description for each response status, while name-tokens-api.routes.ts uses validationResolver to attach full JSON schemas. This means the generated OpenAPI spec will show response codes with descriptions but no body schema for these ENSAwards endpoints. Consider adding content entries with validationResolver for consistency and a more useful spec.
🤖 Prompt for AI Agents
In `@apps/ensapi/src/handlers/ensanalytics-api-v1.routes.ts` around lines 4 - 63,
The route descriptors getReferralLeaderboardV1Route, getReferrerDetailV1Route,
and getEditionConfigSetRoute currently only list response descriptions—update
each responses object to include content entries that attach JSON schemas via
validationResolver (same pattern used in name-tokens-api.routes.ts) so the
OpenAPI spec exposes response body schemas; include the appropriate status codes
(e.g., 200 success schemas and 400/404/500/503 error schemas where applicable)
and reference the existing response schema validators for leaderboard pages,
referrer detail, edition config set, and shared error shapes.
There was a problem hiding this comment.
@notrab This is an important feedback from coderabbit. Appreciate your advice. Is this a goal for a future PR?
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| makeIndexingStatusResponseSchema, | ||
| } from "@ensnode/ensnode-sdk/internal"; | ||
|
|
||
| export const getConfigRoute: DescribeRouteOptions = { |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Inconsistent typing: explicit annotation vs. satisfies.
This file uses : DescribeRouteOptions (Lines 9, 25) while amirealtime-api.routes.ts, registrar-actions-api.routes.ts, and ensanalytics-api-v1.routes.ts use satisfies DescribeRouteOptions. The explicit annotation widens the type and loses literal inference (e.g., tag strings), whereas satisfies preserves it. Consider using one approach consistently across all route files.
Also applies to: 25-25
🤖 Prompt for AI Agents
In `@apps/ensapi/src/handlers/ensnode-api.routes.ts` at line 9, Replace the
explicit type annotation on your route exports with a "satisfies
DescribeRouteOptions" clause to preserve literal type inference (e.g., tag
strings); specifically update getConfigRoute (and the other route export at the
second occurrence) to use "satisfies DescribeRouteOptions" instead of ":
DescribeRouteOptions" so the routes match the style used in
amirealtime-api.routes.ts, registrar-actions-api.routes.ts, and
ensanalytics-api-v1.routes.ts.
| export const getResolveRecordsRoute: DescribeRouteOptions = { | ||
| tags: ["Resolution"], | ||
| summary: "Resolve ENS Records", | ||
| description: "Resolves ENS records for a given name", | ||
| responses: { | ||
| 200: { | ||
| description: "Successfully resolved records", | ||
| content: { | ||
| "application/json": { | ||
| schema: validationResolver(makeResolveRecordsResponseSchema()), | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }; | ||
|
|
||
| export const getResolvePrimaryNameRoute: DescribeRouteOptions = { | ||
| tags: ["Resolution"], | ||
| summary: "Resolve Primary Name", | ||
| description: "Resolves a primary name for a given `address` and `chainId`", | ||
| responses: { | ||
| 200: { | ||
| description: "Successfully resolved name", | ||
| content: { | ||
| "application/json": { | ||
| schema: validationResolver(makeResolvePrimaryNameResponseSchema()), | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }; | ||
|
|
||
| export const getResolvePrimaryNamesRoute: DescribeRouteOptions = { | ||
| tags: ["Resolution"], | ||
| summary: "Resolve Primary Names", | ||
| description: "Resolves all primary names for a given address across multiple chains", | ||
| responses: { | ||
| 200: { | ||
| description: "Successfully resolved records", | ||
| content: { | ||
| "application/json": { | ||
| schema: validationResolver(makeResolvePrimaryNamesResponseSchema()), | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Only 200 responses are documented — error responses are missing from the spec.
All three resolution routes only define a 200 response. If these endpoints can return error status codes (e.g., 400 for invalid input, 404, 500), they should be documented here for a complete OpenAPI spec. This may be pre-existing, but since you're centralizing route metadata, now is a good opportunity to add them.
🤖 Prompt for AI Agents
In `@apps/ensapi/src/handlers/resolution-api.routes.ts` around lines 10 - 56, Add
documented error responses to each route metadata (getResolveRecordsRoute,
getResolvePrimaryNameRoute, getResolvePrimaryNamesRoute): include common error
status entries such as 400 (invalid input), 404 (not found), and 500 (internal
error) under the responses object and reference the appropriate error response
schema (e.g., a shared validationErrorSchema or genericErrorSchema via
validationResolver) so the OpenAPI spec describes possible failures as well as
the 200 success case.
| import { | ||
| getResolvePrimaryNameRoute, | ||
| getResolvePrimaryNamesRoute, | ||
| getResolveRecordsRoute, | ||
| } from "./resolution-api.routes"; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Resolution route descriptors only document the 200 response.
The extracted route descriptors for /records, /primary-name, and /primary-names (in resolution-api.routes.ts) only define a 200 response, unlike name-tokens-api.routes.ts which documents 400/404/500/503 cases. Since these handlers can throw or return errors (e.g., middleware invariant failures), consider adding error response entries to the descriptors for a more complete OpenAPI spec — even if this was pre-existing, this refactor is a good opportunity to address it.
🤖 Prompt for AI Agents
In `@apps/ensapi/src/handlers/resolution-api.ts` around lines 21 - 25, The route
descriptors returned by getResolvePrimaryNameRoute, getResolvePrimaryNamesRoute,
and getResolveRecordsRoute only document a 200 response; update each descriptor
to include standard error responses (e.g., 400, 404, 500, 503) matching the
style used in name-tokens-api.routes.ts so the OpenAPI spec covers middleware
invariant failures and thrown errors; modify the response objects in those
functions to add descriptive schemas/messages for each error status and ensure
any shared error schema is reused if present.
| } from "./handlers/resolution-api.routes"; | ||
|
|
||
| // Stub handler used for all routes — generateSpecs only reads describeRoute metadata | ||
| const stub = (c: any) => c.text(""); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Avoid any — use a minimal typed signature for the stub handler.
Proposed fix
-const stub = (c: any) => c.text("");
+const stub = (c: { text: (s: string) => Response }) => c.text("");Alternatively, import Context from hono and type it as (c: Context) => Response.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const stub = (c: any) => c.text(""); | |
| const stub = (c: { text: (s: string) => Response }) => c.text(""); |
🤖 Prompt for AI Agents
In `@apps/ensapi/src/routes.ts` at line 40, The stub handler currently uses an
untyped parameter (const stub = (c: any) => c.text("")); replace the any with a
minimal Hono type: import { Context } from "hono" and change the signature to
use Context (e.g., const stub = (c: Context) => c.text("") or declare the return
as Response/Promise<Response> if async); update the parameter name if desired
and ensure the import for Context is added so the handler is properly typed.
| export function createRoutesForSpec() { | ||
| const app = new Hono(); | ||
|
|
||
| // /api routes (ensnode-api) | ||
| app.get("/api/config", describeRoute(getConfigRoute), stub); | ||
| app.get("/api/indexing-status", describeRoute(getIndexingStatusRoute), stub); | ||
|
|
||
| // /api/name-tokens routes | ||
| app.get("/api/name-tokens", describeRoute(getNameTokensRoute), stub); | ||
|
|
||
| // /api/registrar-actions routes | ||
| app.get("/api/registrar-actions", describeRoute(getRegistrarActionsRoute), stub); | ||
| app.get( | ||
| "/api/registrar-actions/:parentNode", | ||
| describeRoute(getRegistrarActionsByParentNodeRoute), | ||
| stub, | ||
| ); | ||
|
|
||
| // /api/resolve routes (resolution-api) | ||
| app.get("/api/resolve/records/:name", describeRoute(getResolveRecordsRoute), stub); | ||
| app.get( | ||
| "/api/resolve/primary-name/:address/:chainId", | ||
| describeRoute(getResolvePrimaryNameRoute), | ||
| stub, | ||
| ); | ||
| app.get("/api/resolve/primary-names/:address", describeRoute(getResolvePrimaryNamesRoute), stub); | ||
|
|
||
| // /ensanalytics routes | ||
| app.get("/ensanalytics/referrers", describeRoute(getReferrerLeaderboardRoute), stub); | ||
| app.get("/ensanalytics/referrers/:referrer", describeRoute(getReferrerDetailRoute), stub); | ||
|
|
||
| // /v1/ensanalytics routes | ||
| app.get( | ||
| "/v1/ensanalytics/referral-leaderboard", | ||
| describeRoute(getReferralLeaderboardV1Route), | ||
| stub, | ||
| ); | ||
| app.get("/v1/ensanalytics/referrer/:referrer", describeRoute(getReferrerDetailV1Route), stub); | ||
| app.get("/v1/ensanalytics/editions", describeRoute(getEditionConfigSetRoute), stub); | ||
|
|
||
| // /amirealtime routes | ||
| app.get("/amirealtime", describeRoute(getAmIRealtimeRoute), stub); | ||
|
|
||
| return app; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the index.ts file to see how routes are mounted
cat -n apps/ensapi/src/index.tsRepository: namehash/ensnode
Length of output: 6466
🏁 Script executed:
# Let's also check the handler files to understand the relative paths
find apps/ensapi/src -type f -name "*.ts" | grep -E "(handler|route)" | head -20Repository: namehash/ensnode
Length of output: 1017
🏁 Script executed:
# Check the structure of the ensapi/src directory
tree -L 2 apps/ensapi/src/ 2>/dev/null || find apps/ensapi/src -type f -name "*.ts" | sed 's|[^/]*/| |g' | sortRepository: namehash/ensnode
Length of output: 2053
🏁 Script executed:
# Check ensnode-api.routes.ts to see the relative paths
cat -n apps/ensapi/src/handlers/ensnode-api.routes.tsRepository: namehash/ensnode
Length of output: 1728
🏁 Script executed:
# Check ensanalytics-api.routes.ts
cat -n apps/ensapi/src/handlers/ensanalytics-api.routes.tsRepository: namehash/ensnode
Length of output: 1205
🏁 Script executed:
# Check amirealtime-api.ts (main file, not routes)
cat -n apps/ensapi/src/handlers/amirealtime-api.ts | head -100Repository: namehash/ensnode
Length of output: 3252
🏁 Script executed:
# Check routes.ts to see the full file
cat -n apps/ensapi/src/routes.tsRepository: namehash/ensnode
Length of output: 3809
🏁 Script executed:
# Check the actual route definitions in ensnode-api.ts
cat -n apps/ensapi/src/handlers/ensnode-api.ts | head -80Repository: namehash/ensnode
Length of output: 2525
🏁 Script executed:
# Check ensanalytics-api.ts
cat -n apps/ensapi/src/handlers/ensanalytics-api.ts | head -100Repository: namehash/ensnode
Length of output: 4356
🏁 Script executed:
# Check registrar-actions-api.ts
cat -n apps/ensapi/src/handlers/registrar-actions-api.ts | head -80Repository: namehash/ensnode
Length of output: 3136
🏁 Script executed:
# Check if there are any tests that verify route synchronization
find apps/ensapi -name "*.test.ts" -o -name "*.spec.ts" | xargs grep -l "route" 2>/dev/null | head -5Repository: namehash/ensnode
Length of output: 108
🏁 Script executed:
# Let me check ensanalytics-api.ts for the referrer detail route
cat -n apps/ensapi/src/handlers/ensanalytics-api.ts | sed -n '100,150p'Repository: namehash/ensnode
Length of output: 2127
🏁 Script executed:
# Check the openapi.ts file to understand how routes.ts is used
cat -n apps/ensapi/src/openapi.tsRepository: namehash/ensnode
Length of output: 1541
🏁 Script executed:
# Check name-tokens-api.ts for actual routes
cat -n apps/ensapi/src/handlers/name-tokens-api.tsRepository: namehash/ensnode
Length of output: 7330
🏁 Script executed:
# Check resolution-api.ts for actual routes
cat -n apps/ensapi/src/handlers/resolution-api.tsRepository: namehash/ensnode
Length of output: 7354
🏁 Script executed:
# Check ensanalytics-api-v1.ts
cat -n apps/ensapi/src/handlers/ensanalytics-api-v1.tsRepository: namehash/ensnode
Length of output: 16290
🏁 Script executed:
# Check registrar-actions-api.ts for the parent node route
cat -n apps/ensapi/src/handlers/registrar-actions-api.ts | sed -n '80,150p'Repository: namehash/ensnode
Length of output: 2448
🏁 Script executed:
# Check registrar-actions-api.ts for the parent node route definition
cat -n apps/ensapi/src/handlers/registrar-actions-api.ts | grep -A 20 "parentNode"Repository: namehash/ensnode
Length of output: 3883
🏁 Script executed:
# Final check: verify if there are any integration or end-to-end tests that validate route synchronization
find apps/ensapi -name "*.test.ts" -o -name "*.spec.ts" | xargs grep -l "route\|path" 2>/dev/nullRepository: namehash/ensnode
Length of output: 108
🏁 Script executed:
# Check the test file to see if it validates routes
cat -n apps/ensapi/src/handlers/amirealtime-api.test.tsRepository: namehash/ensnode
Length of output: 9718
Route paths are duplicated across three locations—consider exporting path constants from handlers.
Every route path in routes.ts (e.g., /api/config, /amirealtime) is manually constructed by combining the mount points from index.ts with the relative routes defined in handler files. There's no compile-time or runtime check ensuring these stay synchronized. If a path changes in a handler or mount point in index.ts without updating routes.ts, the OpenAPI spec will silently diverge from the actual API.
Consider exporting path constants from handler modules (e.g., export const REGISTRAR_ACTIONS_ROUTE = "/registrar-actions") and consuming them in both the handler files and routes.ts, or adding a build-time or runtime validation to enforce the sync.
🤖 Prompt for AI Agents
In `@apps/ensapi/src/routes.ts` around lines 42 - 85, The route strings in
createRoutesForSpec are hard-coded and can drift from the handler definitions;
refactor by exporting path constants from each handler module (e.g., export
const CONFIG_ROUTE = "/api/config" in the module that defines getConfigRoute,
REGISTRAR_ACTIONS_ROUTE for getRegistrarActionsRoute, AM_I_REALTIME_ROUTE for
getAmIRealtimeRoute, etc.) and import those constants into routes.ts to use in
createRoutesForSpec instead of literal strings, ensuring each handler
(getConfigRoute, getIndexingStatusRoute, getNameTokensRoute,
getRegistrarActionsRoute, getRegistrarActionsByParentNodeRoute,
getResolveRecordsRoute, getResolvePrimaryNameRoute, getResolvePrimaryNamesRoute,
getReferrerLeaderboardRoute, getReferrerDetailRoute,
getReferralLeaderboardV1Route, getReferrerDetailV1Route,
getEditionConfigSetRoute, getAmIRealtimeRoute) exports its path constant so both
the handler and the router share the single source of truth.
lightwalker-eth
left a comment
There was a problem hiding this comment.
@notrab Sharing some preliminary feedback 😄
| @@ -0,0 +1,18 @@ | |||
| import type { DescribeRouteOptions } from "hono-openapi"; | |||
There was a problem hiding this comment.
Maybe instead of calling each of these files a ".routes.ts" file we call it something like ".meta.ts" since I understand each of these is metadata about the route, not the route implementation itself.
| @@ -0,0 +1,18 @@ | |||
| import type { DescribeRouteOptions } from "hono-openapi"; | |||
|
|
|||
| export const getAmIRealtimeRoute = { | |||
There was a problem hiding this comment.
| export const getAmIRealtimeRoute = { | |
| export const amIRealtimeGetMeta = { |
Goals:
- When
get...is the prefix it sounds like it's a getter function, when this is a const value rather than a function. - Appreciate how there's a desire to identify which HTTP action this metadata is for. Therefore suggested moving "...Get..." to later in the naming pattern.
- Renamed "Route" to "Meta" because this isn't the route itself, it's just metadata about the route.
If this makes sense please apply these patterns across all the updates in this PR 👍
| export const getReferralLeaderboardV1Route = { | ||
| tags: ["ENSAwards"], | ||
| summary: "Get Referrer Leaderboard (v1)", | ||
| description: "Returns a paginated page from the referrer leaderboard for a specific edition", | ||
| responses: { | ||
| 200: { | ||
| description: "Successfully retrieved referrer leaderboard page", | ||
| }, | ||
| 404: { | ||
| description: "Unknown edition slug", | ||
| }, | ||
| 500: { | ||
| description: "Internal server error", | ||
| }, | ||
| 503: { | ||
| description: "Service unavailable", | ||
| }, | ||
| }, | ||
| } satisfies DescribeRouteOptions; | ||
|
|
||
| export const getReferrerDetailV1Route = { | ||
| tags: ["ENSAwards"], | ||
| summary: "Get Referrer Detail for Editions (v1)", | ||
| description: `Returns detailed information for a specific referrer for the requested editions. Requires 1-${MAX_EDITIONS_PER_REQUEST} distinct edition slugs. All requested editions must be recognized and have cached data, or the request fails.`, | ||
| responses: { | ||
| 200: { | ||
| description: "Successfully retrieved referrer detail for requested editions", | ||
| }, | ||
| 400: { | ||
| description: "Invalid request", | ||
| }, | ||
| 404: { | ||
| description: "Unknown edition slug", | ||
| }, | ||
| 500: { | ||
| description: "Internal server error", | ||
| }, | ||
| 503: { | ||
| description: "Service unavailable", | ||
| }, | ||
| }, | ||
| } satisfies DescribeRouteOptions; | ||
|
|
||
| export const getEditionConfigSetRoute = { | ||
| tags: ["ENSAwards"], | ||
| summary: "Get Edition Config Set (v1)", | ||
| description: | ||
| "Returns the currently configured referral program edition config set. Editions are sorted in descending order by start timestamp (most recent first).", | ||
| responses: { | ||
| 200: { | ||
| description: "Successfully retrieved edition config set", | ||
| }, | ||
| 500: { | ||
| description: "Internal server error", | ||
| }, | ||
| 503: { | ||
| description: "Service unavailable", | ||
| }, | ||
| }, | ||
| } satisfies DescribeRouteOptions; |
There was a problem hiding this comment.
@notrab This is an important feedback from coderabbit. Appreciate your advice. Is this a goal for a future PR?
| }, | ||
| }), | ||
| describeRoute(getReferralLeaderboardV1Route), | ||
| validate("query", referrerLeaderboardPageQuerySchema), |
There was a problem hiding this comment.
Perhaps we should move things like referrerLeaderboardPageQuerySchema into these new separate metadata files? Appreciate your advice.
Lite PR
Tip: Review docs on the ENSNode PR process
Summary
describeRoutedefinitions from handler files into sibling.routes.tsfiles with zero config/env dependencies.openapi.ts(documentation config) androutes.ts(lightweight route composition for spec generation).scripts/generate-openapi.tsandpnpm generate:openapiscript to generate the OpenAPI spec without booting the server.Why
The OpenAPI spec was only available at runtime via
GET /openapi.json, which requires a fully configured and running server. We need to generate it in CI and for docs/SDK generation without env vars or network access.Testing
pnpm typecheckpasses with noerrors.pnpm testpasses (63/63 tests)pnpm generate:openapiproduces valid OpenAPI 3.1.0 JSON with all 14 paths. (not committed file, will in another PR)Notes for Reviewer (Optional)
The runtime server (
index.ts) still mounts the real handler files directly.routes.tsandcreateRoutesForSpec()are only used by the generate script. Handler files now import their route descriptions from the.routes.tssiblings.No changes to runtime behaviour.
Pre-Review Checklist (Blocking)