refactor(dapi-client)!: convert to ESM, drop webpack/karma#3676
Closed
PastaPastaPasta wants to merge 3 commits into
Closed
refactor(dapi-client)!: convert to ESM, drop webpack/karma#3676PastaPastaPasta wants to merge 3 commits into
PastaPastaPasta wants to merge 3 commits into
Conversation
…ton/fetch/promisify shims
Non-breaking cleanup pass; package stays CJS, no public API changes, no consumer changes required.
dapi-client: replace winston with a minimal console-backed logger that preserves the same API (.error/.warn/.info/.verbose/.debug/.silly/.getForId). Drop node-fetch and the lib/test/bootstrap setimmediate shim — Node 18+ has both globally. Drop the https.Agent self-signed-TLS branch from requestJsonRpc (was Node-only; consumers wanting this must configure NODE_TLS_REJECT_UNAUTHORIZED at the app layer). Inline lodash/sample in ListDAPIAddressProvider. Add engines.node >=18.18. Remove dependencies: winston, node-fetch, lodash, bs58 (unused), node-inspect-extracted (unused). Remove devDeps: setimmediate.
dapi-grpc: inline the promisify shim in core/v0/web/CorePromiseClient.js and platform/v0/web/PlatformPromiseClient.js so the browser bundle no longer requires Node's util module. Both files document the shim so a future codegen regen does not silently reintroduce require('util').
… API
Adds lib/utils/bytes.js helper (hexToBytes/bytesToHex/base64ToBytes/bytesToBase64/concatBytes/bytesEqual) and converts all Buffer.* call sites in dapi-client lib/ to Uint8Array, with corresponding test updates. Package stays CJS.
Production exceptions where Buffer is retained: BlockHeadersReader passes Buffer to dashcore-lib's BlockHeader (its BufferReader needs .readInt32LE), and GetIdentitiesContractKeysResponse passes Buffer to wasm-dpp's Identifier.from (it explicitly requires Node Buffer).
createGrpcTransportError now handles both raw bytes (grpc-js path) and base64 strings (grpc-web path) for drive-error-data-bin, stack-bin, and dash-serialized-consensus-error-bin metadata fields, restoring the dual-format behavior that Buffer.from(x, 'base64') used to provide implicitly.
Test updates: spec files that construct expected protobuf requests now wrap .toBuffer() with new Uint8Array(...) to match production's normalization (sinon calledOnceWithExactly distinguishes Buffer from plain Uint8Array).
Breaking change for direct consumers: response object byte fields are now Uint8Array. Callers that do response.field.toString('hex') will fail — use bytesToHex(response.field) from lib/utils/bytes instead. Buffer.isBuffer(response.field) now returns false; use response.field instanceof Uint8Array.
Test results: dapi-client 315/315, wallet-lib 377/377, js-dash-sdk 60/60 — downstream consumers continue passing without modification (they exercise dapi-client mostly via mocks).
Converts @dashevo/dapi-client to pure ESM: type: module, exports map with ./lib/* wildcard, all CJS require/module.exports replaced with import/export, .js extensions on relative imports. Engine bumped to >=18.18. Drops webpack.config.js, karma.conf.js, and lib/test/karma/. Removes the @babel/core, babel-loader, browser-polyfill, karma, and webpack devDeps. Consumers must use a modern bundler (Vite/esbuild/webpack 5) which handles ESM natively. Moves 'events' npm package from devDependencies to dependencies — used by DAPIClient, ReconnectableStream, and BlockHeadersProvider for EventEmitter, and resolves correctly in both Node and browser bundlers. BREAKING: CJS consumers (require) no longer work. Downstream consumers wallet-lib (PR 4) and js-dash-sdk (PR 5) are temporarily broken between this PR merging and PRs 4/5 merging — they must land as a sequence. Dashmate is already ESM and continues to work. Test results: dapi-client 315/315. wallet-lib + js-dash-sdk fail as expected (fixed by PRs 4 + 5).
Contributor
|
Important Review skippedToo many files! This PR contains 187 files, which is 37 over the limit of 150. To get a review, narrow the scope: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (13)
📒 Files selected for processing (187)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This was referenced May 19, 2026
Member
Author
|
Closing — reopening as upstream→upstream so CI runs against secrets/runners. New PR link will be added shortly. |
Member
Author
|
Reopened as #3681 (upstream→upstream so CI runs against repo secrets/runners). |
Collaborator
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
I verified the flagged loader/interoperability changes against the checked-out befadd8e30098c41d5f25d3bc61a48fac6cffdec source and the surrounding package/runtime setup. Neither reported issue survives inspection: Mocha 11 already loads require: entries through its ESM-aware requireOrImport() path, and the @dashevo/wasm-dpp package is emitted as a CommonJS namespace object with both named exports and default, which matches the production destructuring in createGrpcTransportError.js.
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
2 participants
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
Converts
@dashevo/dapi-clientto pure ESM. Removes the webpack/karma browser-bundle infrastructure. Consumers must now use a modern bundler (Vite/esbuild/webpack 5) that handles ESM natively.This is PR 3 of 5 in the stacked series. Depends on #3674 and #3675.
Warning
This PR temporarily breaks
wallet-libandjs-dash-sdktests. Both packages currentlyrequire()dapi-client, and CJS cannot require an ESM module. PRs 4 and 5 in this stack convert those packages to ESM and unbreak them. The three PRs (3, 4, 5) should land together.What changes
dapi-client
package.json:"type": "module","exports": { ".": "./lib/index.js", "./lib/*": "./lib/*" }, dropsmain. The wildcard preserves the deep-import paths that wallet-lib and js-dash-sdk use.require/module.exportsto ESMimport/export. All relative imports include the.jsextension (ESM requirement).eventsnpm package moved from devDependencies → dependencies. Used byDAPIClient,ReconnectableStream,BlockHeadersProviderfor EventEmitter. The npmeventspackage works in both Node and browser bundlers.webpack.config.js,karma.conf.js,lib/test/karma/. Removes@babel/core,babel-loader, allkarma-*, all browser-polyfill devDeps (assert-browserify,buffer,crypto-browserify,os-browserify,path-browserify,process,stream-browserify,string_decoder,url,util,webpack,webpack-cli)..mocharc.ymlusesimport:for the ESM bootstrap.Bootstrap
lib/test/bootstrap.jsconverted to ESM with a defensiveloadDpp.default ?? loadDppunwrap (wasm-dpp is CJS; NodeNext interop sometimes wraps the default in a namespace object).Test plan
yarn workspace @dashevo/dapi-client run test:unit— 315 passing ✅yarn workspace @dashevo/wallet-lib run test:unit— 364/377, 13 failing (expected; PR 4 fixes)yarn workspace dash run test:unit— TS compile fails onCannot find module '@dashevo/dapi-client'(expected; PR 5 fixes)Breaking changes
require('@dashevo/dapi-client')breaks..jsextensions:import X from '@dashevo/dapi-client/lib/transport/ReconnectableStream.js'(was...ReconnectableStreambefore).Stack