Merge ENSApi /api/config endpoint into /api/indexing-status endpoint#1671
Merge ENSApi /api/config endpoint into /api/indexing-status endpoint#1671
/api/config endpoint into /api/indexing-status endpoint#1671Conversation
Include `config` field of `EnsApiPublicConfig` type.
Merge `/api/config` endpoint into `/api/indexing-status`. The former endpoint got deleted, while the later includes the `config: EnsApiPublicConfig` object in the response.
Use `EnsIndexerClient` to fetch indexing status.
New names will match `EnsApiProvider*` pattern.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR consolidates ENS API configuration into the indexing status response and renames ENSNode-related APIs to EnsApi throughout the codebase. A new Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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
Consolidates ENSApi metadata fetching by embedding the former /api/config payload into the /api/indexing-status response, and propagates the updated data model through the ENSNode SDK, @ensnode/ensnode-react, and ENSAdmin consumers.
Changes:
- ENSApi
/indexing-statusnow returns{ realtimeProjection, config }and the standalone/configroute is removed. - ENSNode SDK updates indexing-status response types + (de)serialization + Zod schemas to include
config. - ENSNode React + ENSAdmin update provider/options naming and migrate config consumption to come from indexing-status.
Reviewed changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/namehash-ui/src/components/identity/ResolveAndDisplayIdentity.tsx | Passes namespace explicitly into useResolvedIdentity. |
| packages/ensnode-sdk/src/ensapi/config/deserialize.ts | Exposes config unvalidated builder for reuse by indexing-status deserializer. |
| packages/ensnode-sdk/src/ensapi/api/indexing-status/zod-schemas.ts | Adds config to OK schema + serialized OK schema. |
| packages/ensnode-sdk/src/ensapi/api/indexing-status/serialized-response.ts | Adds serialized config to serialized OK response type. |
| packages/ensnode-sdk/src/ensapi/api/indexing-status/serialize.ts | Serializes config into indexing-status OK responses. |
| packages/ensnode-sdk/src/ensapi/api/indexing-status/response.ts | Adds config: EnsApiPublicConfig to indexing-status OK response type. |
| packages/ensnode-sdk/src/ensapi/api/indexing-status/deserialize.ts | Deserializes config from the indexing-status response. |
| packages/ensnode-react/src/utils/query.ts | Switches to EnsApiClient + removes separate config query helpers/keys. |
| packages/ensnode-react/src/types.ts | Renames provider config types to EnsApi-oriented types and adds namespace param to resolved-identity hook params. |
| packages/ensnode-react/src/provider.tsx | Renames provider to EnsApiProvider and helper to createEnsApiOptions. |
| packages/ensnode-react/src/hooks/useResolvedIdentity.ts | Stops deriving namespace from config fetch; accepts namespace directly. |
| packages/ensnode-react/src/hooks/useRegistrarActions.ts | Migrates hooks to use provider “options” via useEnsApiProviderOptions. |
| packages/ensnode-react/src/hooks/useRecords.ts | Migrates hooks to use provider “options” via useEnsApiProviderOptions. |
| packages/ensnode-react/src/hooks/usePrimaryNames.ts | Migrates hooks to use provider “options” via useEnsApiProviderOptions. |
| packages/ensnode-react/src/hooks/usePrimaryName.ts | Migrates hooks to use provider “options” via useEnsApiProviderOptions. |
| packages/ensnode-react/src/hooks/useNameTokens.ts | Migrates hooks to use provider “options” via useEnsApiProviderOptions. |
| packages/ensnode-react/src/hooks/useIndexingStatus.ts | Updates request/response types to EnsApi-prefixed indexing-status types. |
| packages/ensnode-react/src/hooks/useEnsApiProviderOptions.ts | New hook replacing useENSNodeSDKConfig for resolving provider options. |
| packages/ensnode-react/src/hooks/useENSNodeSDKConfig.ts | Removed legacy config hook. |
| packages/ensnode-react/src/hooks/useENSNodeConfig.ts | Removed legacy config-fetch hook. |
| packages/ensnode-react/src/hooks/index.ts | Updates public hook exports to new options hook. |
| packages/ensnode-react/src/context.ts | Renames context to EnsApiContext and updates types. |
| packages/ensnode-react/README.md | Updates docs/examples for new provider/options naming (incomplete). |
| apps/ensapi/src/handlers/ensnode-api.ts | Removes /config endpoint and injects config into /indexing-status responses. |
| apps/ensapi/src/cache/indexing-status.cache.ts | Updates ENSApi’s ENSIndexer client usage/type names. |
| apps/ensadmin/src/hooks/async/use-namespace.ts | Switches namespace derivation to new ENSApi config hook. |
| apps/ensadmin/src/hooks/active/use-active-connection.tsx | Uses new ENSApi config hook for active connection invariant. |
| apps/ensadmin/src/components/registrar-actions/use-stateful-fetch-registrar-actions.ts | Uses indexing-status’ embedded config to gate registrar actions support. |
| apps/ensadmin/src/components/providers/selected-ensnode-provider.tsx | Migrates selected connection provider to EnsApiProvider + createEnsApiOptions. |
| apps/ensadmin/src/components/layout-wrapper.tsx | Updates layout to use renamed selected provider component. |
| apps/ensadmin/src/components/indexing-status/use-indexing-status-with-swr.ts | SWR caching updated to cache both snapshot + config and reproject over time. |
| apps/ensadmin/src/components/connections/require-active-connection.tsx | Uses ENSApi config hook and updates user-facing error copy. |
| apps/ensadmin/src/components/connection/cards/ensnode-info.tsx | Migrates config display to new ENSApi config hook and updated types. |
| apps/ensadmin/src/components/config/useEnsApiConfig.ts | New hook to expose config from indexing-status-derived data. |
Comments suppressed due to low confidence (2)
packages/ensnode-sdk/src/ensapi/api/indexing-status/zod-schemas.ts:17
- The type imports from
./responseand./serialized-responseare unused in this module (they're only referenced in JSDoc). With Biome's recommended rules enabled, this will fail lint due to unused imports; remove these imports or use them in code (e.g., viasatisfieswhere appropriate).
apps/ensapi/src/handlers/ensnode-api.ts:29 - This route now returns both indexing status and the ENSApi public config (via the new
configfield), but the OpenAPIsummary/descriptionstill describe it as only returning an indexing status snapshot from ENSIndexer. Update these strings to reflect the new combined payload so generated docs stay accurate.
describeRoute({
tags: ["Meta"],
summary: "Get ENSIndexer Indexing Status",
description: "Returns the indexing status snapshot most recently captured from ENSIndexer",
responses: {
💡 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (9)
packages/ensnode-sdk/src/ensapi/config/deserialize.ts (1)
12-19:⚠️ Potential issue | 🟡 MinorRemove redundant JSDoc return tag.
The summary already covers the return value.
🧹 Suggested change
- * `@return` An unvalidated {`@link` EnsApiPublicConfig} object.As per coding guidelines: Do not add JSDoc
@returnstags that merely restate the method summary; remove redundancy during PR review.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ensnode-sdk/src/ensapi/config/deserialize.ts` around lines 12 - 19, Remove the redundant JSDoc `@return/`@returns tag from the comment block above buildUnvalidatedEnsApiPublicConfig; keep the existing summary and `@param` description but delete the line that restates the return value so the documentation is not duplicative.packages/ensnode-react/README.md (2)
127-143:⚠️ Potential issue | 🟡 MinorUpdate API reference to use
optionsand EnsApi types.The interface and props list still reference
config/ENSNodeConfig, which no longer match the EnsApiProvider surface.✏️ Suggested doc update
- The provider component that supplies ENSNode configuration to all child components. + The provider component that supplies EnsApi provider options to all child components. interface EnsApiProviderProps { - config: ENSNodeConfig; + options: EnsApiProviderOptions; queryClient?: QueryClient; queryClientOptions?: QueryClientOptions; } - `config`: ENSNode configuration object + `options`: EnsApi provider options🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ensnode-react/README.md` around lines 127 - 143, Update the EnsApiProvider API docs to match the new surface: replace references to the old EnsApiProviderProps/config/ENSNodeConfig with the current props using options and EnsApi types; specifically, document the props as something like options: EnsApiOptions (or options) and api?: EnsApi (or api) / queryClient? / queryClientOptions? as applicable to the actual component signature, and update the Props list bullets to reference options and EnsApi types instead of config/ENSNodeConfig so the README aligns with the EnsApiProvider implementation.
233-251:⚠️ Potential issue | 🟡 MinorAdvanced usage snippets still pass
config; switch tooptions.These examples should mirror the new provider API.
✏️ Suggested doc update
-<EnsApiProvider - config={config} +<EnsApiProvider + options={options} queryClientOptions={{ defaultOptions: { queries: { staleTime: 1000 * 60 * 10, // 10 minutes gcTime: 1000 * 60 * 60, // 1 hour retry: 5, }, }, }} > <App /> </EnsApiProvider> ... - <EnsApiProvider config={config} queryClient={queryClient}> + <EnsApiProvider options={options} queryClient={queryClient}> <App /> </EnsApiProvider>Also applies to: 271-274
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ensnode-react/README.md` around lines 233 - 251, The README examples use the old prop name `config` for EnsApiProvider; update them to use the new `options` prop instead. Find the EnsApiProvider usage examples (including the snippet that mirrors advanced usage and the later similar snippet) and replace `config={config}` with `options={options}` (and update any variable names or surrounding text to match `options` if necessary) so the docs reflect the new provider API.packages/ensnode-react/src/hooks/usePrimaryName.ts (1)
15-16:⚠️ Potential issue | 🟡 MinorRemove redundant JSDoc return tag.
The summary already explains what the hook returns.
🧹 Suggested change
- * `@returns` Query result with resolved primary nameAs per coding guidelines: Do not add JSDoc
@returnstags that merely restate the method summary; remove redundancy during PR review.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ensnode-react/src/hooks/usePrimaryName.ts` around lines 15 - 16, The JSDoc for the usePrimaryName hook contains a redundant `@returns` tag that restates the summary; edit the JSDoc above the usePrimaryName function in packages/ensnode-react/src/hooks/usePrimaryName.ts and remove the `@returns` line so only the descriptive summary remains, leaving parameter tags (e.g., `@param` parameters) intact.packages/ensnode-react/src/hooks/useResolvedIdentity.ts (1)
26-27:⚠️ Potential issue | 🟡 MinorAlign JSDoc param name with
namespace.The docs still reference
namespaceId.✏️ Suggested doc fix
- * `@param` parameters.namespaceId - The {`@link` ENSNamespaceId} that `identity.chainId` should be interpreted + * `@param` parameters.namespace - The {`@link` ENSNamespaceId} that `identity.chainId` should be interpretedAlso applies to: 41-41
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ensnode-react/src/hooks/useResolvedIdentity.ts` around lines 26 - 27, Update the JSDoc for useResolvedIdentity to use the correct parameter name "namespace" (not "namespaceId"); locate the docblocks mentioning "parameters.namespaceId" (around the comment for getResolvePrimaryNameChainIdParam usage) and replace them with "parameters.namespace" so the param name aligns with the actual function signature and do the same for the second occurrence referenced at line ~41.packages/ensnode-react/src/provider.tsx (1)
97-106: 🧹 Nitpick | 🔵 TrivialMinor:
EnsApiClient.defaultOptions()called twice.Both the URL fallback and the spread invoke
defaultOptions(). Consider calling it once:♻️ Suggested refactor
export function createEnsApiOptions(options?: { url?: string | URL }): EnsApiProviderOptions { + const defaults = EnsApiClient.defaultOptions(); - const url = options?.url ? new URL(options.url) : EnsApiClient.defaultOptions().url; + const url = options?.url ? new URL(options.url) : defaults.url; return { client: { - ...EnsApiClient.defaultOptions(), + ...defaults, url, }, }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ensnode-react/src/provider.tsx` around lines 97 - 106, createEnsApiOptions calls EnsApiClient.defaultOptions() twice; assign the result to a local variable (e.g., defaults = EnsApiClient.defaultOptions()), use defaults.url as the fallback when building url (options?.url ? new URL(options.url) : defaults.url), and then spread defaults into the returned client ({ ...defaults, url }) so defaultOptions is invoked only once; update the function createEnsApiOptions accordingly.packages/ensnode-sdk/src/ensapi/api/indexing-status/deserialize.ts (1)
27-33:⚠️ Potential issue | 🟠 MajorOk responses missing
configwill now fail deserialization.Test fixtures in
client.test.ts(lines 142–143 and 200–201) omit theconfigfield despite being typed asSerializedEnsApiIndexingStatusResponseOk. Since the serialized schema requiresconfig, deserializing these fixtures will throw an error. Either addconfigto all existing Ok response payloads/fixtures, or make the field optional in the serialized schema if backward compatibility is required.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ensnode-sdk/src/ensapi/api/indexing-status/deserialize.ts` around lines 27 - 33, The deserializer assumes serializedResponse.config always exists and will throw for existing test fixtures missing config (type SerializedEnsApiIndexingStatusResponseOk). Fix by making config optional at deserialization: in deserialize.ts check for serializedResponse.config and only call buildUnvalidatedEnsApiPublicConfig when present (e.g., config: serializedResponse.config ? buildUnvalidatedEnsApiPublicConfig(serializedResponse.config) : undefined), and update the serialized type (SerializedEnsApiIndexingStatusResponseOk) or schema to mark config as optional for backward compatibility; alternatively, add the missing config objects to the test fixtures in client.test.ts to match the required schema.apps/ensadmin/src/hooks/active/use-active-connection.tsx (1)
5-17:⚠️ Potential issue | 🟡 MinorStale
@returnsqualifier after the hook migration.
useEnsApiConfig().dataisEnsApiPublicConfig— which includesversionandtheGraphFallbackin addition toensIndexerPublicConfig. The parenthetical(currently only the ENSIndexer config)is now inaccurate and should be updated (or removed per the guideline against redundant@returnstags).✏️ Suggested update
- * `@returns` The active ENSNode connection (currently only the ENSIndexer config) + * `@returns` The active ENSApi public configuration (version, theGraphFallback, ensIndexerPublicConfig)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ensadmin/src/hooks/active/use-active-connection.tsx` around lines 5 - 17, The JSDoc `@returns` is stale: update the comment for the hook (use-active-connection / the hook that returns useEnsApiConfig().data) so it accurately describes that the hook returns an EnsApiPublicConfig (including version and theGraphFallback as well as ensIndexerPublicConfig) instead of saying "(currently only the ENSIndexer config)"; either remove the parenthetical or replace it with a concise phrase like "includes version, theGraphFallback, and ensIndexerPublicConfig" and ensure the `@returns` line matches the actual returned type from useEnsApiConfig().data.packages/ensnode-react/src/utils/query.ts (1)
69-165: 🧹 Nitpick | 🔵 TrivialOptional: rename
configparameter tooptionsacross all query builder functions.The parameter was named
configwhen the type wasENSNodeSDKConfig, but now it'sEnsApiProviderOptions. Keeping the old name is mildly confusing — hook callers consistently name the same valueproviderOptions, while the builders call itconfig. Renaming tooptionshere would close that gap.♻️ Example diff (applies to all seven builder functions)
export function createRecordsQueryOptions<SELECTION extends ResolverRecordsSelection>( - config: EnsApiProviderOptions, + options: EnsApiProviderOptions, args: ResolveRecordsRequest<SELECTION>, ) { return { enabled: true, - queryKey: queryKeys.records(config.client.url.href, args), + queryKey: queryKeys.records(options.client.url.href, args), queryFn: async () => { - const client = new EnsApiClient(config.client); + const client = new EnsApiClient(options.client); return client.resolveRecords(args.name, args.selection, args); }, }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ensnode-react/src/utils/query.ts` around lines 69 - 165, Rename the parameter named config to options in all query builder functions (createRecordsQueryOptions, createPrimaryNameQueryOptions, createPrimaryNamesQueryOptions, createIndexingStatusQueryOptions, createRegistrarActionsQueryOptions, createNameTokensQueryOptions), update its type to EnsApiProviderOptions where declared, and replace all usages of config.* with options.* (e.g., queryKeys.*(options.client.url.href, ...), new EnsApiClient(options.client), etc.) so the parameter name aligns with callers using providerOptions.
🤖 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/ensadmin/src/components/providers/selected-ensnode-provider.tsx`:
- Around line 24-27: The options object passed to EnsApiProvider is recreated on
every render because createEnsApiOptions(...) is called directly; wrap the call
in React's useMemo (in the SelectedEnsNodeProvider component) keyed on
selectedConnection.validatedSelectedConnection.url so the options object is
stable and the EnsApiInternalProvider's useMemo can work as intended; ensure you
import useMemo and use the memoized value (options) when rendering
<EnsApiProvider options={options}>{children}</EnsApiProvider>.
In `@packages/ensnode-react/src/hooks/useEnsApiProviderOptions.ts`:
- Around line 8-13: Remove the redundant/misleading JSDoc `@returns` tag from the
useEnsApiProviderOptions hook comment block; update the JSDoc above function
useEnsApiProviderOptions to keep the summary and `@param/`@throws if needed but
delete the `@returns` line (which redundantly repeats the summary and references
ENSNode) so the documentation is concise and accurate.
In `@packages/ensnode-react/src/hooks/useRecords.ts`:
- Around line 56-57: The hook useRecords uses the variable name _config for the
result of useEnsApiProviderOptions, which is inconsistent with sibling hooks
usePrimaryNames and useRegistrarActions that name this providerOptions; rename
_config to providerOptions in useRecords and update all its usages (the variable
returned from useEnsApiProviderOptions and any subsequent references) so the
hook consistently uses providerOptions like the others.
In `@packages/ensnode-react/src/provider.tsx`:
- Around line 36-37: The useMemo call in provider.tsx (const memoizedOptions =
useMemo(() => options, [options])) is a no-op because it only tracks the object
reference; fix by making the memo depend on stable primitive values or move the
memoization upstream in SelectedEnsApiProvider: either compute memoizedOptions
from the specific option fields (e.g., name, url, networkId) so useMemo
stabilizes on primitives, or in SelectedEnsApiProvider wrap
createEnsApiOptions(...) in useMemo with a dependency array of the underlying
primitive inputs passed to createEnsApiOptions; update any references to
memoizedOptions accordingly.
In `@packages/ensnode-react/src/types.ts`:
- Around line 17-23: Update the outdated JSDoc comments in this file to
reference the renamed symbols (EnsApi/options) instead of ENSNode/config: change
the top comment for the EnsApiProviderOptions interface to mention
EnsApi/options and update the inline comment for the client property to
reference EnsApiClientOptions/EnsApi options; also make the same updates for the
similar comments around lines 33-38 that reference the old ENSNode/config naming
so all documentation matches the renamed types.
In `@packages/ensnode-sdk/src/ensapi/api/indexing-status/zod-schemas.ts`:
- Around line 64-68: The serialized Ok response schema currently uses z.object
which allows unknown keys; make it consistent with
makeEnsApiIndexingStatusResponseOkSchema and the error schema by changing
z.object({...}) to z.strictObject({...}) around the response (or, if the
relaxation is intentional, add a clarifying comment explaining why extra keys
must be allowed), and ensure the same pattern is applied where
makeSerializedRealtimeIndexingStatusProjectionSchema and
makeSerializedEnsApiPublicConfigSchema are composed so unknown keys are rejected
unless explicitly intended.
---
Outside diff comments:
In `@apps/ensadmin/src/hooks/active/use-active-connection.tsx`:
- Around line 5-17: The JSDoc `@returns` is stale: update the comment for the hook
(use-active-connection / the hook that returns useEnsApiConfig().data) so it
accurately describes that the hook returns an EnsApiPublicConfig (including
version and theGraphFallback as well as ensIndexerPublicConfig) instead of
saying "(currently only the ENSIndexer config)"; either remove the parenthetical
or replace it with a concise phrase like "includes version, theGraphFallback,
and ensIndexerPublicConfig" and ensure the `@returns` line matches the actual
returned type from useEnsApiConfig().data.
In `@packages/ensnode-react/README.md`:
- Around line 127-143: Update the EnsApiProvider API docs to match the new
surface: replace references to the old EnsApiProviderProps/config/ENSNodeConfig
with the current props using options and EnsApi types; specifically, document
the props as something like options: EnsApiOptions (or options) and api?: EnsApi
(or api) / queryClient? / queryClientOptions? as applicable to the actual
component signature, and update the Props list bullets to reference options and
EnsApi types instead of config/ENSNodeConfig so the README aligns with the
EnsApiProvider implementation.
- Around line 233-251: The README examples use the old prop name `config` for
EnsApiProvider; update them to use the new `options` prop instead. Find the
EnsApiProvider usage examples (including the snippet that mirrors advanced usage
and the later similar snippet) and replace `config={config}` with
`options={options}` (and update any variable names or surrounding text to match
`options` if necessary) so the docs reflect the new provider API.
In `@packages/ensnode-react/src/hooks/usePrimaryName.ts`:
- Around line 15-16: The JSDoc for the usePrimaryName hook contains a redundant
`@returns` tag that restates the summary; edit the JSDoc above the usePrimaryName
function in packages/ensnode-react/src/hooks/usePrimaryName.ts and remove the
`@returns` line so only the descriptive summary remains, leaving parameter tags
(e.g., `@param` parameters) intact.
In `@packages/ensnode-react/src/hooks/useResolvedIdentity.ts`:
- Around line 26-27: Update the JSDoc for useResolvedIdentity to use the correct
parameter name "namespace" (not "namespaceId"); locate the docblocks mentioning
"parameters.namespaceId" (around the comment for
getResolvePrimaryNameChainIdParam usage) and replace them with
"parameters.namespace" so the param name aligns with the actual function
signature and do the same for the second occurrence referenced at line ~41.
In `@packages/ensnode-react/src/provider.tsx`:
- Around line 97-106: createEnsApiOptions calls EnsApiClient.defaultOptions()
twice; assign the result to a local variable (e.g., defaults =
EnsApiClient.defaultOptions()), use defaults.url as the fallback when building
url (options?.url ? new URL(options.url) : defaults.url), and then spread
defaults into the returned client ({ ...defaults, url }) so defaultOptions is
invoked only once; update the function createEnsApiOptions accordingly.
In `@packages/ensnode-react/src/utils/query.ts`:
- Around line 69-165: Rename the parameter named config to options in all query
builder functions (createRecordsQueryOptions, createPrimaryNameQueryOptions,
createPrimaryNamesQueryOptions, createIndexingStatusQueryOptions,
createRegistrarActionsQueryOptions, createNameTokensQueryOptions), update its
type to EnsApiProviderOptions where declared, and replace all usages of config.*
with options.* (e.g., queryKeys.*(options.client.url.href, ...), new
EnsApiClient(options.client), etc.) so the parameter name aligns with callers
using providerOptions.
In `@packages/ensnode-sdk/src/ensapi/api/indexing-status/deserialize.ts`:
- Around line 27-33: The deserializer assumes serializedResponse.config always
exists and will throw for existing test fixtures missing config (type
SerializedEnsApiIndexingStatusResponseOk). Fix by making config optional at
deserialization: in deserialize.ts check for serializedResponse.config and only
call buildUnvalidatedEnsApiPublicConfig when present (e.g., config:
serializedResponse.config ?
buildUnvalidatedEnsApiPublicConfig(serializedResponse.config) : undefined), and
update the serialized type (SerializedEnsApiIndexingStatusResponseOk) or schema
to mark config as optional for backward compatibility; alternatively, add the
missing config objects to the test fixtures in client.test.ts to match the
required schema.
In `@packages/ensnode-sdk/src/ensapi/config/deserialize.ts`:
- Around line 12-19: Remove the redundant JSDoc `@return/`@returns tag from the
comment block above buildUnvalidatedEnsApiPublicConfig; keep the existing
summary and `@param` description but delete the line that restates the return
value so the documentation is not duplicative.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (34)
apps/ensadmin/src/components/config/useEnsApiConfig.tsapps/ensadmin/src/components/connection/cards/ensnode-info.tsxapps/ensadmin/src/components/connections/require-active-connection.tsxapps/ensadmin/src/components/indexing-status/use-indexing-status-with-swr.tsapps/ensadmin/src/components/layout-wrapper.tsxapps/ensadmin/src/components/providers/selected-ensnode-provider.tsxapps/ensadmin/src/components/registrar-actions/use-stateful-fetch-registrar-actions.tsapps/ensadmin/src/hooks/active/use-active-connection.tsxapps/ensadmin/src/hooks/async/use-namespace.tsapps/ensapi/src/cache/indexing-status.cache.tsapps/ensapi/src/handlers/ensnode-api.tspackages/ensnode-react/README.mdpackages/ensnode-react/src/context.tspackages/ensnode-react/src/hooks/index.tspackages/ensnode-react/src/hooks/useENSNodeConfig.tspackages/ensnode-react/src/hooks/useENSNodeSDKConfig.tspackages/ensnode-react/src/hooks/useEnsApiProviderOptions.tspackages/ensnode-react/src/hooks/useIndexingStatus.tspackages/ensnode-react/src/hooks/useNameTokens.tspackages/ensnode-react/src/hooks/usePrimaryName.tspackages/ensnode-react/src/hooks/usePrimaryNames.tspackages/ensnode-react/src/hooks/useRecords.tspackages/ensnode-react/src/hooks/useRegistrarActions.tspackages/ensnode-react/src/hooks/useResolvedIdentity.tspackages/ensnode-react/src/provider.tsxpackages/ensnode-react/src/types.tspackages/ensnode-react/src/utils/query.tspackages/ensnode-sdk/src/ensapi/api/indexing-status/deserialize.tspackages/ensnode-sdk/src/ensapi/api/indexing-status/response.tspackages/ensnode-sdk/src/ensapi/api/indexing-status/serialize.tspackages/ensnode-sdk/src/ensapi/api/indexing-status/serialized-response.tspackages/ensnode-sdk/src/ensapi/api/indexing-status/zod-schemas.tspackages/ensnode-sdk/src/ensapi/config/deserialize.tspackages/namehash-ui/src/components/identity/ResolveAndDisplayIdentity.tsx
💤 Files with no reviewable changes (2)
- packages/ensnode-react/src/hooks/useENSNodeSDKConfig.ts
- packages/ensnode-react/src/hooks/useENSNodeConfig.ts
🦋 Changeset detectedLatest commit: b2f37cf The changes in this PR will be included in the next version bump. This PR includes changesets to release 23 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
@greptile review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 39 out of 39 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (2)
apps/ensadmin/src/app/mock/indexing-stats/page.tsx:14
CrossChainIndexingStatusSnapshotis imported but no longer used after changingfetchMockedIndexingStatusto return the full response object. This unused import will fail Biome lint; remove it.
import {
CrossChainIndexingStatusSnapshot,
createRealtimeIndexingStatusProjection,
EnsApiIndexingStatusResponseOk,
IndexingStatusResponseCodes,
IndexingStatusResponseOk,
OmnichainIndexingStatusIds,
} from "@ensnode/ensnode-sdk";
packages/ensnode-react/README.md:141
- The README's API reference still documents
EnsApiProviderPropsas taking aconfigprop and usesENSNodeConfigtypes, but the implementation now expectsoptions: EnsApiProviderOptions. This will mislead consumers—update the interface snippet and props list to reflect the newoptionsprop and correct type names.
### EnsApiProvider
The provider component that supplies ENSNode configuration to all child components.
```tsx
interface EnsApiProviderProps {
config: ENSNodeConfig;
queryClient?: QueryClient;
queryClientOptions?: QueryClientOptions;
}
Props
config: ENSNode configuration object
</details>
---
💡 <a href="/namehash/ensnode/new/main?filename=.github/instructions/*.instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add Copilot custom instructions</a> for smarter, more guided reviews. <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Learn how to get started</a>.
Greptile SummaryThis PR merges the Confidence Score: 5/5Safe to merge — only P2 style/docs findings remain. All issues found are P2: one misleading JSDoc comment on a cached optional field, and one unreachable guard in a utility function. No logic errors, data-loss risks, or security concerns were identified. The consolidation of config into indexing-status is cleanly implemented with proper error propagation and indefinite caching via lazyProxy. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant Client as ENSAdmin / SDK Client
participant ENSApi as ENSApi /api/indexing-status
participant StackInfoCache as SWRCache<EnsNodeStackInfo>
participant ENSDb as ENSDb (Postgres)
participant IndexingMiddleware as IndexingStatus Middleware
Note over StackInfoCache: lazyProxy — constructed on first access
Client->>ENSApi: GET /api/indexing-status
ENSApi->>StackInfoCache: stackInfoCache.read()
alt Cache cold / error TTL expired
StackInfoCache->>ENSDb: buildEnsDbPublicConfig() → SELECT version()
ENSDb-->>StackInfoCache: EnsDbPublicConfig
StackInfoCache-->>ENSApi: EnsNodeStackInfo (cached ∞)
else Cache warm
StackInfoCache-->>ENSApi: EnsNodeStackInfo (from cache)
end
ENSApi->>IndexingMiddleware: run indexingStatusMiddleware
IndexingMiddleware-->>ENSApi: RealtimeIndexingStatusProjection
ENSApi-->>Client: { responseCode, realtimeProjection, stackInfo }
Reviews (10): Last reviewed commit: "Apply AI PR feedback" | Re-trigger Greptile |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 79 out of 116 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (3)
packages/ensnode-react/src/hooks/useResolvedIdentity.ts:64
namespaceis required inUseResolvedIdentityParameters, but this implementation still treats it as possiblyundefined(namespace ?? ENSNamespaceIds.Mainnetandenabled: ... && namespace !== undefined). Either makenamespaceoptional in the type (and keep this gating), or simplify the hook by removing the fallback/undefined checks now that callers must always provide a namespace.
packages/ensnode-sdk/src/ensnode/client.ts:53- The
EnsNodeClientJSDoc still lists a “Configuration API”, but theconfig()method andGET /api/configendpoint were removed in this PR. Please update this list to avoid misleading SDK users (config now comes fromindexingStatus()viastackInfo).
packages/ensnode-sdk/README.md:19 - README still states the client supports a separate “Configuration API”, but
/api/configandclient.config()were removed. Please update this section to point users toindexingStatus()and thestackInfofield for public config.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
eb6ec63 to
2e8a476
Compare
|
@greptile review |
lightwalker-eth
left a comment
There was a problem hiding this comment.
@tk-o Some really good updates here! I think we're close. Shared a few more suggestions with feedback 👍
| @@ -16,7 +16,7 @@ import { useEnsApiPublicConfig } from "@/components/config/use-ens-api-public-co | |||
| * @throws Error if no active ENSNode connection is available | |||
| */ | |||
| export function useActiveENSNodeConfig() { | |||
There was a problem hiding this comment.
I assume we should rename this hook, this file, and also make a number of updates to the JSDoc here?
.. or what exactly is the reason for this hook being distinct from the useEnsNodeStackInfo hook? Maybe we should just completely remove this hook now?
There was a problem hiding this comment.
I'll rename the hook and update the docs, not sure if we need to remove it. Logged an issue here: #1971
| * | ||
| * @throws when the version cannot be retrieved or parsed from the query result. | ||
| */ | ||
| private async getPostgreSqlVersion(): Promise<string> { |
There was a problem hiding this comment.
| private async getPostgreSqlVersion(): Promise<string> { | |
| private async getPostgresVersion(): Promise<string> { |
| private async getPostgreSqlVersion(): Promise<string> { | ||
| const result = await this.ensDb.execute<{ version: string }>("SELECT version();"); | ||
|
|
||
| // result will be in the form of [{ version: "PostgreSQL 15.5 (Ubuntu 15.5-0ubuntu0.22.04.1) ..." }] |
There was a problem hiding this comment.
Would the logic below return "15" or "15.5"? Seems it might be nice if it returned "15.5" but appreciate your advice 👍 I'm also not sure if that would cause a problem if the raw string here just was "15 ..." and not "15.x ..."
There was a problem hiding this comment.
I've extracted version info parser logic into this function parsePgVersionInfo and have unit tests implemented to describe the space of possible results 👍
Explicitly define types for all fields in the data model. Uses new type aliases `SerializedEnsDbPublicConfig` and `SerializedEnsRainbowPublicConfig`.
…ith-indexing-status
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 88 out of 125 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
packages/ensnode-sdk/README.md:19
- The README still lists a separate "Configuration API", but this PR removes
GET /api/configand the SDK clientconfig()method. Update the README to reflect that stack/config info is now returned viaindexingStatus()(e.g.,stackInfoon the OK response) rather than a standalone Config API.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Also, created an issue to evaluate if we need the `useActiveEnsNodeStackInfo` hook at all. More details here: #1971
|
@greptile review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 95 out of 130 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
packages/ensnode-sdk/src/ensnode/api/indexing-status/zod-schemas.ts:71
makeSerializedEnsApiIndexingStatusResponseOkSchemausesz.object(...)while the non-serialized schema (and other API serialized schemas, e.g. EnsIndexer indexing-status) usez.strictObject(...). If this isn’t intentional, this loosens validation and removesadditionalProperties: falsefrom the generated OpenAPI for/api/indexing-statusOK responses; consider switching back toz.strictObjectfor consistency and stricter contract enforcement.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 95 out of 130 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@greptile review |
lightwalker-eth
left a comment
There was a problem hiding this comment.
@tk-o Nice updates, thank you 👍 Shared a few small remaining comments. Please take the lead to merge when ready!
| * the error appropriately. | ||
| * | ||
| * In the case of a successful retrieval, this will be | ||
| * the ENSNode Stack Info object, which is considered immutable for |
There was a problem hiding this comment.
I don't think it's a good practice for us to transform type names such as EnsNodeStackInfo into text such as "ENSNode Stack Info" inside JSDoc comments.
Note how that makes it a lot harder for us to keep our docs in sync as we rename things and refine terminology. We have docs that are getting out of sync all over the place because of the practice of writing things like "ENSNode Stack Info". Let's completely stop that practice moving forward. It's wasting our time with PR reviews and making our docs get out of sync.
It seems much better to always do one of the following:
- Write "
EnsNodeStackInfo" (in tick marks) - Write "{@link EnsNodeStackInfo}"
Appreciate if you can take note of this for all our work moving forward whenever we're writing type names in JSDoc. For the case of this specific PR please feel welcome to update any JSDoc specifically relevant to this PR but I appreciate how it's a larger project outside the scope of this PR to make this change across all our existing JSDoc everywhere in the monorepo.
There was a problem hiding this comment.
Of course, it makes sense to stick to using @link syntax for the benefit of:
a) Tracking the actual types.
b) Supporting IDE navigation among types.
Replaced `useActiveNamespace` hook with direct use of `useActiveEnsNodeStackInfo` hook
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 102 out of 137 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** | ||
| * ENSRainbow Public Config | ||
| * | ||
| * If undefined, represents that ENSRainbow is currently undergoing | ||
| * a cold start and may take up to an hour to become ready. | ||
| */ | ||
| ensRainbow?: EnsRainbowPublicConfig; | ||
| } |
There was a problem hiding this comment.
ensRainbow is marked optional with a comment explaining an undefined value (cold start), but EnsIndexerPublicConfig.ensRainbowPublicConfig is currently required by both the TS types and Zod schemas. As written, buildEnsNodeStackInfo() will always populate ensRainbow, and clients can’t represent the documented “cold start” state without also changing the embedded ENSIndexer config shape. Consider either making ensRainbow required here (and removing the cold-start semantics), or making ENSRainbow config optional consistently across the ENSIndexer/ENSApi public config types & schemas.
Lite PR
Tip: Review docs on the ENSNode PR process
Summary
/api/configendpoint into/api/indexing-statusendpointEnsApiClient*symbols withEnsNodeClient*to enahnce the "ENSNode" branding.EnsNodeStackInfodata model to effectively describe the public configuration for each app withing the ENSNode stack (ENSApi, ENSDb, ENSIndexer, and ENSRainbow).Why
Testing
Notes for Reviewer (Optional)
/api/configinto/api/indexing-status#1405Pre-Review Checklist (Blocking)
EnsNodeStackInfodata model across the apps within the ENSNode stack.