Open
Conversation
Rename getCanonicalId to getStorageId and CanonicalId type to StorageId across ensindexer, ensapi, and ensnode-sdk. Remove the canonicalId field from the ENSv2Domain GraphQL type. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: d9637fc 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 |
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 6 minutes and 49 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughConsolidates public ENS types into a new Changes
Sequence Diagram(s)sequenceDiagram
participant Contract as ERC1155/EventSource
participant Indexer as ENS Indexer Handler
participant DB as Database
participant API as Omnigraph API Server
Contract->>Indexer: emit LabelRegistered/Transfer/TokenRegenerated (tokenId, registry, labelHash)
Note right of Indexer: compute storageId = getStorageId(tokenId|labelHash)
Indexer->>DB: lookup domain by makeENSv2DomainId(registry, storageId)
DB-->>Indexer: return domain (or null)
alt domain exists
Indexer->>DB: update registration/expiry/resolver using domainId(storageId)
else new domain
Indexer->>DB: insert ENSv2 domain record with domainId(storageId)
end
Indexer->>API: (optional) emit materialized domain event / update subscription
API->>DB: read domain by domainId(storageId) for queries/resolvers
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related issues
Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
…icalId` in favor of accessing `ENSv2Domain.tokenId` directly.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR continues the “Omnigraph Client” work by centralizing shared ENS/ENSv2/CAIP type definitions into the new enssdk package, updating downstream packages to consume those types, and aligning ENSv2 identifiers around “StorageId” (masking lower 32 bits) while removing the canonicalId field from the GraphQL surface.
Changes:
- Introduces/exports a public
enssdkroot entrypoint (types-focused) and wires it into build + package exports. - Replaces prior
canonicalIdusage withgetStorageId/StorageIdand removesENSv2Domain.canonicalIdfrom the GraphQL schema and queries. - Refactors
@ensnode/ensnode-sdkinternals and apps to import shared types fromenssdkinstead of duplicating local type definitions.
Reviewed changes
Copilot reviewed 88 out of 91 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Adds workspace linking for enssdk and new deps (viem, @ensdomains/address-encoder) where needed. |
| packages/enssdk/tsup.config.ts | Adds root index entry to the build. |
| packages/enssdk/src/omnigraph/graphql.ts | Updates GraphQL scalar typings to use enssdk types and viem Address/Hex; stringifies BigInt for client typing. |
| packages/enssdk/src/omnigraph/generated/schema.graphql | Removes ENSv2Domain.canonicalId from generated schema. |
| packages/enssdk/src/omnigraph/generated/graphql-env.d.ts | Regenerates env typings to reflect schema change (no canonicalId). |
| packages/enssdk/src/lib/types/shared.ts | Cleans up shared serialized types module (removes unused import). |
| packages/enssdk/src/lib/types/index.ts | New barrel export for enssdk type modules. |
| packages/enssdk/src/lib/types/evm.ts | Adds EVM/CAIP types (ChainId, AccountId, AssetId, etc.). |
| packages/enssdk/src/lib/types/ensv2.ts | Renames ENSv2 “canonical id” concept to StorageId; defines CanonicalPath here. |
| packages/enssdk/src/lib/types/ens.ts | Removes ENSNamespace re-exports and moves CanonicalPath out (now in ensv2). |
| packages/enssdk/src/lib/types/coin-type.ts | Re-exports CoinType/EvmCoinType from @ensdomains/address-encoder. |
| packages/enssdk/src/index.ts | New root export (export * from ./lib/types). |
| packages/enssdk/package.json | Adds root export mapping and publishConfig for .; adds deps on viem and @ensdomains/address-encoder. |
| packages/ensnode-sdk/src/tokenscope/zod-schemas.ts | Switches AssetId/AssetNamespaces imports to enssdk. |
| packages/ensnode-sdk/src/tokenscope/zod-schemas.test.ts | Updates tests to import AssetId* types from enssdk. |
| packages/ensnode-sdk/src/tokenscope/name-token.ts | Switches AccountId/AssetId/InterpretedName types to enssdk. |
| packages/ensnode-sdk/src/tokenscope/name-token.test.ts | Updates tests to import AccountId/InterpretedName from enssdk. |
| packages/ensnode-sdk/src/tokenscope/assets.ts | Moves asset-related type imports (AccountId, AssetId*, Node, etc.) to enssdk. |
| packages/ensnode-sdk/src/shared/zod-schemas.ts | Switches core shared type imports (AccountId, ChainId, Node, etc.) to enssdk. |
| packages/ensnode-sdk/src/shared/types.ts | Removes duplicated EVM/asset type definitions (now sourced from enssdk). |
| packages/ensnode-sdk/src/shared/serialize.ts | Switches serialized type imports (*String, UrlString, etc.) to enssdk. |
| packages/ensnode-sdk/src/shared/root-registry.ts | Switches AccountId type to enssdk. |
| packages/ensnode-sdk/src/shared/labelhash.ts | Switches LabelHash/LiteralLabel type imports to enssdk. |
| packages/ensnode-sdk/src/shared/interpretation/reinterpretation.ts | Switches Interpreted* types to enssdk. |
| packages/ensnode-sdk/src/shared/interpretation/interpreted-names-and-labels.ts | Switches name/label-related type imports to enssdk. |
| packages/ensnode-sdk/src/shared/interpretation/interpret-tokenid.ts | Switches LabelHash/Node type imports to enssdk. |
| packages/ensnode-sdk/src/shared/interpretation/interpret-record-values.ts | Switches NormalizedName type import to enssdk. |
| packages/ensnode-sdk/src/shared/deserialize.ts | Switches AccountId/ChainId*/UrlString imports to enssdk. |
| packages/ensnode-sdk/src/shared/datasource-contract.ts | Switches AccountId type import to enssdk. |
| packages/ensnode-sdk/src/shared/config/zod-schemas.ts | Switches ChainId type import to enssdk. |
| packages/ensnode-sdk/src/shared/config/types.ts | Switches ChainId/UrlString type imports to enssdk. |
| packages/ensnode-sdk/src/shared/config/rpc-configs-from-env.ts | Switches ChainIdString type import to enssdk. |
| packages/ensnode-sdk/src/shared/config/indexed-blockranges.ts | Switches ChainId type import to enssdk. |
| packages/ensnode-sdk/src/shared/config/indexed-blockranges.test.ts | Updates test to import ChainId from enssdk. |
| packages/ensnode-sdk/src/shared/config/build-rpc-urls.test.ts | Updates test to import ChainId from enssdk. |
| packages/ensnode-sdk/src/shared/account-id.ts | Switches AccountId type import to enssdk. |
| packages/ensnode-sdk/src/shared/account-id.test.ts | Updates test to import AccountId from enssdk. |
| packages/ensnode-sdk/src/resolution/types.ts | Switches ChainId/Name types to enssdk. |
| packages/ensnode-sdk/src/resolution/resolver-records-selection.ts | Switches CoinType type import to enssdk. |
| packages/ensnode-sdk/src/resolution/resolver-records-response.ts | Switches CoinType/Name types to enssdk. |
| packages/ensnode-sdk/src/resolution/ensip19-chainid.ts | Switches ChainId/DefaultableChainId types to enssdk. |
| packages/ensnode-sdk/src/registrars/subregistry.ts | Switches AccountId/Node types to enssdk. |
| packages/ensnode-sdk/src/registrars/registration-lifecycle.ts | Switches Node type import to enssdk. |
| packages/ensnode-sdk/src/registrars/lineanames-subregistry.ts | Switches AccountId/Name types to enssdk. |
| packages/ensnode-sdk/src/registrars/ethnames-subregistry.ts | Switches AccountId/Name types to enssdk. |
| packages/ensnode-sdk/src/registrars/basenames-subregistry.ts | Switches AccountId/Name types to enssdk. |
| packages/ensnode-sdk/src/indexing-status/serialize/omnichain-indexing-status-snapshot.ts | Switches ChainIdString type import to enssdk. |
| packages/ensnode-sdk/src/indexing-status/serialize/chain-indexing-status-snapshot.ts | Switches ChainId/ChainIdString types to enssdk. |
| packages/ensnode-sdk/src/indexing-status/omnichain-indexing-status-snapshot.ts | Switches ChainId type import to enssdk. |
| packages/ensnode-sdk/src/indexing-status/deserialize/omnichain-indexing-status-snapshot.ts | Switches ChainId type import to enssdk. |
| packages/ensnode-sdk/src/indexing-status/cross-chain-indexing-status-snapshot.ts | Switches ChainId type import to enssdk. |
| packages/ensnode-sdk/src/indexing-status/chain-indexing-status-snapshot.ts | Switches ChainId type import to enssdk. |
| packages/ensnode-sdk/src/index.ts | Re-exports ENSNamespaceId(s) from @ensnode/datasources; removes shared/serialized-types re-export. |
| packages/ensnode-sdk/src/identity/types.ts | Switches DefaultableChainId/Name types to enssdk. |
| packages/ensnode-sdk/src/identity/identity.ts | Switches DefaultableChainId type import to enssdk. |
| packages/ensnode-sdk/src/ensv2/index.ts | Stops exporting ./ids; keeps ./ids-lib. |
| packages/ensnode-sdk/src/ensv2/ids-lib.ts | Renames canonical-id concepts to StorageId and updates makeENSv2DomainId/getStorageId. |
| packages/ensnode-sdk/src/ensindexer/config/types.ts | Switches ChainId type import to enssdk. |
| packages/ensnode-sdk/src/ensindexer/config/serialized-types.ts | Switches ChainId type import to enssdk. |
| packages/ensnode-sdk/src/ensindexer/config/serialize.ts | Switches ChainId type import to enssdk. |
| packages/ensnode-sdk/src/ensindexer/config/label-utils.ts | Switches LabelHash type import to enssdk. |
| packages/ensnode-sdk/src/ensapi/api/registrar-actions/zod-schemas.test.ts | Switches InterpretedName type import to enssdk. |
| packages/ensnode-sdk/src/ensapi/api/registrar-actions/response.ts | Switches InterpretedName type import to enssdk. |
| packages/ensnode-sdk/src/ensapi/api/registrar-actions/request.ts | Switches Node type import to enssdk. |
| packages/ensnode-sdk/src/ensapi/api/registrar-actions/filters.ts | Switches Node type import to enssdk. |
| packages/ensnode-sdk/src/ensapi/api/name-tokens/zod-schemas.test.ts | Switches AssetNamespaces and InterpretedName to enssdk. |
| packages/ensnode-sdk/src/ensapi/api/name-tokens/response.ts | Switches InterpretedName/Node types to enssdk. |
| packages/ensnode-sdk/src/ensapi/api/name-tokens/request.ts | Switches Name/Node types to enssdk. |
| packages/ensnode-sdk/src/ens/subname-helpers.ts | Switches LabelHash/Node types to enssdk. |
| packages/ensnode-sdk/src/ens/reverse-name.ts | Switches CoinType/Label/Name types to enssdk; keeps local coin-type constants. |
| packages/ensnode-sdk/src/ens/parse-reverse-name.ts | Switches CoinType/Label/Name types to enssdk. |
| packages/ensnode-sdk/src/ens/parse-labelhash.ts | Switches LabelHash type import to enssdk. |
| packages/ensnode-sdk/src/ens/names.ts | Switches Label/Name/NormalizedName types to enssdk. |
| packages/ensnode-sdk/src/ens/names.test.ts | Switches Name/NormalizedName type imports to enssdk. |
| packages/ensnode-sdk/src/ens/labelhash.ts | Switches LabelHash type import to enssdk. |
| packages/ensnode-sdk/src/ens/is-normalized.ts | Switches Label/Name/NormalizedName types to enssdk. |
| packages/ensnode-sdk/src/ens/index.ts | Re-exports all of enssdk from the ens module and re-exports datasources identifiers; removes local ./types export. |
| packages/ensnode-sdk/src/ens/encode-labelhash.ts | Switches EncodedLabelHash/LabelHash type imports to enssdk. |
| packages/ensnode-sdk/src/ens/dns-encoded-name.ts | Switches DNS-encoded name types to enssdk. |
| packages/ensnode-sdk/src/ens/dns-encoded-name.test.ts | Switches DNS-encoded name types to enssdk. |
| packages/ensnode-sdk/src/ens/constants.ts | Switches Node type import to enssdk. |
| packages/ensnode-sdk/src/ens/coin-type.ts | Uses enssdk types for ChainId/CoinType/EvmCoinType (removes previous local re-export pattern). |
| packages/ensnode-sdk/package.json | Adds enssdk dependency. |
| apps/ensindexer/src/plugins/protocol-acceleration/handlers/ENSv2Registry.ts | Switches from getCanonicalId to getStorageId. |
| apps/ensindexer/src/plugins/ensv2/handlers/ensv2/ETHRegistrar.ts | Switches from getCanonicalId to getStorageId. |
| apps/ensindexer/src/plugins/ensv2/handlers/ensv2/ENSv2Registry.ts | Switches to getStorageId and updates invariants/error text accordingly. |
| apps/ensapi/src/graphql-api/yoga.ts | Removes canonicalId selection from internal query. |
| apps/ensapi/src/graphql-api/schema/query.integration.test.ts | Updates to getStorageId for ENSv2 domain id construction. |
| apps/ensapi/src/graphql-api/schema/domain.ts | Removes ENSv2Domain.canonicalId field from the GraphQL schema implementation. |
| apps/ensapi/src/graphql-api/builder.ts | Switches scalar type imports to enssdk (still using viem for Address/Hex). |
| apps/ensapi/package.json | Adds enssdk dependency. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/graphql-api/schema/query.integration.test.ts`:
- Around line 40-41: Rename the misleading V2_ETH_CANONICAL_ID to a
storage-focused name (e.g., V2_ETH_STORAGE_ID) wherever it is declared and
referenced (the declaration using getStorageId(labelhash("eth")) and its use in
makeENSv2DomainId to build V2_ETH_DOMAIN_ID); update all test variables and any
imports/uses in this file so references match the new identifier and run tests
to ensure no leftover references remain.
In `@apps/ensindexer/src/plugins/ensv2/handlers/ensv2/ENSv2Registry.ts`:
- Around line 60-61: This change alters how domainId is derived (now using
getStorageId and makeENSv2DomainId) and will break lookups and upserts; add a
migration or compatibility layer: implement a DB migration that converts
existing records from the old domainId derivation to the new one (or recomputes
and reindexes all domains), and/or update the handlers and upsert logic
(references: domainId variable, makeENSv2DomainId, getStorageId, v2Domain
upsert, LabelUnregistered and ExpiryUpdated handlers) to first attempt
legacy-key lookups and then the new key, and ensure the v2Domain upsert
merges/updates any existing legacy row instead of creating duplicates so no
orphaned records remain.
In `@packages/ensnode-sdk/src/index.ts`:
- Around line 1-2: Remove the lone explanatory comment at the top of the barrel
file; keep the existing re-exports (ENSNamespaceIds and ENSNamespaceId) intact
but delete the two-line comment so the file is purely an export list (locate the
top of packages/ensnode-sdk/src/index.ts where ENSNamespaceIds and
ENSNamespaceId are re-exported and remove the comment lines).
In `@packages/enssdk/src/omnigraph/graphql.ts`:
- Around line 20-43: The scalar mappings in the exported graphql (created via
initGraphQLTada) are out of sync with the ENS API builder scalars: update the
scalars block so it matches the builder.ts definitions — remove or reconcile the
extraneous ID: string mapping if builder.ts does not define ID, change BigInt
from a stringified template literal to the builder shape ({ Input: bigint;
Output: bigint } or the equivalent used in builder.ts), and update ChainId from
a plain number to the builder's ChainId shape ({ Input: ChainId; Output: ChainId
} or the actual ChainId type); adjust any other scalar entries in the scalars
object to exactly mirror the types in apps/ensapi/src/graphql-api/builder.ts so
the NOTE about keeping them in sync is satisfied.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b5e257c1-3941-409a-931e-98934729f0ec
⛔ Files ignored due to path filters (3)
packages/enssdk/src/omnigraph/generated/graphql-env.d.tsis excluded by!**/generated/**packages/enssdk/src/omnigraph/generated/schema.graphqlis excluded by!**/generated/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (88)
apps/ensapi/package.jsonapps/ensapi/src/graphql-api/builder.tsapps/ensapi/src/graphql-api/schema/domain.tsapps/ensapi/src/graphql-api/schema/query.integration.test.tsapps/ensapi/src/graphql-api/yoga.tsapps/ensindexer/src/plugins/ensv2/handlers/ensv2/ENSv2Registry.tsapps/ensindexer/src/plugins/ensv2/handlers/ensv2/ETHRegistrar.tsapps/ensindexer/src/plugins/protocol-acceleration/handlers/ENSv2Registry.tspackages/ensnode-sdk/package.jsonpackages/ensnode-sdk/src/ens/coin-type.tspackages/ensnode-sdk/src/ens/constants.tspackages/ensnode-sdk/src/ens/dns-encoded-name.test.tspackages/ensnode-sdk/src/ens/dns-encoded-name.tspackages/ensnode-sdk/src/ens/encode-labelhash.tspackages/ensnode-sdk/src/ens/index.tspackages/ensnode-sdk/src/ens/is-normalized.tspackages/ensnode-sdk/src/ens/labelhash.tspackages/ensnode-sdk/src/ens/names.test.tspackages/ensnode-sdk/src/ens/names.tspackages/ensnode-sdk/src/ens/parse-labelhash.tspackages/ensnode-sdk/src/ens/parse-reverse-name.tspackages/ensnode-sdk/src/ens/reverse-name.tspackages/ensnode-sdk/src/ens/subname-helpers.tspackages/ensnode-sdk/src/ensapi/api/name-tokens/request.tspackages/ensnode-sdk/src/ensapi/api/name-tokens/response.tspackages/ensnode-sdk/src/ensapi/api/name-tokens/zod-schemas.test.tspackages/ensnode-sdk/src/ensapi/api/registrar-actions/filters.tspackages/ensnode-sdk/src/ensapi/api/registrar-actions/request.tspackages/ensnode-sdk/src/ensapi/api/registrar-actions/response.tspackages/ensnode-sdk/src/ensapi/api/registrar-actions/zod-schemas.test.tspackages/ensnode-sdk/src/ensindexer/config/label-utils.tspackages/ensnode-sdk/src/ensindexer/config/serialize.tspackages/ensnode-sdk/src/ensindexer/config/serialized-types.tspackages/ensnode-sdk/src/ensindexer/config/types.tspackages/ensnode-sdk/src/ensv2/ids-lib.tspackages/ensnode-sdk/src/ensv2/index.tspackages/ensnode-sdk/src/identity/identity.tspackages/ensnode-sdk/src/identity/types.tspackages/ensnode-sdk/src/index.tspackages/ensnode-sdk/src/indexing-status/chain-indexing-status-snapshot.tspackages/ensnode-sdk/src/indexing-status/cross-chain-indexing-status-snapshot.tspackages/ensnode-sdk/src/indexing-status/deserialize/omnichain-indexing-status-snapshot.tspackages/ensnode-sdk/src/indexing-status/omnichain-indexing-status-snapshot.tspackages/ensnode-sdk/src/indexing-status/serialize/chain-indexing-status-snapshot.tspackages/ensnode-sdk/src/indexing-status/serialize/omnichain-indexing-status-snapshot.tspackages/ensnode-sdk/src/registrars/basenames-subregistry.tspackages/ensnode-sdk/src/registrars/ethnames-subregistry.tspackages/ensnode-sdk/src/registrars/lineanames-subregistry.tspackages/ensnode-sdk/src/registrars/registration-lifecycle.tspackages/ensnode-sdk/src/registrars/subregistry.tspackages/ensnode-sdk/src/resolution/ensip19-chainid.tspackages/ensnode-sdk/src/resolution/resolver-records-response.tspackages/ensnode-sdk/src/resolution/resolver-records-selection.tspackages/ensnode-sdk/src/resolution/types.tspackages/ensnode-sdk/src/shared/account-id.test.tspackages/ensnode-sdk/src/shared/account-id.tspackages/ensnode-sdk/src/shared/config/build-rpc-urls.test.tspackages/ensnode-sdk/src/shared/config/indexed-blockranges.test.tspackages/ensnode-sdk/src/shared/config/indexed-blockranges.tspackages/ensnode-sdk/src/shared/config/rpc-configs-from-env.tspackages/ensnode-sdk/src/shared/config/types.tspackages/ensnode-sdk/src/shared/config/zod-schemas.tspackages/ensnode-sdk/src/shared/datasource-contract.tspackages/ensnode-sdk/src/shared/deserialize.tspackages/ensnode-sdk/src/shared/interpretation/interpret-record-values.tspackages/ensnode-sdk/src/shared/interpretation/interpret-tokenid.tspackages/ensnode-sdk/src/shared/interpretation/interpreted-names-and-labels.tspackages/ensnode-sdk/src/shared/interpretation/reinterpretation.tspackages/ensnode-sdk/src/shared/labelhash.tspackages/ensnode-sdk/src/shared/root-registry.tspackages/ensnode-sdk/src/shared/serialize.tspackages/ensnode-sdk/src/shared/types.tspackages/ensnode-sdk/src/shared/zod-schemas.tspackages/ensnode-sdk/src/tokenscope/assets.tspackages/ensnode-sdk/src/tokenscope/name-token.test.tspackages/ensnode-sdk/src/tokenscope/name-token.tspackages/ensnode-sdk/src/tokenscope/zod-schemas.test.tspackages/ensnode-sdk/src/tokenscope/zod-schemas.tspackages/enssdk/package.jsonpackages/enssdk/src/index.tspackages/enssdk/src/lib/types/coin-type.tspackages/enssdk/src/lib/types/ens.tspackages/enssdk/src/lib/types/ensv2.tspackages/enssdk/src/lib/types/evm.tspackages/enssdk/src/lib/types/index.tspackages/enssdk/src/lib/types/shared.tspackages/enssdk/src/omnigraph/graphql.tspackages/enssdk/tsup.config.ts
💤 Files with no reviewable changes (6)
- packages/enssdk/src/lib/types/shared.ts
- packages/ensnode-sdk/src/ensv2/index.ts
- apps/ensapi/src/graphql-api/schema/domain.ts
- packages/enssdk/src/lib/types/ens.ts
- apps/ensapi/src/graphql-api/yoga.ts
- packages/ensnode-sdk/src/shared/types.ts
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 137 out of 164 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (2)
apps/ensapi/src/omnigraph-api/lib/find-domains/find-domains-resolver-helpers.test.ts:6
- This test mocks
@/omnigraph-api/lib/find-domains/find-domains-by-labelhash-path, but that module/file does not exist underapps/ensapi/src/omnigraph-api/lib/find-domains/. Vitest will fail module resolution here. Remove this mock, or update it to the actual module that needs mocking (if any) forfind-domains-resolver-helperstests.
apps/ensapi/src/omnigraph-api/yoga.ts:12 - Logger name is still
ensnode-graphqleven though this module now implements the Omnigraph API. Consider renaming the logger scope to something Omnigraph-specific (e.g.ensnode-omnigraph) to avoid confusion when both legacy GraphQL and Omnigraph are present.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/ensadmin/src/app/api/omnigraph/page.tsx (1)
7-7: 🧹 Nitpick | 🔵 TrivialConsider renaming
GRAPHQL_API_EXAMPLE_QUERIESfor consistency across the SDK and consuming code.The constant exported from the omnigraph SDK module retains the "GraphQL" prefix while this page is branded as "Omnigraph". Aligning the naming would require renaming the export in
packages/ensnode-sdk/src/omnigraph-api/example-queries.tsand updating usages inapps/ensapi/src/omnigraph-api/schema/example-queries.test.tsandapps/ensapi/src/omnigraph-api/schema/example-queries.integration.test.ts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ensadmin/src/app/api/omnigraph/page.tsx` at line 7, Rename the exported constant GRAPHQL_API_EXAMPLE_QUERIES to OMNIGRAPH_API_EXAMPLE_QUERIES in the omnigraph-api example-queries module, update its export statement accordingly, and then update all imports/usages (including tests and consuming pages) to import the new OMNIGRAPH_API_EXAMPLE_QUERIES identifier; also update any re-exports or documentation references to the old name to keep the SDK and consumers consistent.
🤖 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/api/omnigraph/omnigraph-api.ts`:
- Around line 10-14: The middleware currently returns a raw 503 via c.text when
hasOmnigraphApiConfigSupport(config.ensIndexerPublicConfig) fails; replace that
path to call the shared errorResponse helper so the API returns the standardized
ENSApi error payload. Locate the app.use middleware where
hasOmnigraphApiConfigSupport is called and swap the c.text(...) return with a
call to errorResponse(c, 503, `Service Unavailable: ${prerequisite.reason}`) (or
the project’s errorResponse signature), preserving the status code and reason
text.
In
`@apps/ensapi/src/omnigraph-api/lib/find-domains/find-domains-resolver-helpers.test.ts`:
- Line 4: Remove the unused mock for
"@/omnigraph-api/lib/find-domains/find-domains-by-labelhash-path" from the test
file (the vi.mock(...) call in find-domains-resolver-helpers.test.ts) because
the module under test (find-domains-resolver-helpers) does not import it at
runtime; simply delete that vi.mock line and run tests to confirm nothing else
references that symbol.
In `@packages/enssdk/src/omnigraph/graphql.ts`:
- Around line 25-28: Update the comment that instructs keeping scalars in sync
so it explicitly excludes the BigInt scalar and explains why: mention that the
BigInt type in omnigraph/graphql.ts (symbol BigInt) is intentionally represented
as a stringified bigint (```${bigint}```) because GraphQL clients don't
deserialize bigint scalars, whereas the other scalar types should match the
canonical definitions in omnigraph-api/builder.ts; make the comment state "keep
in sync except BigInt which is intentionally stringified here" and briefly note
the reason to avoid confusion during reviews.
---
Outside diff comments:
In `@apps/ensadmin/src/app/api/omnigraph/page.tsx`:
- Line 7: Rename the exported constant GRAPHQL_API_EXAMPLE_QUERIES to
OMNIGRAPH_API_EXAMPLE_QUERIES in the omnigraph-api example-queries module,
update its export statement accordingly, and then update all imports/usages
(including tests and consuming pages) to import the new
OMNIGRAPH_API_EXAMPLE_QUERIES identifier; also update any re-exports or
documentation references to the old name to keep the SDK and consumers
consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6d1a40e4-818f-4065-ba48-d5813bfe2788
📒 Files selected for processing (55)
apps/ensadmin/src/app/@actions/api/omnigraph/page.tsxapps/ensadmin/src/app/api/omnigraph/page.tsxapps/ensadmin/src/components/app-sidebar.tsxapps/ensadmin/src/hooks/active/use-ensadmin-features.tsxapps/ensapi/biome.jsoncapps/ensapi/package.jsonapps/ensapi/src/handlers/api/omnigraph/omnigraph-api.tsapps/ensapi/src/handlers/api/router.tsapps/ensapi/src/index.tsapps/ensapi/src/omnigraph-api/builder.tsapps/ensapi/src/omnigraph-api/lib/connection-helpers.tsapps/ensapi/src/omnigraph-api/lib/find-domains/domain-cursor.tsapps/ensapi/src/omnigraph-api/lib/find-domains/find-domains-resolver-helpers.test.tsapps/ensapi/src/omnigraph-api/lib/find-domains/find-domains-resolver-helpers.tsapps/ensapi/src/omnigraph-api/lib/find-domains/find-domains-resolver.tsapps/ensapi/src/omnigraph-api/lib/find-events/find-events-resolver.tsapps/ensapi/src/omnigraph-api/lib/write-graphql-schema.tsapps/ensapi/src/omnigraph-api/schema.tsapps/ensapi/src/omnigraph-api/schema/account-id.tsapps/ensapi/src/omnigraph-api/schema/account.integration.test.tsapps/ensapi/src/omnigraph-api/schema/account.tsapps/ensapi/src/omnigraph-api/schema/connection.tsapps/ensapi/src/omnigraph-api/schema/constants.tsapps/ensapi/src/omnigraph-api/schema/domain.integration.test.tsapps/ensapi/src/omnigraph-api/schema/domain.tsapps/ensapi/src/omnigraph-api/schema/event.tsapps/ensapi/src/omnigraph-api/schema/label.tsapps/ensapi/src/omnigraph-api/schema/name-or-node.tsapps/ensapi/src/omnigraph-api/schema/order-direction.tsapps/ensapi/src/omnigraph-api/schema/permissions.integration.test.tsapps/ensapi/src/omnigraph-api/schema/permissions.tsapps/ensapi/src/omnigraph-api/schema/query.integration.test.tsapps/ensapi/src/omnigraph-api/schema/query.tsapps/ensapi/src/omnigraph-api/schema/registration.tsapps/ensapi/src/omnigraph-api/schema/registry-permissions-user.tsapps/ensapi/src/omnigraph-api/schema/registry.integration.test.tsapps/ensapi/src/omnigraph-api/schema/registry.tsapps/ensapi/src/omnigraph-api/schema/renewal.tsapps/ensapi/src/omnigraph-api/schema/resolver-permissions-user.tsapps/ensapi/src/omnigraph-api/schema/resolver-records.tsapps/ensapi/src/omnigraph-api/schema/resolver.integration.test.tsapps/ensapi/src/omnigraph-api/schema/resolver.tsapps/ensapi/src/omnigraph-api/schema/scalars.tsapps/ensapi/src/omnigraph-api/yoga.tsapps/ensapi/src/test/integration/find-domains/domain-pagination-queries.tsapps/ensapi/src/test/integration/find-domains/test-domain-pagination.tsapps/ensapi/src/test/integration/find-events/event-pagination-queries.tsapps/ensapi/src/test/integration/global-setup.tsapps/ensapi/src/test/integration/graphql-utils.tsapps/ensapi/src/test/integration/omnigraph-api-client.tsapps/ensindexer/ponder/ponder.config.tspackages/ensnode-sdk/src/index.tspackages/ensnode-sdk/src/internal.tspackages/ensnode-sdk/src/omnigraph-api/prerequisites.tspackages/enssdk/src/omnigraph/graphql.ts
apps/ensapi/src/omnigraph-api/lib/find-domains/find-domains-resolver-helpers.test.ts
Outdated
Show resolved
Hide resolved
Member
|
@greptile |
Contributor
Greptile SummaryThis PR is a large-scale refactoring across 164 files with three main goals: (1) renaming Key changes include:
Confidence Score: 4/5Safe to merge after the changeset is updated to document the /graphql → /omnigraph route path breaking change. The refactoring is mechanically consistent and well-executed. One P1 finding remains: the API route change from .changeset/dirty-snakes-knock.md (missing route-change breaking-change entry) and apps/ensapi/src/omnigraph-api/schema/scalars.ts (Hex parseValue uses outer closure variable). Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[HTTP Client] -->|POST /api/omnigraph| B[ensapi router]
B --> C[omnigraphApi handler]
C --> D[Yoga GraphQL Server]
D --> E[Pothos Schema Builder]
E --> F[domain.ts / account.ts / registry.ts etc.]
F --> G[Drizzle ORM]
G --> H[(ensDb)]
I[enssdk] -->|semantic types| E
I -->|StorageId / DomainId etc.| J[ensnode-sdk]
J -->|getStorageId / makeENSv2DomainId| K[ensindexer handlers]
K --> G
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+1
to
+5
| --- | ||
| "ensapi": minor | ||
| --- | ||
|
|
||
| Omnigraph API (BREAKING): Removed `ENSv2Domain.canonicalId`. |
Contributor
There was a problem hiding this comment.
The changeset only records the removal of ENSv2Domain.canonicalId, but this PR introduces at least one additional breaking change that affects API consumers:
- The GraphQL API route changed from
/graphqlto/omnigraph(inapps/ensapi/src/handlers/api/router.ts). Any client that was hitting/api/graphqlwill break silently at runtime after this change. This is a breaking change for all HTTP-level API consumers and warrants a separate bullet in the changeset (or amajorbump rather thanminor, depending on your semver policy).
Consider adding a line such as:
Omnigraph API (BREAKING): API route path changed from `/graphql` to `/omnigraph`.
lightwalker-eth
approved these changes
Apr 1, 2026
Member
lightwalker-eth
left a comment
lightwalker-eth
left a comment
There was a problem hiding this comment.
@shrugs Looks awesome! Please take the lead to merge when ready 👍
| "gql.tada": "^1.8.10", | ||
| "graphql": "^16.11.0" | ||
| "graphql": "^16.11.0", | ||
| "viem": "catalog:" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #1803
closes #1848
CanonicalIdtoStorageIdto match the new terminology in the ENSv2 contractsensnode-sdktoenssdkand hooks them up to the omnigraph client, updating all relevant imports (so now projects like ensnode-sdk and ensapi depend on enssdk)