Extend ensdb-sdk with EnsDbConfig data model#1868
Conversation
We do not want the users of ENSDb SDK to have to worry about ENSNode SDK dependency on their end
Added `EnsDbConfig` type and `validateEnsDbConfig` function
…dation for the `EnsDbConfig` data model.
Defined `buildEnsDbConfigFromEnvironment` function to source ENSDb config from env vars.
Defined `buildEnsDbConfigFromEnvironment` function to source ENSDb config from env vars.
🦋 Changeset detectedLatest commit: caab8f0 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
|
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 23 minutes and 26 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)
📝 WalkthroughWalkthroughCentralized ENSDb configuration validation into the Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application Module
participant ConfigMod as App Config Module
participant LazyProxy as lazyProxy<EnsDbConfig>
participant Env as process.env
participant SDK as ensdb-sdk.validateEnsDbConfig
App->>ConfigMod: import ensDbConfig
ConfigMod->>LazyProxy: create lazyProxy(for builder)
App->>LazyProxy: access ensDbUrl / ensIndexerSchemaName
activate LazyProxy
LazyProxy->>Env: read ENSDB_URL, ENSINDEXER_SCHEMA_NAME
LazyProxy->>SDK: validateEnsDbConfig(unvalidated)
SDK->>SDK: parse via Zod schemas
alt validation succeeds
SDK-->>LazyProxy: return validated EnsDbConfig
LazyProxy-->>App: return requested values
else validation fails
SDK-->>LazyProxy: throw Error (prettified)
LazyProxy->>ConfigMod: log error
LazyProxy->>ConfigMod: process.exit(1)
end
deactivate LazyProxy
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 consolidates the Key changes:
Confidence Score: 5/5Safe to merge — no runtime bugs or data-integrity issues found; the only finding is a P2 test mock design smell. All previous P1 concerns (duplicate schema definitions, missing deduplication) are addressed. The remaining finding (double-validation in mock getters) is a test-only style issue that does not affect production behaviour and does not reduce the score below 5. apps/ensindexer/src/lib/test/mockConfig.ts — double-validation getter pattern worth tidying for long-term test maintainability. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
subgraph ensdb-sdk
EC[EnsDbConfig interface]
ZS[EnsDbConfigSchema Zod schema]
VF[validateEnsDbConfig throws on failure]
ZS --> VF
EC --> VF
end
subgraph ensapi
AEC[ensdb-config.ts buildEnsDbConfigFromEnvironment wrapped in lazyProxy]
ACS[config.schema.ts EnsApiConfigSchema .transform includeEnsDbConfig]
AEC --> ACS
end
subgraph ensindexer
IEC[ensdb-config.ts buildEnsDbConfigFromEnvironment eager module-level init]
ICS[config.schema.ts ENSIndexerConfigSchema .transform includeEnsDbConfig]
IEC --> ICS
end
VF --> AEC
VF --> IEC
process.env --> AEC
process.env --> IEC
Reviews (2): Last reviewed commit: "Apply AI PR feedback" | Re-trigger Greptile |
There was a problem hiding this comment.
Pull request overview
This PR centralizes ENSDb configuration typing and validation into @ensnode/ensdb-sdk, then updates ENSIndexer/ENSApi to consume that shared validation while removing duplicated app-local schemas and direct dependencies.
Changes:
- Added
EnsDbConfig+ Zod validation (EnsDbConfigSchema,validateEnsDbConfig) topackages/ensdb-sdk. - Replaced app-local ENSDb config schema/validation in ENSIndexer and ENSApi with the SDK validator.
- Updated dependencies/lockfile to move
pg-connection-string/zodusage intoensdb-sdkand remove them from apps.
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Lockfile updates for dependency moves (notably Zod v4 usage and pg-connection-string relocation). |
| packages/ensdb-sdk/src/client/zod-schemas/ensdb-config.ts | Introduces Zod schemas for ENSDb URL + schema name and config object. |
| packages/ensdb-sdk/src/client/validate/ensdb-config.ts | Adds validateEnsDbConfig helper that throws formatted errors on invalid config. |
| packages/ensdb-sdk/src/client/index.ts | Exposes new client exports (config type + validator). |
| packages/ensdb-sdk/src/client/ensdb-config.ts | Adds EnsDbConfig interface to the SDK. |
| packages/ensdb-sdk/package.json | Adds runtime deps (pg-connection-string, zod) required by the new validator/schemas. |
| apps/ensindexer/src/lib/subgraph/ids.test.ts | Updates test setup to include ENSDb config mocking helper. |
| apps/ensindexer/src/lib/graphnode-helpers.test.ts | Updates test setup to include ENSDb config mocking helper. |
| apps/ensindexer/src/lib/test/mockConfig.ts | Adds setupEnsDbConfigMock helper for tests. |
| apps/ensindexer/src/config/ensdb-config.ts | New ENSIndexer ENSDb config builder using validateEnsDbConfig. |
| apps/ensindexer/src/config/config.test.ts | Mocks ENSDb config module and updates assertions to new error messages. |
| apps/ensindexer/src/config/config.schema.ts | Removes ENSDb fields from schema and injects validated ENSDb config via transform. |
| apps/ensindexer/package.json | Removes pg-connection-string from app deps (now provided via SDK). |
| apps/ensapi/src/lib/ensdb/singleton.ts | Updates import path for ENSDb config builder. |
| apps/ensapi/src/config/ensdb-config.ts | Replaces app-local schema with SDK validator + lazy default export. |
| apps/ensapi/src/config/ensdb-config.schema.ts | Deletes old ENSApi-specific ENSDb config schema/validator module. |
| apps/ensapi/src/config/config.schema.ts | Injects ENSDb config via transform instead of extending app-local schema. |
| apps/ensapi/src/config/config.schema.test.ts | Mocks ENSDb config module to keep schema tests isolated from env. |
| apps/ensapi/package.json | Removes pg-connection-string from app deps (now provided via SDK). |
| .changeset/fluffy-forks-film.md | Declares a minor bump for @ensnode/ensdb-sdk due to new public validation API. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/ensindexer/src/config/config.schema.ts`:
- Around line 99-106: The transform includeEnsDbConfig currently reads the
process-global ensDbConfig (default-exported from `@/config/ensdb-config`) causing
stale values; instead, inside buildConfigFromEnvironment(_env) validate and
build an ENSDb config from the provided _env (using the same logic as
buildEnsDbConfigFromEnvironment but with _env) and merge those fields into the
returned config (ensDbUrl and ensIndexerSchemaName) rather than referencing the
module-level ensDbConfig singleton; update buildConfigFromEnvironment to call
the ENSDb build/validation with the passed _env (checking _env.ENSDB_URL and
_env.ENSINDEXER_SCHEMA_NAME) and then spread those validated values into the
final config.
In `@apps/ensindexer/src/config/ensdb-config.ts`:
- Around line 9-22: The ENS config builder here is app-specific and
side-effectful; extract the pure builder into the SDK as
buildEnsDbConfig(ensDbUrl, ensIndexerSchemaName) that simply calls
validateEnsDbConfig({ensDbUrl, ensIndexerSchemaName}) and returns the validated
EnsDbConfig, then update buildEnsDbConfigFromEnvironment to only read env
(ENSDB_URL, ENSINDEXER_SCHEMA_NAME), call the new SDK buildEnsDbConfig, and keep
the try/catch that logs the error and exits; reference validateEnsDbConfig,
buildEnsDbConfig (new SDK export), and buildEnsDbConfigFromEnvironment to locate
and change the code.
In `@packages/ensdb-sdk/src/client/ensdb-config.ts`:
- Around line 5-8: Update the comment block that starts "PostgreSQL connection
string for ENSDb." to document both accepted URI schemes by stating that ENSDb
accepts either "postgresql://" or the shorter "postgres://" and show the
expected format (e.g., postgres[s]://username:password@host:port/database).
Modify the docstring text near that comment so consumers know both schemes are
supported by the shared validator.
In `@packages/ensdb-sdk/src/client/validate/ensdb-config.ts`:
- Around line 8-14: Remove the redundant JSDoc `@returns` tag from the "Validate
ENSDb config" comment block (the JSDoc above the function that validates ENSDb
config); keep the summary and the `@throws` line but delete the `@returns` line that
merely restates the summary so the JSDoc is concise and follows the guideline.
In `@packages/ensdb-sdk/src/client/zod-schemas/ensdb-config.ts`:
- Around line 22-29: The schema EnsIndexerSchemaNameSchema is currently
file-local; export it so downstream consumers can reuse it from the SDK. Modify
the declaration of EnsIndexerSchemaNameSchema to be an exported symbol (export
const EnsIndexerSchemaNameSchema) and ensure any other modules that import it
use the exported name; preserve the existing z.string().trim().nonempty(...)
validation and error messages when exporting.
🪄 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: 0d832320-c8a4-4026-9c59-8159cb4f56fe
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (19)
.changeset/fluffy-forks-film.mdapps/ensapi/package.jsonapps/ensapi/src/config/config.schema.test.tsapps/ensapi/src/config/config.schema.tsapps/ensapi/src/config/ensdb-config.schema.tsapps/ensapi/src/config/ensdb-config.tsapps/ensapi/src/lib/ensdb/singleton.tsapps/ensindexer/package.jsonapps/ensindexer/src/config/config.schema.tsapps/ensindexer/src/config/config.test.tsapps/ensindexer/src/config/ensdb-config.tsapps/ensindexer/src/lib/__test__/mockConfig.tsapps/ensindexer/src/lib/graphnode-helpers.test.tsapps/ensindexer/src/lib/subgraph/ids.test.tspackages/ensdb-sdk/package.jsonpackages/ensdb-sdk/src/client/ensdb-config.tspackages/ensdb-sdk/src/client/index.tspackages/ensdb-sdk/src/client/validate/ensdb-config.tspackages/ensdb-sdk/src/client/zod-schemas/ensdb-config.ts
💤 Files with no reviewable changes (3)
- apps/ensapi/package.json
- apps/ensindexer/package.json
- apps/ensapi/src/config/ensdb-config.schema.ts
|
@greptile review |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/ensindexer/src/config/types.ts`:
- Line 4: Import RpcConfig using a type-only import since it's only referenced
in JSDoc and not used at runtime: change the import line that currently brings
in RpcConfig and RpcConfigs to import RpcConfig with the `type` modifier (e.g.,
`type RpcConfig`) while keeping RpcConfigs as-is, ensuring JSDoc {`@link`
RpcConfig} still resolves but no runtime value is imported.
In `@packages/ensdb-sdk/src/client/ensdb-config.ts`:
- Around line 4-20: The interface EnsDbConfig currently documents invariants on
its fields ensDbUrl and ensIndexerSchemaName; extract those invariants into
dedicated type aliases (e.g., PostgresConnectionString and PostgresSchemaName)
with the exact documentation once, then update EnsDbConfig to use those aliases
for ensDbUrl and ensIndexerSchemaName respectively so the invariant docs live on
the type aliases rather than the interface properties.
🪄 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: f6e58d09-c1c8-4fc3-84a5-7452f49b16ec
📒 Files selected for processing (5)
apps/ensindexer/src/config/types.tspackages/ensdb-sdk/src/client/ensdb-config.tspackages/ensnode-sdk/src/ensindexer/config/zod-schemas.test.tspackages/ensnode-sdk/src/shared/config/types.tspackages/ensnode-sdk/src/shared/config/zod-schemas.ts
💤 Files with no reviewable changes (2)
- packages/ensnode-sdk/src/ensindexer/config/zod-schemas.test.ts
- packages/ensnode-sdk/src/shared/config/zod-schemas.ts
# Conflicts: # apps/ensindexer/src/lib/graphnode-helpers.test.ts
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 24 changed files in this pull request and generated 6 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
packages/ensnode-sdk/src/ensindexer/config/zod-schemas.test.ts:33
- The tests for
makeEnsIndexerSchemaNameSchemaparsing and its custom value-label error messages were removed, butmakeEnsIndexerSchemaNameSchemais still used bymakeEnsIndexerPublicConfigSchema/makeSerializedEnsIndexerPublicConfigSchema. Add a focused test case to keep coverage for schema-name validation and ensure error messages remain stable.
describe("Parsing", () => {
it("can parse a list of plugin name values", () => {
expect(
makePluginsListSchema().parse([
`${PluginName.Subgraph}`,
`${PluginName.Registrars}`,
`${PluginName.ProtocolAcceleration}`,
]),
).toStrictEqual([
PluginName.Subgraph,
PluginName.Registrars,
PluginName.ProtocolAcceleration,
]);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
cf4067a to
d10db1f
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 24 changed files in this pull request and generated 4 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
apps/ensindexer/src/lib/subgraph/ids.test.ts:16
setupEnsDbConfigMock()/setupConfigMock()are called before laterimportstatements in source order, but static ESM imports are evaluated before the module body runs. As a result,./ids(which imports@/config) will be loaded before the mocks are registered, so the real config module can execute and read env/exit. To ensure the mocks apply, declare the neededvi.mock(...)calls at top-level in this test file (so Vitest can hoist them), or switch to dynamically importing./idsafter registering the mocks.
import {
resetMockConfig,
setupConfigMock,
setupEnsDbConfigMock,
updateMockConfig,
} from "@/lib/__test__/mockConfig";
// Setup mocks before any imports that depend on them
setupEnsDbConfigMock();
setupConfigMock();
import { labelhash, namehash, zeroAddress } from "viem";
import { makeEventId, makeRegistrationId, makeResolverId } from "./ids";
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import { setupConfigMock, setupEnsDbConfigMock } from "@/lib/__test__/mockConfig"; | ||
| import "@/lib/__test__/mockLogger"; | ||
|
|
||
| setupConfigMock(); // setup config mock before importing dependent modules | ||
| // Setup mocks before any imports that depend on them | ||
| setupEnsDbConfigMock(); | ||
| setupConfigMock(); | ||
|
|
There was a problem hiding this comment.
Same issue as ids.test.ts: setupEnsDbConfigMock() / setupConfigMock() run in the module body, but import declarations (including ./graphnode-helpers) execute first. This can load modules that depend on @/config / @/config/ensdb-config before the mocks are active. Prefer top-level vi.mock(...) declarations (hoisted) or use dynamic await import(...) after registering mocks.
| --- | ||
| "@ensnode/ensdb-sdk": minor | ||
| --- | ||
|
|
||
| Added `validateEnsDbConfig` function to support validation for the `EnsDbConfig` data model. |
There was a problem hiding this comment.
This changeset describes only adding validateEnsDbConfig, but the PR also introduces the EnsDbConfig data model and its schema. Consider updating the changeset text to mention the new EnsDbConfig export as part of the minor release so consumers understand what was added.
| export function setupEnsDbConfigMock() { | ||
| vi.mock("@/config/ensdb-config", async () => { | ||
| const { validateEnsDbConfig } = | ||
| await vi.importActual<typeof import("@ensnode/ensdb-sdk")>("@ensnode/ensdb-sdk"); | ||
|
|
||
| // Default test values when env vars are not explicitly set | ||
| const defaultEnsDbUrl = "postgresql://postgres:postgres@localhost:5432/postgres"; | ||
| const defaultEnsIndexerSchemaName = "ensindexer_0"; | ||
|
|
There was a problem hiding this comment.
setupEnsDbConfigMock() registers a mock for @/config/ensdb-config, but it can’t protect against modules that have already imported @/config/ensdb-config as a side effect of importing this mockConfig.ts module (e.g., via the module-scope _defaultMockConfig = buildConfigFromEnvironment(...)). If any test imports mockConfig.ts before calling this helper, the real ensdb-config module may already have run (and potentially process.exit(1)), making the mock ineffective. Consider restructuring the test utilities so no config-building happens at module scope, or ensure @/config/ensdb-config is mocked at top-level in each test file before importing mockConfig.ts.
| // include the validated ENSDb config values in the parsed EnsIndexerConfig | ||
| ensDbUrl: ensDbConfig.ensDbUrl, | ||
| ensIndexerSchemaName: ensDbConfig.ensIndexerSchemaName, |
There was a problem hiding this comment.
This comment says “parsed EnsApiConfig” but this is the ENSIndexer config builder. Please rename to “parsed EnsIndexerConfig” (and similarly adjust any other EnsApi references) to avoid confusion.
Lite PR
Tip: Review docs on the ENSNode PR process
Summary
EnsDbConfigdata model andvalidateEnsDbConfigfunction to ENSDb SDKEnsDbConfigdata model into ENSApi and ENSIndexerEnsDbConfigobject is built for each app based on the environment configurationvalidateEnsDbConfigto enforce invariantsEnsDbConfigobject is merged into each app's config object (EnsApiConfig/EnsIndexerConfig)includeEnsDbConfigfunctionWhy
Testing
Notes for Reviewer (Optional)
Pre-Review Checklist (Blocking)