refactor(ensnode-sdk): move ENSNodeClient into ENSApi module#1636
Conversation
This update ties `ENSNodeClient` to ENSApi.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
📝 WalkthroughWalkthroughReorganizes the SDK API surface: renames exported client class from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4✅ 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 |
There was a problem hiding this comment.
Pull request overview
This PR moves ENSNodeClient and related files (client-error, deployments) from the root level (./client, ./client-error, ./deployments) into the ENSApi module (./ensapi/client, ./ensapi/client-error, ./ensapi/deployments). This is the third PR in a refactoring series (following #1633 and #1635) that reorganizes the ENSNode SDK to support separate API modules for ENSApi and ENSIndexer, enabling future work on issue #1252 which requires creating a dedicated ENSIndexerClient.
Changes:
- Moved ENSNodeClient implementation and related files from root to
./ensapisubdirectory - Updated all internal import paths to reflect the new file locations
- Re-exported client exports through
./ensapimodule to maintain public API compatibility
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ensnode-sdk/src/index.ts | Removed direct exports of client-related modules (now exported via ensapi) |
| packages/ensnode-sdk/src/ensapi/index.ts | Added exports for client, client-error, and deployments modules |
| packages/ensnode-sdk/src/ensapi/deployments.ts | Moved from root level to ensapi subdirectory |
| packages/ensnode-sdk/src/ensapi/deployments.test.ts | Updated import path for ENSNamespaceIds from ./ens to ../ens |
| packages/ensnode-sdk/src/ensapi/client.ts | Updated import paths to reference local modules and relative paths |
| packages/ensnode-sdk/src/ensapi/client.test.ts | Updated all import paths to reflect new file structure |
| packages/ensnode-sdk/src/ensapi/client-error.ts | Updated import path for ErrorResponse from ./ensapi/api to ./api |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tk-o
left a comment
There was a problem hiding this comment.
Self-review completed
| @@ -1,2 +1,5 @@ | |||
| export * from "./api"; | |||
| export { type ClientOptions, ENSNodeClient } from "./client"; | |||
There was a problem hiding this comment.
@lightwalker-eth, I wonder if we should rename ENSNodeClient to ENSApiClient? Of course, we'd keep ENSNodeClient as alias for now. Thoughts?
Greptile SummaryThis PR moves
Confidence Score: 5/5
Important Files Changed
Flowchartflowchart TD
A["@ensnode/ensnode-sdk<br/>(src/index.ts)"] -->|"export * from './ensapi'"| B["src/ensapi/index.ts"]
B -->|"export * from './api'"| C["src/ensapi/api/"]
B -->|"export ENSNodeClient, ClientOptions"| D["src/ensapi/client.ts"]
B -->|"export * from './client-error'"| E["src/ensapi/client-error.ts"]
B -->|"export * from './config'"| F["src/ensapi/config/"]
B -->|"export * from './deployments'"| G["src/ensapi/deployments.ts"]
D -->|imports| C
D -->|imports| E
D -->|imports| G
D -->|"imports ResolverRecordsSelection"| H["src/resolution/"]
E -->|"imports ErrorResponse"| C
style A fill:#e1f5fe
style B fill:#fff9c4
style D fill:#c8e6c9
style E fill:#c8e6c9
style G fill:#c8e6c9
Last reviewed commit: 42db185 |
Keeps deprecated `ENSNodeClient` to be an alias for `ENSApiClient`.
lightwalker-eth
left a comment
There was a problem hiding this comment.
@tk-o Looks good 👍 Please merge when ready
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/ensnode-sdk/src/ensapi/client.test.ts`:
- Line 205: Add a test block verifying the deprecated ENSNodeClient wrapper
still behaves as an alias for ENSApiClient: create an ENSNodeClient instance and
assert it is instanceof ENSApiClient and that its options equal
ENSApiClient.defaultOptions() (use client.getOptions() to fetch options). Place
the new describe/it alongside the existing ENSApiClient tests so the deprecated
wrapper’s construction and basic API are covered by the test suite.
In `@packages/ensnode-sdk/src/ensapi/client.ts`:
- Around line 672-678: Update the JSDoc on the ENSNodeClient declaration to
include a version/timeline and migration guidance: replace the bare
"@deprecated" with something like "@deprecated since vX.Y.Z — scheduled for
removal in vA.B.C; use ENSApiClient instead" and add a short note about
migration (e.g., constructor/signature compatibility) to help consumers; modify
the docblock above the ENSNodeClient class (which extends ENSApiClient)
accordingly so tooling and IDEs surface the deprecation version and removal
target.
Lite PR
Tip: Review docs on the ENSNode PR process
Summary
ENSNodeClienthas been moved into ENSApi moduleWhy
./ensapi/api) and one for ENSIndexer (./ensindexer/api). Each of those separate API modules will be able to use shared Indexing Status data model.ENSNodeClientwhich works with ENSApi. However, to achieve goals of issue ENSApi: Persist ENSIndexerPublicConfig to the database #1252, we'll also needENSIndexerClientthat will use data model distinct fromENSNodeClient.Testing
Notes for Reviewer (Optional)
ENSIndexerClientat./ensindexer/clientand use it while solving for issue ENSApi: Persist ENSIndexerPublicConfig to the database #1252.Pre-Review Checklist (Blocking)