import fixes #1480
import fixes #1480
Conversation
9.1.0 is the version that added ESM support.
can't omit the file extension in ESM imports.
can't omit the file extension in ESM imports.
These packages are written in naked CJS, with a style that resists static analysis, and named exports don't work, which means namespace exports don't work.
dynst
force-pushed
the
chore/esm-import-fixes
branch
from
18a4611 to
1f44095
Compare
dynst
force-pushed
the
chore/esm-import-fixes
branch
from
5a47df4 to
f72dd40
Compare
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
📝 WalkthroughWalkthroughPatch bumps bignumber.js to 9.1.2 across packages and examples. Adjusts ESM compatibility: switches certain namespace imports to default, appends .js extensions for coinselect, cosmjs-types, and protobufjs/minimal imports, updates TS module declarations, regenerates protobuf outputs, and tweaks gen scripts. Adds a jest import in a Radix test. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
packages/xchain-base/package.json (1)
43-44: Duplicate of the BigNumber.js upgrade concern raised in@xchainjs/xchain-wallet.packages/xchain-cosmos/package.json (1)
50-51: Duplicate of the BigNumber.js upgrade concern raised in@xchainjs/xchain-wallet.packages/xchain-thorchain/package.json (1)
53-54: Duplicate of the BigNumber.js upgrade concern raised in@xchainjs/xchain-wallet.
🧹 Nitpick comments (9)
packages/xchain-mayachain/src/client.ts (2)
257-262: Typo:demonAmountlooks like a copy-paste slipThe variable represents a denom amount, but the current spelling reads like “demon”. Renaming improves readability.
- const demonAmount = { amount: amount.amount().toString(), denom } + const denomAmount = { amount: amount.amount().toString(), denom }
457-477: Emptycatch {}blocks silently discard root-cause errors
roundRobinSignswallows every exception when iterating overclientUrls, making debugging impossible and masking permanent mis-configuration.- } catch {} + } catch (err) { + // Consider logging at debug level or aggregating the last error + this.logger?.debug?.(`signing failed against ${url}: ${err}`) + lastError = err + } ... - throw Error('No clients available. Can not sign transaction') + throw lastError ?? Error('No clients available. Cannot sign transaction')Alternatively bubble the original error after the loop.
packages/xchain-dash/src/modules.d copy.ts (1)
1-1: File name looks accidental (modules.d copy.ts)The extra copy suffix is easy to overlook and may be excluded from
tsconfigincludes.
Rename tomodules.d.ts(or merge into an existing declaration file) to avoid duplicate/ignored declarations.packages/xchain-mayachain/genMsgs.sh (1)
20-22:sedonly fixes double-quoted imports – single quotes still break ESMGenerated code sometimes uses
'protobufjs/minimal'. Extend the pattern to handle both quote styles.-sed -i -E 's|"(protobufjs/minimal)"|"\1.js"|' "$MSG_COMPILED_OUTPUTFILE" +sed -i -E "s|(['\"])(protobufjs/minimal)\\1|\\1\\2.js\\1|g" "$MSG_COMPILED_OUTPUTFILE"packages/xchain-thorchain/genMsgs.sh (1)
44-46: Same quoting edge-case as Maya scriptMirror the enhanced
sedto capture single-quoted occurrences for consistency.packages/xchain-util/package.json (1)
34-34: Changelog entry recommended for utility consumers
@xchainjs/xchain-utilis widely imported outside the monorepo. Add a brief note to the package CHANGELOG highlighting the BigNumber upgrade so integrators can pin if needed.packages/xchain-ethereum/package.json (1)
44-44: Consistent upgrade – run a workspace-wide dedupeAfter bumping many leaf packages,
bignumber.jscan still end up duplicated if an older constraint survives in a transitive dependency.
Recommend running:yarn dedupe --strategy highest(or
npm dedupe) and committing the updated lockfile to guarantee a single copy in node_modules.packages/xchain-bsc/package.json (1)
44-44: Minor note: favour a tilde if you only want patch releasesThe caret (
^) will allow 9.x minor bumps automatically.
If the intent is to freeze at the feature level that fixed the ESM typings (9.1), switch to~9.1.2. Otherwise leave as-is.packages/xchain-litecoin/src/client.ts (1)
5-6: Importing via sub-path with explicit.jsties you to the package’s internal layout
import accumulative from 'coinselect/accumulative.js'works today for ESM resolution, but:
coinselectdoes not guarantee this file path is stable.- TypeScript path mapping must stay in sync (
modules.d.tsalready updated – good).- Consumers bundling for CJS may need
preserveSymlinks/plugins: { nodeResolve: { extensions: ['.js'] } }.Consider adding an alias in your build (e.g.
exportsfield or ts-path mapping) so futurecoinselectrefactors don’t break you.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (44)
.changeset/ripe-heads-stop.md(1 hunks)examples/check-tx/package.json(1 hunks)examples/estimate-swap/package.json(1 hunks)examples/liquidity/package.json(1 hunks)packages/xchain-arbitrum/package.json(1 hunks)packages/xchain-avax/package.json(1 hunks)packages/xchain-base/package.json(1 hunks)packages/xchain-bitcoin/src/client.ts(1 hunks)packages/xchain-bitcoin/src/modules.d.ts(1 hunks)packages/xchain-bitcoincash/src/client.ts(1 hunks)packages/xchain-bitcoincash/src/clientKeystore.ts(1 hunks)packages/xchain-bitcoincash/src/modules.d.ts(1 hunks)packages/xchain-bitcoincash/src/utils.ts(1 hunks)packages/xchain-bsc/package.json(1 hunks)packages/xchain-cosmos/package.json(1 hunks)packages/xchain-cosmos/src/client.ts(1 hunks)packages/xchain-dash/src/client.ts(1 hunks)packages/xchain-dash/src/clientKeystore.ts(1 hunks)packages/xchain-dash/src/clientLedger.ts(1 hunks)packages/xchain-dash/src/modules.d copy.ts(1 hunks)packages/xchain-dash/src/utils.ts(1 hunks)packages/xchain-doge/src/client.ts(1 hunks)packages/xchain-doge/src/modules.d.ts(1 hunks)packages/xchain-ethereum/package.json(1 hunks)packages/xchain-evm/package.json(1 hunks)packages/xchain-kujira/src/client.ts(1 hunks)packages/xchain-litecoin/src/client.ts(1 hunks)packages/xchain-litecoin/src/modules.d.ts(1 hunks)packages/xchain-mayachain-query/package.json(1 hunks)packages/xchain-mayachain/genMsgs.sh(1 hunks)packages/xchain-mayachain/package.json(1 hunks)packages/xchain-mayachain/src/client.ts(1 hunks)packages/xchain-mayachain/src/types/proto/MsgCompiled.js(1 hunks)packages/xchain-radix/__tests__/client.test.ts(1 hunks)packages/xchain-thorchain-query/package.json(1 hunks)packages/xchain-thorchain/genMsgs.sh(1 hunks)packages/xchain-thorchain/package.json(1 hunks)packages/xchain-thorchain/src/client.ts(1 hunks)packages/xchain-thorchain/src/clientKeystore.ts(1 hunks)packages/xchain-thorchain/src/clientLedger.ts(1 hunks)packages/xchain-thorchain/src/types/proto/MsgCompiled.d.ts(1 hunks)packages/xchain-util/package.json(1 hunks)packages/xchain-wallet/package.json(1 hunks)tools/txJammer/package.json(1 hunks)
🔇 Additional comments (38)
packages/xchain-bitcoin/src/modules.d.ts (1)
1-2:.jssuffix aligns the declaration with the new import paths – looks goodThe declaration now matches the runtime import (
coinselect/accumulative.js) introduced elsewhere in the PR, preventing “cannot find module” errors when TypeScript looks up the module name.packages/xchain-mayachain/src/types/proto/MsgCompiled.js (1)
4-4: Confirm the.js-suffixed require works in all build targets
require("protobufjs/minimal.js")will resolve correctly in Node (the file exists at the package root).
If any webpack/rollup config usesmainFieldsthat omit plain files, resolution might fail. Please run the build for browser/webpack targets to verify there are no “module not found” errors.packages/xchain-dash/src/clientLedger.ts (1)
1-1: No action needed – default CJS interop flags are enabledThe
packages/xchain-dash/tsconfig.jsonextends the root config, which has both
"esModuleInterop": true
and
"allowSyntheticDefaultImports": true
soimport dashcore from '@dashevo/dashcore-lib'works as intended.packages/xchain-dash/src/client.ts (1)
1-1: Check typings fordashcoredefault import.
@dashevo/dashcore-libships as CJS only.
Switching fromimport * as dashcore …toimport dashcore …relies on TypeScript emitting animport _default = require()shim (esModuleInterop|allowSyntheticDefaultImports).
If those compiler flags are not enabled,dashcorewill be typed asanyand every member access (dashcore.Script, etc.) will lose type-safety.If the project already compiles under
moduleResolution: node16/nodenextwithesModuleInteropset, nothing to do. Otherwise, add{ "compilerOptions": { "esModuleInterop": true, "allowSyntheticDefaultImports": true } }or revert to
import * as dashcore from '@dashevo/dashcore-lib'to retain typings.
packages/xchain-kujira/src/client.ts (1)
17-17: LGTM – explicit.jsextension unblocks Node-16 ESM resolution.
cosmjs-typespublishes transpiled JS files; appending.jsavoids the
“ERR_MODULE_NOT_FOUND” issue under"type":"module"packages. No runtime or type-safety concerns here.packages/xchain-bitcoin/src/client.ts (1)
6-6: Good call adding the.jssuffix forcoinselect/accumulative.
coinselectdistributes plain JS; explicit extension is required for native
ESM loaders. Nothing else is affected – the imported function signature is unchanged.packages/xchain-cosmos/src/client.ts (1)
15-15: Import path corrected – no further action.The generated
cosmjs-typesbundle expects the.jssuffix in ESM mode. The change preserves typings (TxRawlives in the same package). ✅packages/xchain-dash/src/utils.ts (1)
10-10: Consistent with other UTXO clients – looks good.The change mirrors the Bitcoin client and keeps the utility in sync with ESM rules. No functional impact on
accumulativeusage.packages/xchain-mayachain/src/client.ts (1)
25-25: Import extension update LGTMExplicitly adding
.jskeeps the module ESM-safe and is in line with the repo-wide pattern.packages/xchain-dash/src/clientKeystore.ts (1)
2-2: ES-module interop flags confirmed
The roottsconfig.jsonhas both"esModuleInterop": trueand"allowSyntheticDefaultImports": true, soimport dashcore from '@dashevo/dashcore-lib'is valid as written.tools/txJammer/package.json (1)
34-34: Confirm workspace lockfile regeneration after BigNumber.js bumpThe change is just a minor-version bump (
^9.0.0→^9.1.2).
Please make sure you have re-generated the monorepo lockfile (yarn install/pnpm install) so every workspace picks up the new sub-version and CI doesn’t end up with a mixed resolution.examples/liquidity/package.json (1)
33-33: Ensure example scripts still compile against BigNumber 9.1.2The examples import BigNumber directly in several
.tsfiles. Version 9.1.x started shipping dual ESM/CJS bundles – usually harmless, but a stale ts-node or an older Rollup/ESBuild plugin might mis-detect the entry point.
Just runyarn buildfor the examples locally to confirm there are no “Cannot find module ‘bignumber.js’” errors.examples/estimate-swap/package.json (1)
25-25: Validate the runtime rounding behaviourBigNumber 9.1.0 fixed a subtle rounding-mode edge case. Down-stream swap-fee or slippage calculations could shift by one least-significant unit. Run the existing swap-estimation test suite (or a quick manual comparison) to be safe.
packages/xchain-radix/__tests__/client.test.ts (1)
13-13: Importingjestfrom@jest/globalsis correct for ESM, but double-check Jest config
Make sure
jest.config.(js|ts)hasextensionsToTreatAsEsm: ['.ts']and
preset: 'ts-jest/presets/default-esm'(or equivalent) so the import path resolves.Other test files still relying on the global
jestwon’t break, but for consistency you might migrate them in a follow-up.No action required if CI is green.
examples/check-tx/package.json (1)
23-23: Lockfile regeneration after BigNumber.js bumpI confirmed no remaining
"bignumber.js": "^9.0.0"entries in the repo. Before merging, reinstall dependencies to rebuild the lockfile with a single 9.1.x entry:rm -rf node_modules yarn.lock # or package-lock.json yarn install # or npm installpackages/xchain-arbitrum/package.json (1)
43-43: Dependency aligned; verify downstream type compatibilityNothing else changed here, so risk is low.
Still, CI should compile all TypeScript targets to ensure Arbitrum-specific wrappers don’t rely on the now-removedBNtype exports present pre-9.1.
No action required if the build already passes.packages/xchain-mayachain-query/package.json (1)
43-43: LGTM – remember to update Docker / CI cache layersIf CI images cache
node_modules, invalidate them so the new BigNumber version is picked up; otherwise tests might still run against 9.0.0.packages/xchain-wallet/package.json (1)
39-40: Confirm BigNumber.js v9.1.2 doesn’t introduce runtime regressionsBumping from 9.0.x → 9.1.2 is a minor release, but BigNumber.js occasionally alters default rounding / configuration flags. Please run unit-tests (esp. around toFixed/toFormat) and verify that the package hoisting in the monorepo resolves to a single 9.1.2 instance.
packages/xchain-mayachain/package.json (1)
50-50:bignumber.jspatch-level bump looks safe, but please re-run unit + type testsGoing from
^9.0.0→^9.1.2is a non-breaking upgrade, yet9.1.0introduced ESM packaging changes. Make sure:
- the workspace’s TypeScript
moduleResolutionisnode16/nodenext,- bundlers still tree-shake correctly, and
- no BigNumber constructor overloads changed in a way that affects runtime.
A quick
yarn test && yarn type-checkon all consumer packages will confirm.packages/xchain-evm/package.json (1)
44-44: Same BigNumber upgrade – verify downstream hex/wei helpers
@xchainjs/xchain-evmheavily relies onbignumber.jsfor gwei/wei math. Newer versions fixed some rounding edge-cases; double-check integration tests around:
BaseAmount.toBigNumber()- gas‐price normalisation.
If they pass, 👍.
packages/xchain-avax/package.json (1)
44-44: Dependency alignment LGTMKeeping all EVM packages on the same
bignumber.jsminor avoids duplication in the bundle. No additional feedback.packages/xchain-thorchain/src/client.ts (1)
16-16: Adding the.jsextension fixes ESM import – but confirm TS resolver modeThe explicit
.jssuffix is mandatory for Node ≥ 20 in"type":"module"packages.
However, if any consumer project still compiles withmoduleResolution: "node"(classic), TypeScript will fail to locate the corresponding*.d.ts.Action items:
-import { TxRaw } from 'cosmjs-types/cosmos/tx/v1beta1/tx.js' +// ✅ Works in Node >16 with `moduleResolution: node16|nodenext` +import { TxRaw } from 'cosmjs-types/cosmos/tx/v1beta1/tx.js'Please verify all TS builds (both CJS + ESM output targets) before merging.
packages/xchain-thorchain-query/package.json (1)
42-42: Patch update acknowledgedNothing else to flag.
packages/xchain-thorchain/src/clientKeystore.ts (1)
16-16: LGTM: Correct ESM compatibility fixThe addition of the
.jsextension to the cosmjs-types import path correctly addresses ESM module resolution requirements. This change aligns with the PR's objective to fix CommonJS import issues.packages/xchain-doge/src/client.ts (1)
5-5: LGTM: Consistent ESM import fixThe addition of the
.jsextension to the coinselect/accumulative import path correctly follows the ESM compatibility pattern established in this PR. This ensures proper module resolution for the accumulative function.packages/xchain-bitcoincash/src/utils.ts (1)
8-8: LGTM: Correct CommonJS workaround implementationThis change properly implements the recommended workaround for CommonJS-only packages mentioned in the PR objectives. Switching from namespace import to default import for
bitcore-lib-cashresolves the "Named export not found" errors that occur with CommonJS modules in ESM environments.packages/xchain-bitcoincash/src/clientKeystore.ts (1)
1-1: LGTM: Consistent CommonJS import fixThis change correctly applies the same CommonJS workaround pattern used elsewhere in the codebase for
bitcore-lib-cash. The switch to default import ensures compatibility with ESM module resolution while maintaining the same API usage throughout the file.packages/xchain-thorchain/src/clientLedger.ts (1)
10-11: LGTM: Consistent ESM compatibility fixesBoth import statements correctly add
.jsextensions to the cosmjs-types module paths, following the established pattern for ESM compatibility throughout this PR. These changes ensure proper module resolution for theSignModeandTxRawimports.packages/xchain-bitcoincash/src/modules.d.ts (1)
2-6: LGTM! Correct ESM module declaration update.The addition of the
.jsextension to the module declaration aligns with ESM requirements and properly reflects the actual module file name. This change ensures correct module resolution in ESM environments.packages/xchain-bitcoincash/src/client.ts (2)
2-2: LGTM! Correct CommonJS import workaround.The change from namespace import to default import for
bitcore-lib-cashimplements the recommended workaround for CommonJS-only packages mentioned in the PR objectives. This addresses the "Named export not found" error by importing the entire package as default and allowing destructuring.
6-6: LGTM! Proper ESM import path with extension.Adding the
.jsextension to thecoinselect/accumulativeimport ensures proper module resolution in ESM environments, which aligns with the standardization efforts described in the PR.packages/xchain-litecoin/src/modules.d.ts (1)
2-5: LGTM! Consistent ESM module declaration.The update to include the
.jsextension in the module declaration maintains consistency with similar changes across the xchainjs ecosystem and ensures proper ESM module resolution.packages/xchain-doge/src/modules.d.ts (1)
2-7: LGTM! Consistent ESM module declaration pattern.The module declaration correctly includes the
.jsextension, maintaining consistency with similar changes across other packages in the xchainjs ecosystem. This ensures proper ESM module resolution..changeset/ripe-heads-stop.md (1)
1-27: LGTM! Comprehensive changeset for ESM dependency update.The changeset correctly documents the BigNumber.js version update to 9.1.2 across all affected packages in the xchainjs ecosystem. This version requirement aligns with the PR objectives regarding ESM support and TypeScript compatibility fixes.
packages/xchain-thorchain/src/types/proto/MsgCompiled.d.ts (4)
640-646: LGTM! Status enum properly defined.The
Statusenum correctly defines transaction states with appropriate numeric values following protobuf conventions.
647-796: LGTM! ObservedTx types follow protobuf patterns correctly.The interface and class declarations properly handle optional properties during construction while maintaining type safety for the implemented class properties.
797-892: Note: PascalCase properties maintained for protobuf compatibility.The
PubKeyandSignatureproperties use PascalCase naming, which is preserved from the protobuf schema for wire format compatibility. The Uint8Array type is appropriate for binary cryptographic data.
893-2218: LGTM! All attestation and quorum types are properly structured.The new type declarations for transaction attestation, network fees, solvency proofs, and errata handling are well-structured and follow consistent protobuf patterns. The type hierarchy appropriately models the relationship between individual attestations and their quorum states.
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
packages/xchain-mayachain/genMsgs.sh (1)
1-2: Addset -euo pipefailfor safer scriptingThe script now performs external network operations and file mutations. Failing fast and surfacing the exact failing command will save debugging time and avoid partial artefacts.
#!/bin/bash +set -euo pipefail
♻️ Duplicate comments (6)
packages/xchain-arbitrum/package.json (1)
38-44: Consistent version bump – looks goodNo further action – just ensure lock-file regeneration as mentioned in the util package comment.
packages/xchain-ethereum/package.json (1)
38-45: Version alignment acknowledgedSame note about re-installing dependencies to avoid mixed BigNumber versions.
packages/xchain-mayachain-query/package.json (1)
43-43: Same BigNumber bump – see earlier note.packages/xchain-wallet/package.json (1)
39-39: Same BigNumber bump – see earlier note.packages/xchain-base/package.json (1)
43-43: Same BigNumber bump – see earlier note.packages/xchain-cosmos/package.json (1)
50-50: Same BigNumber bump – see earlier note.
🧹 Nitpick comments (2)
packages/xchain-radix/__tests__/client.test.ts (1)
12-14: Prefer importing the full Jest ESM surface for stricter type-safetyOnly the
jestobject is imported, whiledescribe,it,test, andexpectare still consumed from the global namespace. When running the suite in pure ESM (or with--experimental-vm-modules) this still works at runtime, but TypeScript will silently fall back to ambient global typings, which may diverge from the actual Jest version innode_modules.Importing the commonly-used globals explicitly keeps everything in one place, guarantees the types come from the same
@jest/globalspackage, and makes the file self-contained.-import { jest } from '@jest/globals' +import { jest, describe, it, test, expect } from '@jest/globals'If you prefer to avoid verbosity, you can suppress the linter rule that forbids implicit globals instead, but being explicit is generally clearer.
.changeset/ripe-heads-stop.md (1)
25-26: Capitalize and punctuate summary sentence for consistencyChangeset summaries are displayed verbatim in release notes. A quick tweak keeps style consistent with existing notes.
-require bignumber.js 9.1.2 for esm support +Require BigNumber.js 9.1.2 for ESM support.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (44)
.changeset/ripe-heads-stop.md(1 hunks)examples/check-tx/package.json(1 hunks)examples/estimate-swap/package.json(1 hunks)examples/liquidity/package.json(1 hunks)packages/xchain-arbitrum/package.json(1 hunks)packages/xchain-avax/package.json(1 hunks)packages/xchain-base/package.json(1 hunks)packages/xchain-bitcoin/src/client.ts(1 hunks)packages/xchain-bitcoin/src/modules.d.ts(1 hunks)packages/xchain-bitcoincash/src/client.ts(1 hunks)packages/xchain-bitcoincash/src/clientKeystore.ts(1 hunks)packages/xchain-bitcoincash/src/modules.d.ts(1 hunks)packages/xchain-bitcoincash/src/utils.ts(1 hunks)packages/xchain-bsc/package.json(1 hunks)packages/xchain-cosmos/package.json(1 hunks)packages/xchain-cosmos/src/client.ts(1 hunks)packages/xchain-dash/src/client.ts(1 hunks)packages/xchain-dash/src/clientKeystore.ts(1 hunks)packages/xchain-dash/src/clientLedger.ts(1 hunks)packages/xchain-dash/src/modules.d copy.ts(1 hunks)packages/xchain-dash/src/utils.ts(1 hunks)packages/xchain-doge/src/client.ts(1 hunks)packages/xchain-doge/src/modules.d.ts(1 hunks)packages/xchain-ethereum/package.json(1 hunks)packages/xchain-evm/package.json(1 hunks)packages/xchain-kujira/src/client.ts(1 hunks)packages/xchain-litecoin/src/client.ts(1 hunks)packages/xchain-litecoin/src/modules.d.ts(1 hunks)packages/xchain-mayachain-query/package.json(1 hunks)packages/xchain-mayachain/genMsgs.sh(1 hunks)packages/xchain-mayachain/package.json(1 hunks)packages/xchain-mayachain/src/client.ts(1 hunks)packages/xchain-mayachain/src/types/proto/MsgCompiled.js(1 hunks)packages/xchain-radix/__tests__/client.test.ts(1 hunks)packages/xchain-thorchain-query/package.json(1 hunks)packages/xchain-thorchain/genMsgs.sh(1 hunks)packages/xchain-thorchain/package.json(1 hunks)packages/xchain-thorchain/src/client.ts(1 hunks)packages/xchain-thorchain/src/clientKeystore.ts(1 hunks)packages/xchain-thorchain/src/clientLedger.ts(1 hunks)packages/xchain-thorchain/src/types/proto/MsgCompiled.d.ts(1 hunks)packages/xchain-util/package.json(1 hunks)packages/xchain-wallet/package.json(1 hunks)tools/txJammer/package.json(1 hunks)
🔇 Additional comments (34)
packages/xchain-thorchain-query/package.json (1)
42-42: Confirmed: All packages use BigNumber^9.1.2
Verified that everypackage.jsonin the workspace references"bignumber.js": "^9.1.2". No mismatches found—no further changes required.packages/xchain-bitcoin/src/modules.d.ts (1)
1-2: LGTM! Correct ESM module declaration update.The explicit
.jsextension in the module declaration aligns with ESM requirements and matches the corresponding import path changes in client files.packages/xchain-dash/src/clientLedger.ts (1)
1-1: LGTM! Correct CommonJS import workaround applied.The change from namespace import to default import correctly addresses the CommonJS module compatibility issue mentioned in the PR objectives. The usage throughout the file (e.g.,
new dashcore.Transaction()) is compatible with this import style.packages/xchain-dash/src/client.ts (1)
1-1: LGTM! Consistent CommonJS import fix.This change aligns with the same fix applied in
clientLedger.tsand correctly addresses the CommonJS module import issue. The usage throughout the file is compatible with the default import pattern.packages/xchain-dash/src/utils.ts (1)
10-10: LGTM! Correct ESM import path update.The explicit
.jsextension addition aligns with ESM module resolution requirements and is consistent with similar changes across the ecosystem.packages/xchain-bitcoin/src/client.ts (1)
6-6: LGTM! Import path correctly updated for ESM compatibility.The explicit
.jsextension matches the corresponding module declaration inmodules.d.tsand ensures proper ESM module resolution.packages/xchain-doge/src/client.ts (1)
5-5: LGTM! ESM compatibility improvement.The addition of the
.jsextension to thecoinselect/accumulativeimport path correctly addresses ESM module resolution requirements. This change maintains functionality while ensuring compatibility with ECMAScript modules.packages/xchain-dash/src/clientKeystore.ts (1)
2-2: LGTM! Correctly addresses CommonJS import issue.The change from namespace import to default import for
@dashevo/dashcore-libimplements the recommended workaround for CommonJS-only packages, resolving the "Named export not found" errors mentioned in the PR objectives. The usage throughout the file remains consistent.packages/xchain-bitcoincash/src/modules.d.ts (1)
2-6: LGTM! TypeScript declaration updated for ESM compatibility.The module declaration and comments are correctly updated to include the
.jsextension, ensuring TypeScript properly recognizes the module with ESM-compatible import paths.packages/xchain-doge/src/modules.d.ts (1)
2-7: LGTM! Consistent TypeScript declaration update.The module declaration and documentation are correctly updated to include the
.jsextension, maintaining consistency with the ESM compatibility improvements across the codebase.packages/xchain-litecoin/src/modules.d.ts (1)
2-5: LGTM! ESM compatibility improvementThe addition of the
.jsextension to the module declaration and corresponding comment update correctly addresses ESM module resolution requirements. This change aligns with the PR objective to fix CommonJS import issues.packages/xchain-litecoin/src/client.ts (1)
5-5: LGTM! Import path updated for ESM compatibilityThe addition of the
.jsextension to thecoinselect/accumulativeimport path correctly addresses ESM module resolution requirements and is consistent with the corresponding TypeScript declaration file update.packages/xchain-bitcoincash/src/utils.ts (1)
8-8: LGTM! CommonJS import workaround applied correctlyThe change from namespace import (
import * as bitcore) to default import (import bitcore) correctly implements the recommended workaround for CommonJS modules that don't support named exports in ESM environments. This addresses the specific issue mentioned in the PR objectives where named imports fail for certain packages.packages/xchain-bitcoincash/src/clientKeystore.ts (1)
1-1: LGTM! Consistent CommonJS import workaroundThe change from namespace import to default import for
bitcore-lib-cashis consistent with the approach used in other files within the package and correctly implements the CommonJS module workaround described in the PR objectives.packages/xchain-bitcoincash/src/client.ts (2)
2-2: LGTM! CommonJS import workaround appliedThe change from namespace import to default import for
bitcore-lib-cashcorrectly implements the recommended workaround for CommonJS modules in ESM environments, consistent with other files in the package.
6-6: LGTM! Import path updated for ESM compatibilityThe addition of the
.jsextension to thecoinselect/accumulativeimport path correctly addresses ESM module resolution requirements, consistent with similar changes across other xchain packages.packages/xchain-kujira/src/client.ts (1)
17-17: ESM compatibility fix looks good!The addition of the
.jsextension to thecosmjs-typesimport path correctly addresses ECMAScript module resolution requirements. This change aligns with the PR objectives for fixing CommonJS import issues and ensuring ESM compatibility.packages/xchain-thorchain/src/clientKeystore.ts (1)
16-16: ESM compatibility fix is correct.The
.jsextension addition to theTxRawimport path ensures proper ESM module resolution. This change is consistent with the ecosystem-wide update for ESM compatibility.packages/xchain-thorchain/src/client.ts (1)
16-16: Approved: ESM import path fix.Adding the
.jsextension to theTxRawimport path correctly addresses ESM module resolution requirements. This change is part of the coordinated effort to ensure ESM compatibility across the xchainjs ecosystem.packages/xchain-cosmos/src/client.ts (1)
15-15: ESM compatibility update approved.The
.jsextension addition to the import path ensures compliance with ESM module resolution rules. This change maintains consistency with the broader ecosystem update for ESM compatibility.packages/xchain-thorchain/src/clientLedger.ts (1)
10-11: ESM compatibility fixes for both imports are correct.Both import path modifications correctly add
.jsextensions forSignModeandTxRawfromcosmjs-types. These changes ensure proper ESM module resolution and maintain consistency with the ecosystem-wide ESM compatibility update.packages/xchain-thorchain/genMsgs.sh (1)
44-46: Good ESM compatibility fix for protobufjs importsThe sed command correctly adds the
.jsextension to protobufjs imports, which aligns with the PR's objective to fix ESM compatibility issues. This is a proper workaround for the known protobufjs issue #2047.packages/xchain-thorchain/src/types/proto/MsgCompiled.d.ts (1)
639-2217: Generated protobuf TypeScript declarations look correctThese new type declarations for transaction attestation and observation functionality follow the standard protobufjs patterns. Since this is a generated file from the
genMsgs.shscript, the additions are expected and properly structured.tools/txJammer/package.json (1)
34-34: Correct bignumber.js version update for ESM compatibilityUpdating to version 9.1.2 ensures proper ESM support and fixes the TypeScript type definitions that were broken in earlier versions, as mentioned in the PR objectives.
examples/estimate-swap/package.json (1)
25-25: Consistent bignumber.js version updateThis update to 9.1.2 is consistent with the ecosystem-wide change for ESM compatibility.
examples/liquidity/package.json (1)
33-33: Consistent bignumber.js version updateThis update maintains consistency across all packages in the ecosystem for ESM compatibility.
packages/xchain-mayachain/src/types/proto/MsgCompiled.js (1)
4-4: No duplicate protobuf runtimes detectedA scan of the
packages/directory shows only the.jsspecifier is used:• packages/xchain-mayachain/src/types/proto/MsgCompiled.js → require("protobufjs/minimal.js")
• packages/xchain-thorchain/src/types/proto/MsgCompiled.js → require("protobufjs/minimal.js")
• BothgenMsgs.shscripts apply the same.jssuffix replacement viasedSince there are no remaining
require("protobufjs/minimal")usages, all generated files consistently load the same runtime and there’s no risk of dual singletons. No further action required.examples/check-tx/package.json (1)
20-24: Dependency version syncedChange is trivial; no concerns.
packages/xchain-bsc/package.json (1)
44-44: Validate lockfile + downstream compile/tests after bumping to^9.1.2.The caret range will admit any future
9.xreleases.
Please make sure the workspace lockfile is re-generated (yarn install --immutable) and that every package compiling against BigNumber still passes unit & type tests – v9.1 introduced the ESM fix but may pull in later minors with additional surface-area changes.packages/xchain-mayachain/package.json (1)
50-50: LGTM! bignumber.js version update for ESM compatibility.The upgrade to version 9.1.2 addresses the ESM compatibility issues mentioned in the PR objectives, specifically fixing TypeScript type definitions for ESM support.
packages/xchain-thorchain/package.json (1)
53-53: LGTM! Consistent bignumber.js version update.This update maintains consistency across the xchainjs ecosystem and ensures ESM compatibility.
packages/xchain-evm/package.json (1)
44-44: LGTM! Ecosystem-wide bignumber.js consistency maintained.The version update aligns with the coordinated ESM compatibility effort across all xchainjs packages.
packages/xchain-avax/package.json (1)
44-44: LGTM! Final package completes the coordinated update.This completes the consistent bignumber.js version update across all reviewed xchainjs packages.
packages/xchain-mayachain/src/client.ts (1)
25-25: LGTM! Import path fix addresses CommonJS compatibility issue.Adding the
.jsextension resolves the ESM import issue with CommonJS modules as described in the PR objectives. This follows the recommended workaround for packages that don't support named exports in ESM environments.
| @@ -1 +1 @@ | |||
| declare module 'coinselect/accumulative' | |||
| declare module 'coinselect/accumulative.js' | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify if this is an intentional duplicate file.
The module declaration update for ESM compatibility is correct, but the filename "modules.d copy.ts" suggests this might be an unintentional duplicate. Please confirm if this file should exist alongside the original modules.d.ts or if it should be removed.
🏁 Script executed:
#!/bin/bash
# Check if there's an original modules.d.ts file in the same directory
fd "modules.d.ts" packages/xchain-dash/src/Length of output: 81
Remove unintended duplicate modules.d copy.ts
The file packages/xchain-dash/src/modules.d copy.ts appears to be an accidental copy of modules.d.ts and should be removed. The ESM module declaration for 'coinselect/accumulative.js' is already correctly handled in modules.d.ts.
• Delete: packages/xchain-dash/src/modules.d copy.ts
🤖 Prompt for AI Agents
In packages/xchain-dash/src/modules.d copy.ts at line 1, this file is an
unintended duplicate of modules.d.ts and contains redundant module declarations.
Remove the entire file packages/xchain-dash/src/modules.d copy.ts to avoid
confusion and duplication, as the necessary declarations are already present in
modules.d.ts.
| # Fix import to be ESM-compatible (no omitted file extension) | ||
| sed -i -E 's|"(protobufjs/minimal)"|"\1.js"|' "$MSG_COMPILED_OUTPUTFILE" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
sed -i -E is not portable across BSD/macOS – consider a cross-platform variant
On GNU sed (typical in CI images) the current command is fine, but BSD sed (macOS) requires the backup-suffix immediately after -i, e.g. sed -i '' -E ….
If any contributors run this script locally on macOS it will abort with
sed: -i may not be used with -e. Switching to a portable approach avoids surprises.
-# Fix import to be ESM-compatible (no omitted file extension)
-sed -i -E 's|"(protobufjs/minimal)"|"\1.js"|' "$MSG_COMPILED_OUTPUTFILE"
+# Fix import to be ESM-compatible (no omitted file extension)
+# Works on both GNU and BSD sed.
+sed -E 's|"(protobufjs/minimal)"|"\1.js"|' "$MSG_COMPILED_OUTPUTFILE" \
+ | tee "$MSG_COMPILED_OUTPUTFILE.tmp" \
+ && mv "$MSG_COMPILED_OUTPUTFILE.tmp" "$MSG_COMPILED_OUTPUTFILE"Alternatively, use perl -pe or node -e for complete portability.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Fix import to be ESM-compatible (no omitted file extension) | |
| sed -i -E 's|"(protobufjs/minimal)"|"\1.js"|' "$MSG_COMPILED_OUTPUTFILE" | |
| # Fix import to be ESM-compatible (no omitted file extension) | |
| # Works on both GNU and BSD sed. | |
| sed -E 's|"(protobufjs/minimal)"|"\1.js"|' "$MSG_COMPILED_OUTPUTFILE" \ | |
| | tee "$MSG_COMPILED_OUTPUTFILE.tmp" \ | |
| && mv "$MSG_COMPILED_OUTPUTFILE.tmp" "$MSG_COMPILED_OUTPUTFILE" |
🤖 Prompt for AI Agents
In packages/xchain-mayachain/genMsgs.sh around lines 20 to 21, the sed command
using `-i -E` is not portable to BSD/macOS because BSD sed requires a backup
suffix immediately after `-i`. To fix this, modify the sed command to include an
empty backup suffix like `-i '' -E` for macOS compatibility, or replace the sed
command with a portable alternative such as a `perl -pe` or `node -e` one-liner
to ensure cross-platform compatibility.
| "dependencies": { | ||
| "bignumber.js": "^9.0.0" | ||
| "bignumber.js": "^9.1.2" | ||
| }, |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Resolved conflict in packages/xchain-bitcoincash/src/utils.ts: - Kept master's path fix: './bchaddrjs' - Kept ESM's import fix: 'import bitcore from bitcore-lib-cash'
- Improve changeset description to detail all ESM compatibility fixes - Document import pattern changes and version updates - Explain the purpose: enable library usage in ESM environments
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.changeset/ripe-heads-stop.md (1)
26-35: Strengthen the note with the actual CJS workaround, protobuf.js reference, and the bignumber.js nuance.This helps downstream users understand the practical changes and why 9.1.2 matters.
Apply the following edits to the bullets for added clarity:
Fix ESM (ECMAScript Module) compatibility issues - Update bignumber.js to 9.1.2 for proper ESM support -- Change bitcore-lib-cash imports from namespace to default imports for ESM compatibility +- Change bitcore-lib-cash imports from namespace to default imports for ESM compatibility - Change @dashevo/dashcore-lib imports from namespace to default imports for ESM compatibility - Add .js extensions to coinselect/accumulative imports for ESM - Add .js extensions to cosmjs-types imports for ESM - Update module type declarations for ESM compatibility - Regenerate protobuf files with correct ESM import patterns + +Additional context and examples: + +- For CJS-only deps (e.g., bitcore-lib-cash, @dashevo/dashcore-lib), use default import + destructuring: + + ```ts + import pkg from 'bitcore-lib-cash' + const { Networks, Transaction } = pkg + ``` + +- protobufjs: apply the known workaround (protobuf.js#2047) by importing from 'protobufjs/minimal.js' in generated code. + +- bignumber.js: ESM landed in 9.1.0, and TypeScript ESM typings were fixed in 9.1.2; we require >=9.1.2 to avoid TS issues.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (20)
.changeset/ripe-heads-stop.md(1 hunks)examples/check-tx/package.json(1 hunks)examples/estimate-swap/package.json(1 hunks)examples/liquidity/package.json(1 hunks)packages/xchain-arbitrum/package.json(1 hunks)packages/xchain-avax/package.json(1 hunks)packages/xchain-base/package.json(1 hunks)packages/xchain-bitcoincash/src/utils.ts(1 hunks)packages/xchain-bsc/package.json(1 hunks)packages/xchain-cosmos/package.json(1 hunks)packages/xchain-ethereum/package.json(1 hunks)packages/xchain-evm/package.json(1 hunks)packages/xchain-mayachain-query/package.json(1 hunks)packages/xchain-mayachain/package.json(1 hunks)packages/xchain-mayachain/src/types/proto/MsgCompiled.js(1 hunks)packages/xchain-thorchain-query/package.json(1 hunks)packages/xchain-thorchain/package.json(1 hunks)packages/xchain-util/package.json(1 hunks)packages/xchain-wallet/package.json(1 hunks)tools/txJammer/package.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- examples/estimate-swap/package.json
🚧 Files skipped from review as they are similar to previous changes (18)
- packages/xchain-thorchain-query/package.json
- packages/xchain-thorchain/package.json
- packages/xchain-mayachain-query/package.json
- packages/xchain-base/package.json
- packages/xchain-ethereum/package.json
- examples/check-tx/package.json
- packages/xchain-mayachain/package.json
- packages/xchain-cosmos/package.json
- packages/xchain-mayachain/src/types/proto/MsgCompiled.js
- packages/xchain-util/package.json
- packages/xchain-bsc/package.json
- packages/xchain-bitcoincash/src/utils.ts
- packages/xchain-arbitrum/package.json
- packages/xchain-evm/package.json
- packages/xchain-wallet/package.json
- examples/liquidity/package.json
- packages/xchain-avax/package.json
- tools/txJammer/package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (1)
.changeset/ripe-heads-stop.md (1)
26-36: Release note reads clearly and captures the intent.Good, concise summary of the ESM-related workarounds and dependency bump. This will help consumers understand why the patch releases are being cut.
| '@xchainjs/xchain-mayachain-query': patch | ||
| '@xchainjs/xchain-thorchain-query': patch | ||
| '@xchainjs/xchain-mayachain': patch | ||
| '@xchainjs/xchain-thorchain': patch | ||
| '@xchainjs/xchain-arbitrum': patch | ||
| '@xchainjs/xchain-ethereum': patch | ||
| '@xchainjs/xchain-litecoin': patch | ||
| '@xchainjs/xchain-bitcoin': patch | ||
| 'xchainjs-estimate-swap': patch | ||
| '@xchainjs/xchain-cosmos': patch | ||
| '@xchainjs/xchain-kujira': patch | ||
| '@xchainjs/xchain-wallet': patch | ||
| '@xchainjs/xchain-avax': patch | ||
| '@xchainjs/xchain-base': patch | ||
| '@xchainjs/xchain-doge': patch | ||
| '@xchainjs/xchain-util': patch | ||
| '@xchainjs/xchain-bsc': patch | ||
| '@xchainjs/xchain-evm': patch | ||
| 'xchainjs-liquidity': patch | ||
| 'xchainjs-check-tx': patch | ||
| 'xchainjs-loans': patch | ||
| 'txJammer': patch | ||
| --- |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify all package names exist (and match casing) in the workspace before merging.
If any entry here doesn’t exactly match a package.json "name", Changesets will skip it or error, and you’ll miss expected releases. "txJammer" in particular stands out due to uppercase letters, which is uncommon for npm package names and may not match the actual workspace name.
Run the script below to:
- List package names declared in this changeset.
- Compare against all package.json "name" fields in the repo.
- Flag unknown names, duplicates, and names with uppercase characters.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
CS_FILE=".changeset/ripe-heads-stop.md"
# 1) Extract declared package names from the frontmatter
mapfile -t DECLARED < <(awk 'BEGIN{fs=0} /^---\s*$/{fs++} fs==1 && $0 !~ /^---\s*$/{print} fs>1{exit}' "$CS_FILE" \
| sed -nE "s/^[[:space:]]*'([^']+)':[[:space:]]*(patch|minor|major)[[:space:]]*$/\1/p" \
| sort)
# 2) Extract actual workspace package names
# Avoid node_modules and other vendor dirs if present
mapfile -t ACTUAL < <(fd -t f -a package.json \
-E "**/node_modules/**" -E "**/dist/**" -E "**/build/**" \
| xargs -I{} jq -r '.name' {} | sort -u)
echo "Declared in changeset:"
printf ' - %s\n' "${DECLARED[@]}"
echo
echo "Actual package.json names:"
printf ' - %s\n' "${ACTUAL[@]}"
echo
# 3) Find declared names that don't exist in the repo
echo "Declared but not found:"
comm -23 <(printf "%s\n" "${DECLARED[@]}") <(printf "%s\n" "${ACTUAL[@]}") | sed 's/^/ - /' || true
echo
# 4) Duplicates within the changeset
echo "Duplicates in changeset frontmatter:"
printf "%s\n" "${DECLARED[@]}" | uniq -d | sed 's/^/ - /' || true
echo
# 5) Names with uppercase letters in changeset
echo "Names with uppercase letters in changeset:"
printf "%s\n" "${DECLARED[@]}" | grep -nE '[A-Z]' | sed 's/^/ - /' || echo " - none"Length of output: 407
I’ve adjusted the verification script to avoid process-substitution and ensure we reliably extract both the declared and actual package names. Please run this instead:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
CS_FILE=".changeset/ripe-heads-stop.md"
# 1) Extract declared package names into declared.txt
awk '
BEGIN { in_front = 0 }
/^---/ {
in_front++
next
}
in_front == 1 && !/^---/ {
if (match($0, /^'\''([^'\'']+)'\'' *: *(patch|minor|major)/, m)) {
print m[1]
}
}
in_front > 1 { exit }
' "$CS_FILE" | sort > declared.txt
# 2) Extract actual workspace package names into actual.txt
find . -type f -name 'package.json' \
-not -path '*/node_modules/*' \
-not -path '*/dist/*' \
-not -path '*/build/*' \
-print0 | xargs -0 -I{} jq -r '.name' {} | sort -u > actual.txt
# 3) Report results
echo "Declared in changeset:" && sed 's/^/ - /' declared.txt
echo
echo "Actual package.json names:" && sed 's/^/ - /' actual.txt
echo
echo "Declared but not found:" && comm -23 declared.txt actual.txt | sed 's/^/ - /' || true
echo
echo "Duplicates in changeset frontmatter:" && uniq -d declared.txt | sed 's/^/ - /' || true
echo
echo "Names with uppercase letters in changeset:" && grep -nE '[A-Z]' declared.txt | sed 's/^/ - /' || echo " - none"Length of output: 3224
Review frontmatter package names need correction before merging
Please address the following before merging:
- The entry xchainjs-loans is declared in
.changeset/ripe-heads-stop.mdbut nopackage.jsonin the workspace has that name. Either remove it or correct it to match the actual package name. - All other entries map exactly to existing packages—but note that txJammer is intentionally uppercase in
package.json, which is uncommon for npm packages. Ensure this casing is deliberate and matches across both the workspace and changeset.
🤖 Prompt for AI Agents
.changeset/ripe-heads-stop.md around lines 2 to 24: the changeset lists a
package "xchainjs-loans" that does not exist in the workspace and may break the
release; remove that entry or replace it with the exact package name found in
the repo's package.json (match casing and scope exactly). Also verify the
"txJammer" entry matches the package.json casing — if package.json uses
"txJammer" keep it, otherwise correct the changeset to the exact package name
used in the workspace so all entries map 1:1 to actual packages.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/xchain-dash/src/modules.d.ts (2)
2-2: Optionally add a typed surface for accumulative to catch misuse at compile-timeRight now the ambient module is
any, so bad call sites won’t be flagged. If you want lightweight safety without pulling in external typings, consider replacing the bare declaration with a minimal signature that aligns with coinselect’s accumulative usage (UTXOs in, targets out, feeRate in, result or null).Apply this diff:
-declare module 'coinselect/accumulative.js' +declare module 'coinselect/accumulative.js' { + export type UTXO = { + txId: string + vout: number + value: number + [k: string]: unknown + } + export type Target = { + address?: string + value: number + [k: string]: unknown + } + export type CoinSelectResult = { + inputs: UTXO[] + outputs: Array<Target & { value: number }> + fee: number + } | null + + const accumulative: ( + utxos: UTXO[], + outputs: Target[], + feeRate: number, + options?: Record<string, unknown> + ) => CoinSelectResult + + export default accumulative +}
1-1: Optional: tighten dashcore-lib typing to discourage named importsGiven the runtime requirement to use a default import for
@dashevo/dashcore-lib(CJS), you can declare only a default export here so TS will error onimport { Network } ...and nudge callers to the safe pattern.Apply this diff (only if the rest of the repo already uses default import + destructuring):
-declare module '@dashevo/dashcore-lib' +declare module '@dashevo/dashcore-lib' { + const dashcore: any + export default dashcore +}If there are existing named imports, hold off to avoid breaking changes; consider doing this in a follow-up PR to align with the ESM/CJS guidance in this PR’s description.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/xchain-dash/src/modules.d copy.ts(0 hunks)packages/xchain-dash/src/modules.d.ts(1 hunks)packages/xchain-thorchain/genMsgs.sh(1 hunks)
💤 Files with no reviewable changes (1)
- packages/xchain-dash/src/modules.d copy.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/xchain-thorchain/genMsgs.sh
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (2)
packages/xchain-dash/src/modules.d.ts (2)
2-2: Good ESM interop fixAdding the explicit
.jssubpath keeps Node’s ESM resolver happy for the CJS-only coinselect submodule. Matches the repo-wide import standardization noted in the PR.
1-2: Sanity checks passed: no extensionless imports & TS interop flags consistent
- Ran a repo‐wide search for any
coinselect/accumulativeimports without the.jsextension—none were found, confirming there are no lingering extensionless imports.- Confirmed all
coinselect/accumulativeimports use the.jssuffix, in line with thedeclare module 'coinselect/accumulative.js'inpackages/xchain-dash/src/modules.d.ts.- Verified that the root
tsconfig.jsonenablesesModuleInterop: true,allowSyntheticDefaultImports: true, andmoduleResolution: node, and thatpackages/xchain-dash/tsconfig.jsonsimply extends the root config—ensuring uniform TS interop settings across the repository.No further action required.
4 participants
Importing from CJS-only packages, namespace imports only work if named imports work, and named imports only work if the static analysis heuristic that Node.js uses can find them, which it can't, in the case of
bitcore-lib-cashand@dashevo/dashcore-lib. (Static analysis can find them for the more simply-writtenbchaddr.)https://nodejs.org/api/esm.html#commonjs-namespaces
bignumber.jshad busted TypeScript type definitions (for ESM) until v9.1.2. v9.1.0 was the first version with any ESM support at all.Apply a workaround for protobufjs/protobuf.js#2047 in generated code.
Prerequisites for #1410
(I'm guessing thorchain and mayachain protobuf code hasn't been regenerated in a while, because a lot of new code suddenly appeared in
MsgCompiledre-running the script.)Summary by CodeRabbit
Bug Fixes
Chores
Tests