Lazy-evaluate ENSDb modules in ENSApi#2174
Conversation
🦋 Changeset detectedLatest commit: a1ad07c The changes in this PR will be included in the next version bump. This PR includes changesets to release 22 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ 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 (6)
📝 WalkthroughWalkthroughRemoves the ENS DB singleton and updates the DI container to instantiate ENSDbReader from environment; all ENS DB/schema/client consumers are changed to read those dependencies from di.context at runtime, including caches, probes, library functions, GraphQL resolvers, and tests. ChangesSingleton Dependency Injection Refactor
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
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 |
Greptile SummaryThis PR completes the migration away from
Confidence Score: 5/5Safe to merge — this is a mechanical refactoring with no changes to business logic or data paths. All 38 files follow the same consistent migration pattern. The new async init() correctly sequences ENSDb health verification, cache population, and RPC probing. Previously flagged issues (isHealthy return value not checked, missing MockEnsDbReader.destroy(), unawaited di.init() in tests) are resolved in this PR. No files require special attention. The @ts-expect-error in packages/ensdb-sdk/src/client/ensdb-reader.ts for $client.end() is documented and covered by new tests. Important Files Changed
Reviews (6): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile |
There was a problem hiding this comment.
Pull request overview
This PR migrates ENSApi’s ENSDb access away from the @/lib/ensdb/singleton lazyProxy-based singleton and onto the @/di container so ENSDb modules are instantiated lazily via DI getters instead of import-time evaluation.
Changes:
- Replace
ensDb/ensIndexerSchema/ensDbClientimports from@/lib/ensdb/singletonwithdi.context.*across Omnigraph schema + supporting libs/handlers. - Move ENSDb config + client construction into
apps/ensapi/src/di.ts(build config from env on-demand; instantiateEnsDbReaderon first access). - Remove
apps/ensapi/src/lib/ensdb/singleton.tsand update tests/caches to reference DI.
Reviewed changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/ensapi/src/omnigraph-api/schema/resolver.ts | Swap ENSDb/schema access to di.context inside resolver loaders/field resolvers. |
| apps/ensapi/src/omnigraph-api/schema/resolver-records.ts | Use di.context.ensDb for ResolverRecords loader. |
| apps/ensapi/src/omnigraph-api/schema/resolver-permissions-user.ts | Update PermissionsUser typing to reference DI-provided schema types. |
| apps/ensapi/src/omnigraph-api/schema/renewal.ts | Use di.context.ensDb for Renewal loader. |
| apps/ensapi/src/omnigraph-api/schema/registry.ts | Use DI for registry loading + subregistry-domain connection queries. |
| apps/ensapi/src/omnigraph-api/schema/registry-permissions-user.ts | Update PermissionsUser typing to reference DI-provided schema types. |
| apps/ensapi/src/omnigraph-api/schema/registration.ts | Use DI for registration loading + renewals queries. |
| apps/ensapi/src/omnigraph-api/schema/query.ts | Use DI for “dev method” connections (allDomains/resolvers/registrations). |
| apps/ensapi/src/omnigraph-api/schema/permissions.ts | Use DI for Permissions/Resource/User loaders + event connections. |
| apps/ensapi/src/omnigraph-api/schema/label.ts | Update Label typing to reference DI-provided schema types. |
| apps/ensapi/src/omnigraph-api/schema/event.ts | Use di.context.ensDb for Event loader. |
| apps/ensapi/src/omnigraph-api/schema/domain.ts | Use DI for domain loading + registrations/events/permissions queries. |
| apps/ensapi/src/omnigraph-api/schema/account.ts | Use DI for account loading + permissions queries. |
| apps/ensapi/src/omnigraph-api/lib/get-latest-registration.ts | Read ENSDb via DI inside helper. |
| apps/ensapi/src/omnigraph-api/lib/get-domain-resolver.ts | Read ENSDb via DI inside helper. |
| apps/ensapi/src/omnigraph-api/lib/get-domain-by-interpreted-name.ts | Read ENSDb/schema via DI before executing recursive SQL. |
| apps/ensapi/src/omnigraph-api/lib/find-events/find-events-resolver.ts | Replace ENSDb/schema singleton usage with DI throughout event query builder. |
| apps/ensapi/src/omnigraph-api/lib/find-domains/find-domains-resolver.ts | Replace ENSDb/schema singleton usage with DI throughout domain query builder. |
| apps/ensapi/src/omnigraph-api/lib/find-domains/find-domains-resolver-helpers.ts | Replace schema singleton usage with DI in ordering/cursor helpers. |
| apps/ensapi/src/omnigraph-api/context.ts | Use DI for DataLoader query against ENSDb/schema. |
| apps/ensapi/src/lib/registrar-actions/find-registrar-actions.ts | Replace ENSDb/schema singleton usage with DI throughout query building. |
| apps/ensapi/src/lib/protocol-acceleration/get-records-from-index.ts | Replace ENSDb singleton usage with DI. |
| apps/ensapi/src/lib/protocol-acceleration/get-primary-name-from-index.ts | Replace ENSDb singleton usage with DI. |
| apps/ensapi/src/lib/protocol-acceleration/find-resolver.ts | Replace ENSDb singleton usage with DI in index-backed resolver lookup. |
| apps/ensapi/src/lib/name-tokens/find-name-tokens-for-domain.ts | Replace ENSDb/schema singleton usage with DI in typed select + query. |
| apps/ensapi/src/lib/ensdb/singleton.ts | Remove ENSApi ENSDb singleton module. |
| apps/ensapi/src/lib/ensanalytics/referrer-leaderboard/database.ts | Replace ENSDb/schema singleton usage with DI for referral queries. |
| apps/ensapi/src/handlers/ensapi-probes/ensapi-probes-api.ts | Use di.context.ensDbClient for health/readiness probes. |
| apps/ensapi/src/di.ts | Move ENSDb config + EnsDbReader instantiation into DI context getters. |
| apps/ensapi/src/config/ensdb-config.ts | Tighten env typing for ENSDb config builder to EnsDbEnvironment. |
| apps/ensapi/src/config/config.singleton.test.ts | Update singleton bootstrap tests to initialize/use DI container. |
| apps/ensapi/src/config/config.schema.test.ts | Remove ENSDb singleton mocks; adjust imports accordingly. |
| apps/ensapi/src/cache/stack-info.cache.ts | Switch to dynamic DI import + di.context.ensDbClient for metadata reads. |
| apps/ensapi/src/cache/indexing-status.cache.ts | Switch to dynamic DI import + di.context.ensDbClient for metadata reads. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… `@/di` imports Mechanical updates only
d943075 to
3fa3adb
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/ensapi/src/cache/indexing-status.cache.ts`:
- Around line 32-37: The dynamic import of "`@/di`" and the subsequent access to
di.context/ensDbClient occur before the existing try block so any failures skip
the loader’s structured error logging; move the import("`@/di`") and the const {
ensDbClient } = di.context into the existing try block (or wrap them in their
own try/catch that forwards errors to the same loader error handler) so import
rejections and missing context errors are caught and logged by the loader’s
structured error path, ensuring the same error handling used elsewhere in this
module.
In `@apps/ensapi/src/cache/stack-info.cache.ts`:
- Around line 43-48: Move the dynamic DI import and context access into the
existing try block so any failures are caught and routed through the function's
error logging; specifically, relocate the await import("`@/di`").then((mod) =>
mod.default) and the subsequent const { ensDbClient } = di.context into the try
that wraps the cache load logic (the block that currently catches and logs load
errors) so that errors resolving di or accessing ensDbClient are logged like
other load failures.
🪄 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: 01efee16-ee34-4955-be6a-7db3525f8067
📒 Files selected for processing (34)
apps/ensapi/src/cache/indexing-status.cache.tsapps/ensapi/src/cache/stack-info.cache.tsapps/ensapi/src/config/config.schema.test.tsapps/ensapi/src/config/config.singleton.test.tsapps/ensapi/src/config/ensdb-config.tsapps/ensapi/src/di.tsapps/ensapi/src/handlers/ensapi-probes/ensapi-probes-api.tsapps/ensapi/src/lib/ensanalytics/referrer-leaderboard/database.tsapps/ensapi/src/lib/ensdb/singleton.tsapps/ensapi/src/lib/name-tokens/find-name-tokens-for-domain.tsapps/ensapi/src/lib/protocol-acceleration/find-resolver.tsapps/ensapi/src/lib/protocol-acceleration/get-primary-name-from-index.tsapps/ensapi/src/lib/protocol-acceleration/get-records-from-index.tsapps/ensapi/src/lib/registrar-actions/find-registrar-actions.tsapps/ensapi/src/omnigraph-api/context.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/get-domain-by-interpreted-name.tsapps/ensapi/src/omnigraph-api/lib/get-domain-resolver.tsapps/ensapi/src/omnigraph-api/lib/get-latest-registration.tsapps/ensapi/src/omnigraph-api/schema/account.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/permissions.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.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.ts
💤 Files with no reviewable changes (1)
- apps/ensapi/src/lib/ensdb/singleton.ts
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 34 out of 34 changed files in this pull request and generated 7 comments.
Comments suppressed due to low confidence (1)
apps/ensapi/src/config/config.singleton.test.ts:37
- These tests call
di.init()withoutawait, butinit()is now async. This will cause unhandled Promise rejections and race withafterEach(di.destroy()). Also,di.init()now performs ENSDb/RPC connectivity checks; without stubbing/mocking those, the suite will try to hit real services and become flaky. Awaitdi.init()and mock the health checks (or test context construction without runninginit()).
it("constructs EnsDbReader from real env wiring without errors", async () => {
di.init();
const { ensDbClient, ensDb, ensIndexerSchema } = di.context;
expect(ensDbClient.ensIndexerSchemaName).toBe(VALID_ENSINDEXER_SCHEMA_NAME);
expect(ensDb).toBeDefined();
expect(ensIndexerSchema).toBeDefined();
}, 10_000);
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/ensapi/src/config/config.singleton.test.ts`:
- Around line 32-33: The test calls di.init() without awaiting it and then
accesses di.context, which can leak unhandled rejections; update the tests to
await the returned promise from di.init() (e.g., await di.init()) and for the
ENSDB_URL/ENSINDEXER_SCHEMA_NAME negative cases assert that await di.init()
rejects (using your test runner's reject assertion) instead of accessing
di.context. Ensure all tests that rely on di.context run only after awaiting
di.init(), and replace direct di.context.ensDbClient access in failure scenarios
with assertions on the rejected promise from di.init().
In `@apps/ensapi/src/di.ts`:
- Around line 258-270: The DI init path leaves this._context populated if
connectivity/cache/RPC checks fail; update the init() flow (after calling
loadContext()) to clear this._context on any caught error to avoid a
half-initialized container: in each catch block that wraps ENSDb/RPC/cache/other
checks, set this._context = undefined (or call a single
unloadContext/clearContext helper) before re-throwing so retries start from a
clean state; apply the same fix to the similar catch blocks referenced (lines
around 272-286 and 290-299) and centralize the cleanup where possible.
🪄 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: 451e5d54-43d1-49ce-907d-76637cf91475
📒 Files selected for processing (34)
apps/ensapi/src/cache/indexing-status.cache.tsapps/ensapi/src/cache/stack-info.cache.tsapps/ensapi/src/config/config.schema.test.tsapps/ensapi/src/config/config.singleton.test.tsapps/ensapi/src/config/ensdb-config.tsapps/ensapi/src/di.tsapps/ensapi/src/handlers/ensapi-probes/ensapi-probes-api.tsapps/ensapi/src/lib/ensanalytics/referrer-leaderboard/database.tsapps/ensapi/src/lib/ensdb/singleton.tsapps/ensapi/src/lib/name-tokens/find-name-tokens-for-domain.tsapps/ensapi/src/lib/protocol-acceleration/find-resolver.tsapps/ensapi/src/lib/protocol-acceleration/get-primary-name-from-index.tsapps/ensapi/src/lib/protocol-acceleration/get-records-from-index.tsapps/ensapi/src/lib/registrar-actions/find-registrar-actions.tsapps/ensapi/src/omnigraph-api/context.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/get-domain-by-interpreted-name.tsapps/ensapi/src/omnigraph-api/lib/get-domain-resolver.tsapps/ensapi/src/omnigraph-api/lib/get-latest-registration.tsapps/ensapi/src/omnigraph-api/schema/account.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/permissions.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.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.ts
💤 Files with no reviewable changes (1)
- apps/ensapi/src/lib/ensdb/singleton.ts
… allows cleaning up database connection resources when the connection is no longer needed.
Lite PR
Tip: Review docs on the ENSNode PR process
Summary
@/lib/ensdb/singletonwith the lazily-evaluated counterparts from@/di.Why
lazy/lazyProxyprimitives from ENSNode codebase.Testing
Notes for Reviewer (Optional)
Pre-Review Checklist (Blocking)