refactor: move replaceBigInts + toJson into ensnode-sdk#1978
Merged
Conversation
Adds replaceBigInts (sourced from @ponder/utils) and toJson helpers to @ensnode/ensnode-sdk. toJson now takes an options object with pretty defaulting to false. Migrates all in-repo call sites and drops @ponder/utils from ensapi. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: f2aefd5 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
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
💤 Files with no reviewable changes (2)
📝 WalkthroughWalkthroughAdds bigint-handling utilities to Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 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 docstrings
🧪 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 |
Contributor
There was a problem hiding this comment.
Pull request overview
Moves bigint-safe JSON serialization utilities into @ensnode/ensnode-sdk, migrates in-repo usages to the SDK, and removes the now-unnecessary @ponder/utils dependency from ensapi.
Changes:
- Add
replaceBigIntsandtoJsonhelpers to@ensnode/ensnode-sdkand export them from the SDK entrypoint. - Update
ensindexer/ensapicall sites to use the SDK helpers; delete theensindexerone-off helper. - Drop
@ponder/utilsfromapps/ensapi(and update lockfile) since it’s no longer needed.
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Removes @ponder/utils from the lockfile graph for ensapi. |
| packages/ensnode-sdk/src/shared/to-json.ts | Adds new toJson helper with { pretty?: boolean } option. |
| packages/ensnode-sdk/src/shared/replace-bigints.ts | Adds replaceBigInts utility (sourced from @ponder/utils with attribution). |
| packages/ensnode-sdk/src/shared/replace-bigints.test.ts | Adds unit tests for replaceBigInts behavior and typing. |
| packages/ensnode-sdk/src/index.ts | Exports replaceBigInts and toJson from the SDK public surface. |
| apps/ensindexer/src/plugins/ensv2/handlers/ensv2/ETHRegistrar.ts | Switches to SDK toJson and opts into pretty output for invariant errors. |
| apps/ensindexer/src/plugins/ensv2/handlers/ensv2/ENSv2Registry.ts | Switches to SDK toJson and opts into pretty output for sanity/invariant errors. |
| apps/ensindexer/src/plugins/ensv2/handlers/ensv1/RegistrarController.ts | Switches to SDK toJson and updates pretty formatting call sites. |
| apps/ensindexer/src/plugins/ensv2/handlers/ensv1/NameWrapper.ts | Switches to SDK toJson and updates pretty formatting call sites. |
| apps/ensindexer/src/plugins/ensv2/handlers/ensv1/BaseRegistrar.ts | Switches to SDK toJson and updates pretty formatting call sites. |
| apps/ensindexer/src/lib/managed-names.ts | Switches to SDK toJson for error messages. |
| apps/ensindexer/src/lib/json-stringify-with-bigints.ts | Deletes local helper now replaced by SDK toJson. |
| apps/ensindexer/src/lib/heal-addr-reverse-subname-label.ts | Switches to SDK toJson and opts into pretty output for invariant errors. |
| apps/ensapi/src/lib/resolution/forward-resolution.ts | Replaces ad-hoc replaceBigInts+JSON.stringify with SDK toJson for tracing attributes/errors. |
| apps/ensapi/src/handlers/api/resolution/resolution-api.ts | Imports replaceBigInts from SDK and uses it for JSON response serialization. |
| apps/ensapi/package.json | Drops @ponder/utils dependency. |
| .changeset/replace-bigints-in-sdk.md | Changeset documenting the new SDK exports and toJson API shape/default. |
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: 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 `@packages/ensnode-sdk/src/index.ts`:
- Around line 31-34: The current toJson implementation pre-walks values with
replaceBigInts which strips built-in toJSON behavior (e.g., Date) and the
ReplaceBigInts type uses an odd mutable-tuple inference pattern; update toJson
to call JSON.stringify with a replacer that converts bigint to string so native
toJSON is preserved (refer to the toJson function), and simplify the
ReplaceBigInts type to a straightforward recursive mapped type that maps bigint
to string while recursing into arrays/objects (refer to the ReplaceBigInts type
and replaceBigInts helper) and adjust the replacer function signature to match
this simpler type.
In `@packages/ensnode-sdk/src/shared/replace-bigints.test.ts`:
- Line 11: The test currently calls expectTypeOf<Hex>(out) without a chained
matcher so no assertion runs; update the assertion to chain a matcher (e.g.,
expectTypeOf(out).toEqualTypeOf<Hex>() or
expectTypeOf(out).toMatchTypeOf<Hex>()) to perform the actual type check, and
for the non-const array case returned by replaceBigInts([5n], ...) use
.toMatchTypeOf() (or change the input to a const/readonly tuple) to avoid a
mismatch between mutable inferred type and the readonly tuple expectation;
locate the test by the expectTypeOf, Hex, out identifiers and the replaceBigInts
call to apply this change.
🪄 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: 10a2cfd9-d568-4fd4-84eb-7e3431022f3a
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (16)
.changeset/replace-bigints-in-sdk.mdapps/ensapi/package.jsonapps/ensapi/src/handlers/api/resolution/resolution-api.tsapps/ensapi/src/lib/resolution/forward-resolution.tsapps/ensindexer/src/lib/heal-addr-reverse-subname-label.tsapps/ensindexer/src/lib/json-stringify-with-bigints.tsapps/ensindexer/src/lib/managed-names.tsapps/ensindexer/src/plugins/ensv2/handlers/ensv1/BaseRegistrar.tsapps/ensindexer/src/plugins/ensv2/handlers/ensv1/NameWrapper.tsapps/ensindexer/src/plugins/ensv2/handlers/ensv1/RegistrarController.tsapps/ensindexer/src/plugins/ensv2/handlers/ensv2/ENSv2Registry.tsapps/ensindexer/src/plugins/ensv2/handlers/ensv2/ETHRegistrar.tspackages/ensnode-sdk/src/index.tspackages/ensnode-sdk/src/shared/replace-bigints.test.tspackages/ensnode-sdk/src/shared/replace-bigints.tspackages/ensnode-sdk/src/shared/to-json.ts
💤 Files with no reviewable changes (2)
- apps/ensapi/package.json
- apps/ensindexer/src/lib/json-stringify-with-bigints.ts
Covers bigint serialization, compact-by-default, pretty option, nested structures, and primitive passthrough. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 18 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
datasources is the only runtime consumer of @ponder/utils (mergeAbis); moving it to peerDependencies avoids double-installation when consumers also bring ponder-utils transitively. Inlines the version spec in datasources' package.json and removes it from the workspace catalog since no other package needs it. Drops the vestigial @ponder/utils listing from ensadmin (no source usage). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 21 changed files in this pull request and generated 2 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
- ReplaceBigInts now matches readonly unknown[] instead of only readonly [] so non-empty readonly tuples get tuple-mapped types instead of falling through to the object branch. - toJson uses JSON.stringify's replacer callback instead of pre-walking with replaceBigInts, preserving native toJSON behavior (Date, etc.). - Chain .toEqualTypeOf() on expectTypeOf assertions so they actually run. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Member
Author
|
@greptile re-review |
Contributor
Greptile SummaryThis PR moves Confidence Score: 5/5Safe to merge — purely a refactor with no runtime behavior changes, all call sites updated correctly, and solid test coverage. No P0 or P1 issues found. The implementation is correct: toJson uses JSON.stringify's native replacer (preserving toJSON), the default flip from pretty=true to compact is handled at every call site, replaceBigInts is vendored verbatim with attribution, and all dependency removals are clean. Tests are thorough. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["ensapi / ensindexer\ncall sites"] -->|"toJson(val, { pretty? })"| B["@ensnode/ensnode-sdk\ntoJson"]
A -->|"replaceBigInts(val, fn)"| C["@ensnode/ensnode-sdk\nreplaceBigInts"]
B --> D["JSON.stringify\nwith replacer callback\n(bigint → String)"]
C --> E["Recursive object\ntraversal (vendored\nfrom @ponder/utils)"]
F["@ensnode/datasources"] -->|"@ponder/utils ^0.2.18\n(only runtime consumer)"| G["@ponder/utils\n(external)"]
H["pnpm-workspace catalog"] -.->|"entry removed"| G
Reviews (1): Last reviewed commit: "fix(ensnode-sdk): address review feedbac..." | Re-trigger Greptile |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/ensnode-sdk/src/shared/replace-bigints.ts`:
- Around line 4-18: The exported ReplaceBigInts type doesn't match runtime
replaceBigInts: _ReplaceBigInts treats only tuple shapes and doesn't recurse
into nested elements, causing bigint[] to become readonly [] and nested bigints
to remain untransformed; fix by updating _ReplaceBigInts/ReplaceBigInts so
arrays (both tuple and general arrays) map element types recursively (i.e., when
arr is a tuple keep tuple recursion but for arr extends readonly (infer T)[]
produce readonly ReplaceBigInts<T, type>[]), and change the tuple branch to use
ReplaceBigInts<first, type> instead of first extends bigint ? type : first; then
add the suggested type assertions in the test using
expectTypeOf(replaceBigInts(...)).toEqualTypeOf<...>() to verify bigint[] ->
readonly string[] and deep nested shapes are correctly transformed.
🪄 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: ad4bbe05-1f19-4f24-b6cc-18143903f624
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
apps/ensadmin/package.jsonpackages/datasources/package.jsonpackages/ensnode-sdk/src/shared/replace-bigints.test.tspackages/ensnode-sdk/src/shared/replace-bigints.tspackages/ensnode-sdk/src/shared/to-json.test.tspackages/ensnode-sdk/src/shared/to-json.tspnpm-workspace.yaml
💤 Files with no reviewable changes (2)
- pnpm-workspace.yaml
- apps/ensadmin/package.json
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 22 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ecursion - General arrays (e.g. bigint[]) now produce ReplaceBigInts<T, U>[] instead of collapsing to readonly []. - Tuple branch now recurses via ReplaceBigInts<first, type> so nested bigints inside tuple elements are transformed. - Output is mutable (arrays via map, objects via Object.fromEntries — both produce mutable values at runtime). - Documented that the helper is designed for plain JSON-like values; non-plain objects (Date, Map, class instances) are not preserved. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
tk-o
reviewed
Apr 23, 2026
| }, | ||
| "dependencies": { | ||
| "@ponder/utils": "catalog:" | ||
| "@ponder/utils": "^0.2.18" |
Member
There was a problem hiding this comment.
I note how we still rely on @ponder/util dependency her to support using mergeAbis function inside the datasources package.
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.
Summary
replaceBigInts(sourced verbatim from@ponder/utils, with upstream attribution) andtoJsonto@ensnode/ensnode-sdk, with unit tests for both.apps/ensindexer/src/lib/json-stringify-with-bigints.tshelper.@ponder/utilsfromapps/ensapi(no longer needed).@ponder/utilslisting fromapps/ensadmin(no source usage; gets it transitively via datasources).@ponder/utilsversion spec on@ensnode/datasources(its only runtime consumer) and removed the entry from the workspace catalog.Why
replaceBigIntswas pulling@ponder/utilsinto ensapi andponderinto a trivial ensindexer helper for a generic bigint-in-JSON utility. owning it in the sdk lets consumers use it without the ponder dependency, and unifies the duplicatedtoJsonpattern across ensindexer/ensapi.also tightened
toJson's api:toJson(value, { pretty?: boolean }), default compact. currently-pretty call sites (ensindexer invariant errors + one thrown ensapi error) pass{ pretty: true }; span-attribute sites stay compact.datasources is now the only runtime consumer of
@ponder/utils, so the version spec lives alongside its sole user.Testing
replaceBigInts(scalar/array/readonly/nested/mixed/null/empty/identity/String/numberToHex) andtoJson(bigint serialization, pretty/compact, nested, primitives).pnpm typecheckacross the monorepo → clean.pnpm lintpnpm test --project @ensnode/datasources --project @ensnode/ensnode-sdk --project ensapi --project ensindexer --project ensadmin→ 802 passed.Notes for Reviewer
toJsondefault flipped from pretty to compact — every currently-pretty site was updated to{ pretty: true }explicitly.@ponder/utilsversion spec moved inline topackages/datasources/package.json(^0.2.18) since datasources is the only consumer; catalog entry removed.Checklist