Skip to content

chore(deps): quarterly batch dependency upgrade 2026-Q2#3271

Merged
kuan121 merged 12 commits into
mainfrom
test-consolidate-deps-workflow
Apr 23, 2026
Merged

chore(deps): quarterly batch dependency upgrade 2026-Q2#3271
kuan121 merged 12 commits into
mainfrom
test-consolidate-deps-workflow

Conversation

@kuan121

@kuan121 kuan121 commented Apr 10, 2026

Copy link
Copy Markdown
Collaborator

High Level Overview of Change

Batches 30 open Dependabot dependency upgrade PRs into a single quarterly upgrade. Upgrades 23 packages across the monorepo, skips 5 blocked by peer dependency constraints or migration scope, and classifies 2 as no-ops.

Context of Change

Dependabot PRs accumulate faster than they can be individually reviewed and merged. This batch upgrade applies all compatible upgrades at once, fixes breaking changes introduced by major version bumps, and validates the full test suite against the combined result.

Breaking dependency changes fixed:

  • bignumber.js 9→10: constructor now throws on invalid input instead of returning NaN. Wrapped in try-catch in both xrpl (xrpConversion.ts, quality.ts) and ripple-binary-codec (quality.ts, amount.ts) to preserve existing error behavior.
  • https-proxy-agent 7→9: now ESM-only. Updated Jest config to transpile ESM imports.
  • @scure/base 1→2: stricter Uint8Array generics. Widened type annotations in ripple-binary-codec (amount.ts, uint-64.ts) to resolve compile errors.

This change is pure dependency maintenance — no public API of any published package changes.

Type of Change

Did you update HISTORY.md?

  • No, this change does not impact library users

Test Plan

  • npm run build — all 6 packages compile
  • npm run lint — no lint errors
  • npm test — 1032 unit tests pass
  • npm run test:integration — 159 integration tests pass against xrpld Docker container
  • npm run test:browser — 159 browser tests pass (also fixed a pre-existing TS1503 error in ripple-keypairs by adding target: "es2018" to the shared ts-loader config)
  • npm run test:faucet — 4 faucet tests pass

Superseded Dependabot PRs

PR Package From To Status MajorVersionUpgrade
#3276 follow-redirects 1.15.11 1.16.0 Upgraded No
#3272 axios, nx 1.12.0→1.15.0, 22.6.4→22.6.5 (grouped) Upgraded No
#3267 lodash 4.17.23 4.18.1 Upgraded No
#3266 serialize-javascript, terser-webpack-plugin (grouped) (grouped) No-op (transitive, resolved by lockfile) No
#3265 eslint-plugin-jsdoc 52.0.4 62.9.0 Skipped (peer dep conflict: @xrplf/eslint-config@^3 requires eslint-plugin-jsdoc@^52) Yes (v62)
#3264 https-proxy-agent 7.0.6 9.0.0 Upgraded Yes (v8, v9)
#3263 @types/chai 4.3.20 5.2.3 Upgraded Yes (chai v5)
#3262 eslint-plugin-tsdoc 0.5.0 0.5.2 Upgraded No
#3261 ws 8.19.0 8.20.0 Upgraded No
#3260 webpack-bundle-analyzer 5.1.1 5.3.0 Upgraded No
#3259 typescript-eslint 8.52.0 8.58.0 Upgraded No
#3258 jest-mock 30.2.0 30.3.0 Upgraded No
#3257 @eslint/js 9.39.2 10.0.1 Skipped (peer dep conflict: @xrplf/eslint-config@^3 requires @eslint/js@^9) Yes (v10)
#3255 prettier 3.7.4 3.8.1 Upgraded No
#3253 expect 30.2.0 30.3.0 Upgraded No
#3252 bignumber.js 9.3.0 10.0.2 Upgraded Yes (v10)
#3251 typedoc 0.28.15 0.28.18 Upgraded No
#3250 jest 30.2.0 30.3.0 Upgraded No
#3249 eslint 9.39.2 10.1.0 Skipped (peer dep conflict: @xrplf/eslint-config@^3 requires eslint@^9) Yes (v10)
#3248 react 19.2.3 19.2.4 Upgraded No
#3247 eslint-plugin-n 17.23.1 17.24.0 Upgraded No
#3246 eslint-plugin-array-func 5.1.0 5.1.1 Upgraded No
#3245 globals 16.3.0 17.4.0 Skipped (peer dep conflict: @xrplf/eslint-config@^3 requires globals@^16) Yes (v17)
#3244 typescript 5.2.2 6.0.2 Skipped (requires monorepo-wide moduleResolution migration) Yes (v6)
#3243 eslint-plugin-prettier 5.5.4 5.5.5 Upgraded No
#3242 webpack 5.104.1 5.105.4 Upgraded No
#3241 webpack-cli 6.0.1 7.0.2 Upgraded Yes (v7)
#3080 @scure/base 1.2.6 2.0.0 Upgraded Yes (v2)
#3051 form-data, lerna (grouped) (grouped) No-op (transitive, resolved by lockfile) No
#3013 ws, @types/ws 8.18.1→8.18.2, 8.18.0→8.18.1 (grouped) Upgraded No

Closing Instructions

After merging, close the following superseded PRs (Skipped ones remain open for future handling): #3276, #3272, #3267, #3266, #3264, #3263, #3262, #3261, #3260, #3259, #3258, #3255, #3253, #3252, #3251, #3250, #3248, #3247, #3246, #3243, #3242, #3241, #3080, #3051, #3013

The following PRs were Skipped and should remain open: #3265 (eslint-plugin-jsdoc), #3257 (@eslint/js), #3249 (eslint), #3245 (globals), #3244 (typescript).

@coderabbitai

coderabbitai Bot commented Apr 10, 2026

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Updated dependency versions across the monorepo; extended Jest transforms to include built JS from two agent packages; added defensive BigNumber parsing (throwing ValidationError) in several numeric utilities; minor TypeScript annotations and new batch-deps-upgrade Claude skill docs.

Changes

Cohort / File(s) Summary
Jest config
jest.config.base.js
Enable ts-jest (with allowJs: true, target: es2020) for built JS in node_modules/https-proxy-agent/ and node_modules/agent-base/; remove those packages from transformIgnorePatterns.
Root & Tooling deps
package.json
Bumped multiple devDependencies (ESLint plugins, Jest, Prettier, Webpack, etc.); no script or public-field changes.
Workspace dependency bumps
packages/isomorphic/package.json, packages/ripple-address-codec/package.json, packages/ripple-binary-codec/package.json, packages/xrpl/package.json
Updated package dependencies: ws/@types/ws, @scure/base, bignumber.js, https-proxy-agent, lodash, react, typedoc, fast-json-stable-stringify, and others.
Type annotations
packages/ripple-binary-codec/src/types/amount.ts, packages/ripple-binary-codec/src/types/uint-64.ts
Added explicit Uint8Array type annotations to local variables; no runtime changes.
BigNumber validation guards
packages/xrpl/src/utils/quality.ts, packages/xrpl/src/utils/xrpConversion.ts, packages/ripple-binary-codec/src/quality.ts, packages/ripple-binary-codec/src/types/amount.ts
Wrap BigNumber construction/conversion in try/catch and throw ValidationError or Error with clearer messages on parse failures; removed some post-conversion 'NaN' string checks.
Build/test config
webpack.test.config.js
Set TypeScript compilerOptions.target to es2018 in ts-loader for test webpack config.
New docs / skill
.claude/skills/batch-deps-upgrade/README.md, .claude/skills/batch-deps-upgrade/SKILL.md
Add documentation and skill definition for an automated batch-dependency-upgrade workflow (Dependabot consolidation, validation, outputs).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

Possibly related PRs

Suggested labels

dependencies

Suggested reviewers

  • ckeshava
  • khancode
  • pdp2121
  • Patel-Raj11
  • mvadari

Poem

🐇 I hopped through locks of package trees tonight,
I nudged BigNumber guards to keep values right.
Jest got a tweak, deps got a lift,
Docs and skills added — a tidy gift.
Hop, test, merge — then onward into light!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change as a quarterly batch dependency upgrade for Q2 2026, which accurately summarizes the changeset's primary objective.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The PR description comprehensively covers all required template sections: high-level overview of batched dependency upgrades, detailed context explaining breaking changes and fixes, type of change acknowledgment, HISTORY.md status, extensive test plan results, and superseded PRs table with closing instructions.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch test-consolidate-deps-workflow

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
.claude/commands/batch-deps-upgrade.md (1)

59-59: Tighten repetitive wording in output description.

Line 59 repeats “explanation/explaining”; consider a cleaner phrase for readability.

Suggested wording
-1. **Code changes explanation** — write a markdown file (`.claude/commands/batch-deps-upgrade-code-changes.md`) documenting every non-package.json source code change, explaining what broke, why, and the minimal fix applied.
+1. **Code changes explanation** — write a markdown file (`.claude/commands/batch-deps-upgrade-code-changes.md`) documenting every non-package.json source code change, including what broke, why, and the minimal fix applied.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.claude/commands/batch-deps-upgrade.md at line 59, The line currently
repeats "explanation/explaining"; edit the sentence in
.claude/commands/batch-deps-upgrade.md (the "Code changes explanation"
instruction) to remove the redundancy and tighten wording—for example, rephrase
to "Code changes — write a markdown file
(.claude/commands/batch-deps-upgrade-code-changes.md) documenting every
non-package.json source code change: what broke, why, and the minimal fix
applied." Update the single sentence wherever the repeated words occur so the
output description reads clearly and concisely.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@package.json`:
- Line 51: The package.json upgrade of the "lerna" dependency to "^9.0.4"
violates the team's decision to remain on v8; revert the version spec back to an
appropriate v8 pin (e.g., replace "^9.0.4" with the project's agreed v8 version)
in package.json or, if this PR is intentionally upgrading to v9, add the
approved v9 migration reference/approval in the PR description and include that
approval identifier in the commit message so reviewers can verify the decision;
locate the "lerna" entry in package.json to make the change.

In `@packages/ripple-binary-codec/package.json`:
- Line 15: The BigNumber constructor can now throw on invalid input; update
codec parse paths to catch those throws or validate before constructing to
preserve stable errors: wrap all BigNumber creation sites in quality.ts
(specifically in functions decodeQuality and encode) and in types/amount.ts
(places that currently call BigNumber directly and locations relying on
assertIouIsValid) with try-catch blocks that convert/normalize errors to the
library's existing error types, or add pre-validation logic before calling new
BigNumber(...); ensure decodeQuality remains safe for public input by returning
the same error shape as prior behavior instead of letting BigNumber exceptions
leak.

---

Nitpick comments:
In @.claude/commands/batch-deps-upgrade.md:
- Line 59: The line currently repeats "explanation/explaining"; edit the
sentence in .claude/commands/batch-deps-upgrade.md (the "Code changes
explanation" instruction) to remove the redundancy and tighten wording—for
example, rephrase to "Code changes — write a markdown file
(.claude/commands/batch-deps-upgrade-code-changes.md) documenting every
non-package.json source code change: what broke, why, and the minimal fix
applied." Update the single sentence wherever the repeated words occur so the
output description reads clearly and concisely.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: cf44caaf-d5c3-4463-bffa-47d3ef73b9b1

📥 Commits

Reviewing files that changed from the base of the PR and between 5cf0921 and 12fc9d5.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (12)
  • .claude/commands/batch-deps-upgrade.md
  • .claude/docs/batch-deps-upgrade-README.md
  • jest.config.base.js
  • package.json
  • packages/isomorphic/package.json
  • packages/ripple-address-codec/package.json
  • packages/ripple-binary-codec/package.json
  • packages/ripple-binary-codec/src/types/amount.ts
  • packages/ripple-binary-codec/src/types/uint-64.ts
  • packages/xrpl/package.json
  • packages/xrpl/src/utils/quality.ts
  • packages/xrpl/src/utils/xrpConversion.ts

Comment thread package.json
Comment thread packages/ripple-binary-codec/package.json
@kuan121

kuan121 commented Apr 10, 2026

Copy link
Copy Markdown
Collaborator Author

/ai-review

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 @.claude/skills/batch-deps-upgrade/SKILL.md:
- Line 69: The markdown in .claude/skills/batch-deps-upgrade/SKILL.md uses
non-canonical product casing on the PR template instruction; update the text at
the PR template reference (the sentence "write a markdown file
(`.claude/skills/batch-deps-upgrade/pr-description.md`) following the repo's PR
template") to use the official product casing "GitHub" instead of any other
variant so the line reads using "GitHub" consistently.
- Line 65: Edit the bullet titled "Code changes explanation" to remove the
duplicated wording "explanation/explaining" by rephrasing the sentence; replace
the current line with something like: "Write a markdown file documenting every
non-package.json source code change: explain what broke, why it broke, and the
minimal fix applied." This targets the exact heading "Code changes explanation"
and the sentence that repeats "explanation/explaining."
- Around line 26-29: The step that tells maintainers to "remove unused
dependencies" based solely on `npx knip` output is risky; update the SKILL.md
instructions around the `npx knip` step (the bullet starting "Unused
dependencies") so removals require human confirmation or a secondary
verification (e.g., run full test suite, search for dynamic/optional imports, or
run a CI job that checks runtime usage) before editing package.json, and add a
short note next to the `npx knip` / "Unlisted dependencies" guidance to
explicitly require adding transitive-but-used packages only after manual
validation.
- Around line 42-45: Replace the vague "Wait for healthy" instruction with a
concrete readiness check and bounded timeout: poll the rippled_standalone
container health/status (e.g., docker inspect .State.Health.Status or curl to
the node's health endpoint) in a loop with a clear timeout (e.g., 60–120s) and
fail fast if not ready, then only run "npm run test:integration && npm run
test:browser" after the readiness check succeeds; ensure the instructions name
the container "rippled_standalone" so the command to check readiness and the
timeout are unambiguous.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 207a49ef-f320-4ce9-9003-34ab5cffdf46

📥 Commits

Reviewing files that changed from the base of the PR and between 12fc9d5 and d86ec3b.

📒 Files selected for processing (3)
  • .claude/skills/batch-deps-upgrade/README.md
  • .claude/skills/batch-deps-upgrade/SKILL.md
  • packages/xrpl/src/utils/xrpConversion.ts
✅ Files skipped from review due to trivial changes (1)
  • .claude/skills/batch-deps-upgrade/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/xrpl/src/utils/xrpConversion.ts

Comment thread .claude/skills/batch-deps-upgrade/SKILL.md Outdated
Comment thread .claude/skills/batch-deps-upgrade/SKILL.md Outdated
Comment thread .claude/skills/batch-deps-upgrade/SKILL.md Outdated
Comment thread .claude/skills/batch-deps-upgrade/SKILL.md

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 @.claude/skills/batch-deps-upgrade/SKILL.md:
- Line 43: The Docker command in the SKILL.md snippet uses incorrect image,
binary and volume path; update the docker run invocation so the image tag uses
rippleci/rippled:develop (not rippleci/xrpld:develop), run the rippled binary
(replace xrpld --standalone with rippled --standalone), and change the mounted
config path from /etc/opt/xrpld/ to /etc/opt/ripple/ so the command uses the
correct image name, executable and config mount.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5af207f4-7594-468f-b3dc-ffb157aa549d

📥 Commits

Reviewing files that changed from the base of the PR and between d86ec3b and 9f844b1.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (1)
  • .claude/skills/batch-deps-upgrade/SKILL.md

Comment thread .claude/skills/batch-deps-upgrade/SKILL.md Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
packages/ripple-binary-codec/src/quality.ts (1)

15-27: Minor: stylistic inconsistency between encode and decode.

encode uses BigNumber(quality) (factory-style) while decode at Line 40 uses new BigNumber(...). Both are valid in bignumber.js, but mixing them in adjacent methods is easy to trip over. Also, decimal?.e at Line 22 no longer needs the optional chain — after the try/catch assigns or throws, decimal is guaranteed to be defined.

♻️ Optional cleanup
   static encode(quality: string): Uint8Array {
     let decimal: BigNumber
     try {
-      decimal = BigNumber(quality)
+      decimal = new BigNumber(quality)
     } catch (_err) {
       throw new Error(`${quality} is not a valid quality`)
     }
-    const exponent = (decimal?.e || 0) - 15
+    const exponent = (decimal.e || 0) - 15
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/ripple-binary-codec/src/quality.ts` around lines 15 - 27, The encode
method uses the factory call BigNumber(quality) and an unnecessary optional
chain on decimal (decimal?.e), causing stylistic mismatch with decode which uses
new BigNumber(...); update encode to use new BigNumber(quality) and remove the
optional chaining (use decimal.e) after the try/catch so the code is consistent
and reflects that decimal is always defined or an error was thrown.
packages/ripple-binary-codec/src/types/amount.ts (1)

112-172: BigNumber guarding looks correct, but error type diverges from the rest of the PR.

The three try/catch blocks correctly convert bignumber.js v10's new throw-on-invalid into a descriptive domain error, and the ${value} is an illegal amount message matches the existing test contract (e.g. packages/ripple-binary-codec/test/amount.test.ts:86-90), so behavior is preserved.

Minor consistency note: the companion changes in packages/xrpl/src/utils/xrpConversion.ts and packages/xrpl/src/utils/quality.ts throw ValidationError on the same class of failure. Here we throw a plain Error. This is intentional given the existing tests in ripple-binary-codec, but worth calling out so future consumers don't assume a uniform error type across packages.

Also note that the pre-existing MPT path at Line 325 (BigInt(amount) & BigInt(mptMask)) will still throw a raw SyntaxError for BigNumber-valid but BigInt-invalid inputs such as "1e5" — not introduced here, but the broader hardening in this PR might be a good moment to wrap it too.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/ripple-binary-codec/src/types/amount.ts` around lines 112 - 172,
Summary: Replace plain Error throws for invalid BigNumber/BigInt conversions
with a consistent ValidationError and guard BigInt parsing in the MPT path.
Update the catch in the isAmountObjectIOU BigNumber conversion to throw
ValidationError instead of Error (change the current catch that throws
`${value.value} is an illegal amount`), and similarly change the other two
BigNumber try/catch sites in this module to throw ValidationError; additionally
wrap BigInt parsing in the isAmountObjectMPT path (the const num =
BigInt(value.value) line) and the existing BigInt mask operation (BigInt(amount)
& BigInt(mptMask)) in try/catch blocks that throw ValidationError on failure so
BigInt-invalid inputs like "1e5" produce a ValidationError rather than a raw
SyntaxError. Ensure you import or reference the ValidationError class used
elsewhere in the repo.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.claude/skills/batch-deps-upgrade/SKILL.md:
- Around line 42-50: The fenced code block containing the shell snippet (the
block starting with ```) is missing a language tag which triggers markdownlint
MD040; update the opening fence to include a language (e.g., add "bash" after
the backticks) so the block becomes ```bash, preserving the existing shell
content (SECONDS loop, nc checks, docker logs, exit) and nothing else.

---

Nitpick comments:
In `@packages/ripple-binary-codec/src/quality.ts`:
- Around line 15-27: The encode method uses the factory call BigNumber(quality)
and an unnecessary optional chain on decimal (decimal?.e), causing stylistic
mismatch with decode which uses new BigNumber(...); update encode to use new
BigNumber(quality) and remove the optional chaining (use decimal.e) after the
try/catch so the code is consistent and reflects that decimal is always defined
or an error was thrown.

In `@packages/ripple-binary-codec/src/types/amount.ts`:
- Around line 112-172: Summary: Replace plain Error throws for invalid
BigNumber/BigInt conversions with a consistent ValidationError and guard BigInt
parsing in the MPT path. Update the catch in the isAmountObjectIOU BigNumber
conversion to throw ValidationError instead of Error (change the current catch
that throws `${value.value} is an illegal amount`), and similarly change the
other two BigNumber try/catch sites in this module to throw ValidationError;
additionally wrap BigInt parsing in the isAmountObjectMPT path (the const num =
BigInt(value.value) line) and the existing BigInt mask operation (BigInt(amount)
& BigInt(mptMask)) in try/catch blocks that throw ValidationError on failure so
BigInt-invalid inputs like "1e5" produce a ValidationError rather than a raw
SyntaxError. Ensure you import or reference the ValidationError class used
elsewhere in the repo.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: cae30c97-fef7-4f4a-93b5-941567982d34

📥 Commits

Reviewing files that changed from the base of the PR and between 604ee9c and c711a62.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (5)
  • .claude/skills/batch-deps-upgrade/SKILL.md
  • package.json
  • packages/ripple-binary-codec/src/quality.ts
  • packages/ripple-binary-codec/src/types/amount.ts
  • webpack.test.config.js
✅ Files skipped from review due to trivial changes (2)
  • webpack.test.config.js
  • package.json

Comment thread .claude/skills/batch-deps-upgrade/SKILL.md Outdated
Comment thread webpack.test.config.js Outdated
Comment thread jest.config.base.js
@@ -12,7 +12,7 @@ module.exports = {
},

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes in this file are caused by: https-proxy-agent 7 → 9

v9 is ESM-only (import syntax), but Jest runs in CommonJS. Without this, Jest fails with SyntaxError: Cannot use import statement outside a module when tests import the xrpl client (which uses https-proxy-agent).

Fix: Added transform entries so ts-jest transpiles the ESM code, and excluded them from transformIgnorePatterns so Jest doesn't skip them.

@@ -21,12 +21,12 @@ export function dropsToXrp(dropsToConvert: BigNumber.Value): number {
* decimal point followed by zeros, e.g. '1.00'.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes in this file are caused by: bignumber.js 9 → 10

v9: new BigNumber("FOO") returns a BigNumber with value NaN.
v10: new BigNumber("FOO") throws [BigNumber Error] Not a number: FOO.

The existing code passed invalid input to BigNumber first, then checked if the result was NaN to throw a custom ValidationError. In v10 the BigNumber constructor throws before the NaN check runs, so the user-facing error message changes from the expected ValidationError to a raw BigNumber error.

Fix: Wrap new BigNumber() calls in try-catch, re-throwing as the same ValidationError the code already intended. Also removed the now-dead NaN checks that followed (unreachable since the constructor throws first). Applied in both dropsToXrp and xrpToDrops.

@@ -17,7 +17,11 @@ function percentToDecimal(percent: string): string {
throw new ValidationError(`Value ${percent} contains too many % signs`)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to the changes in xrpConversion.ts, the changes in this file are caused by: same bignumber.js 9 → 10

Three functions accept user string input and construct a BigNumber: percentToDecimal, decimalToTransferRate, and decimalToQuality. All three previously relied on v9's NaN-return behavior to either propagate a 'NaN' string downstream or trip an explicit if (billionths === 'NaN') check that threw ValidationError("Value is not a number").

In v10, the constructor throws a raw [BigNumber Error] before any downstream check can run, so the expected ValidationError is never produced and tests (e.g., percentToQuality throws with gibberish, decimalToQuality throws with gibberish in quality.test.ts) fail with a raw BigNumber error.

Fix:

  • Wrap each new BigNumber() call in try-catch and re-throw ValidationError("Value is not a number") — the exact error class and message consumers already observed in v9.
  • Remove the now-unreachable NaN checks in decimalToTransferRate and decimalToQuality (the stringified 'NaN' path is dead in v10 because the constructor throws first).a

@@ -13,8 +13,13 @@ class quality {
* @returns Serialized quality

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes in this file are caused by: same bignumber.js 9 → 10

encode(quality) and decode(quality) construct BigNumbers from strings. In v10 both will throw raw [BigNumber Error] on invalid input instead of returning NaN.

Fix: Wrap both constructor calls in try-catch and re-throw a clear Error: <value> is not a valid quality instead.instead.

}

let amount = new Uint8Array(8)
let amount: Uint8Array = new Uint8Array(8)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is caused by: @scure/base 1 → 2

v2 introduced stricter Uint8Array generics. new Uint8Array(8) is inferred as Uint8Array<ArrayBuffer>, but concat() (via @noble/hashes) returns Uint8Array<ArrayBufferLike> — a wider type that includes SharedArrayBuffer. TypeScript won't assign the wider return type back to the narrower variable, causing a compile error. This is purely a compile-time issue — at runtime there is no difference.

Fix: Explicit type annotation Uint8Array (without generic parameter) so TypeScript uses the unparameterized base type that accepts both.

@@ -110,7 +110,12 @@ class Amount extends SerializedType {
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The following changes are caused by: same bignumber.js 9 → 10

Three constructor call sites accept user input and could throw in v10:

  • Line 113: new BigNumber(value.value) inside from() for IOU amounts
  • Line 264: new BigNumber(amount) inside assertXrpIsValid
  • Line 304: new BigNumber(amount) inside assertMptIsValid

Fix: Wrap each constructor call in try-catch, re-throwing the existing Error: <amount> is an illegal amount message to preserve API behavior. Also removed the dead if (decimal.isNaN()) check in assertMptIsValid — unreachable in v10 because the constructor throws first.

}

let buf = new Uint8Array(UInt64.width)
let buf: Uint8Array = new Uint8Array(UInt64.width)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is caused by: @scure/base 1 → 2

v2 introduced stricter Uint8Array generics. new Uint8Array(8) is inferred as Uint8Array<ArrayBuffer>, but concat() (via @noble/hashes) returns Uint8Array<ArrayBufferLike> — a wider type that includes SharedArrayBuffer. TypeScript won't assign the wider return type back to the narrower variable, causing a compile error. This is purely a compile-time issue — at runtime there is no difference.

Fix: Explicit type annotation Uint8Array (without generic parameter) so TypeScript uses the unparameterized base type that accepts both.

@kuan121 kuan121 force-pushed the test-consolidate-deps-workflow branch from 16c5dd4 to 6d84fdc Compare April 22, 2026 14:06
@kuan121 kuan121 force-pushed the test-consolidate-deps-workflow branch from 6d84fdc to fa51f79 Compare April 22, 2026 14:08
Comment thread .claude/skills/batch-deps-upgrade/SKILL.md Outdated
} catch (_err) {
throw new Error(`${quality} is not a valid quality`)
}
const exponent = (decimal.e || 0) - 15

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const exponent = (decimal.e || 0) - 15
const exponent = (decimal.e) - 15

Since the BIgNumber object is constructed successfully, the exponent attribute must always be present.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the decimal is 0, decimal.e can be null, which would result in null - 15. So it’s safer to keep the || 0.

Comment thread packages/ripple-binary-codec/src/quality.ts
Comment thread packages/ripple-binary-codec/src/types/amount.ts
Patel-Raj11
Patel-Raj11 previously approved these changes Apr 22, 2026
Patel-Raj11
Patel-Raj11 previously approved these changes Apr 23, 2026
Comment thread packages/ripple-binary-codec/src/types/amount.ts Outdated
Comment thread packages/xrpl/src/utils/xrpConversion.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants