refactor(js-dash-sdk,platform-test-suite)!: convert to ESM, consume ESM dapi-client and wallet-lib#3678
Closed
PastaPastaPasta wants to merge 5 commits into
Closed
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).
Converts @dashevo/wallet-lib to pure ESM so it can consume the ESM dapi-client from PR 3. Adds 'type: module' to package.json. All src/, fixtures/, and tests/ files converted from CJS require/module.exports to ESM import/export with .js extensions on relative imports. Deletes webpack.config.js, karma/, src/test/karma/. Removes browser-polyfill devDeps (buffer, crypto-browserify, stream-browserify, etc.) and webpack/karma. Tests run via Mocha in Node 18+; browser builds are out of scope. Engines: >=18.18. CJS-named-import fixes: lodash, crypto-js, @dashevo/dashcore-lib, @dashevo/wasm-dpp, @dashevo/grpc-common all use default-import + destructure pattern because Node ESM cannot statically enumerate named exports of CJS modules. Surfaces three previously-silent CJS bugs that ESM strict mode catches: missing UnknownStrategy export from errors/index.js, missing named exports for coinSelection strategies, and a 'type=' implicit-global in DerivableKeyChain.spec.js. Defensive (loadDpp.default ?? loadDpp)() unwrap in IdentitySyncWorker.onStart for the same wasm-dpp NodeNext interop reason as PR 3's bootstrap. Test results: wallet-lib 377/377 passing. dapi-client unchanged (315/315). js-dash-sdk still broken (PR 5 fixes).
…SM dapi-client and wallet-lib
Final PR in the ESM migration stack. Converts the two remaining workspace packages that consumed dapi-client/wallet-lib, restoring monorepo-wide green tests after PR 3.
js-dash-sdk: TypeScript-based package with NodeNext module resolution. Adds 'type: module' + exports map. tsconfig.build.json → module: nodenext, moduleResolution: nodenext, target: es2020. All .ts files get .js extensions on relative imports (TS NodeNext compiles .ts to .js but the import string must already say .js). Drops ts-mock-imports by refactoring register.ts to accept an injectable randomBytes parameter (ts-mock-imports relies on mutable module exports which ESM forbids). Deletes webpack/karma + browser polyfills. Replaces winston with console-backed logger. CJS-named-import conversions for @dashevo/dashcore-lib, @dashevo/wasm-dpp, @dashevo/grpc-common, and the system-id imports from contract packages. Adds parallel 'import type' aliases (via 'import type { X as XType }; type X = XType;') in 22 .ts files where destructured value names are also used as types — works around TS2440 namespace collision under NodeNext.
platform-test-suite: converted ~35 files from CJS to ESM. Test bootstrap converted; .mocharc.yml uses 'import:' for ESM root hooks.
Defensive (loadDpp.default ?? loadDpp)() unwrap in 4 sites (Platform.initializeDppModule, createIdentityFixtureInAccount, contracts/get.spec.ts, contracts/history.spec.ts). createGrpcTransportError-style dual-format byte handling preserved.
Test results: 752 passing across the monorepo (dapi-client 315, wallet-lib 377, js-dash-sdk 60). platform-test-suite is end-to-end and requires live testnet credentials in .env (pre-existing constraint).
Contributor
|
Important Review skippedToo many files! This PR contains 262 files, which is 112 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 (38)
📒 Files selected for processing (262)
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 |
Member
Author
|
Closing — reopening as upstream→upstream so CI runs against secrets/runners. New PR link will be added shortly. |
Member
Author
|
Reopened as #3683 (upstream→upstream so CI runs against repo secrets/runners). |
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
1 participant
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
Final PR in the ESM migration stack. Converts
dash(@dashevo/js-dash-sdk) and@dashevo/platform-test-suiteto ESM. Restores monorepo-wide green tests after PR 3.This is PR 5 of 5. Depends on #3674, #3675, #3676, #3677.
What changes
js-dash-sdk
package.json:"type": "module",exportsmap,engines.node >= 18.18. Dropsunpkg/browser,test:browsers*,prepublishOnly. Removes devDeps: allkarma-*,webpack, browser polyfills,ts-mock-imports(replaced).tsconfig.build.jsonandtsconfig.json:module: nodenext,moduleResolution: nodenext,target: es2020..tssource files get.jsextensions on relative imports (TS NodeNext requirement: TS compiles.tsto.js, but the import string must already say.js).src/SDK/Client/Platform/methods/names/register.tsto accept an injectablerandomBytesparameter instead of relying onts-mock-imports. ESM module exports are frozen;ts-mock-imports-style export mutation no longer works.winstonwith console-backed logger.@dashevo/dashcore-lib,@dashevo/wasm-dpp,@dashevo/grpc-common, and the system-id imports from contract packages (@dashevo/dpns-contract,@dashevo/dashpay-contract, etc.).import type { X as XType }; type X = XType;aliases in 22.tsfiles where destructured value names are also used in type positions. Works around TS2440 namespace collision undermodule: nodenext.(loadDpp.default ?? loadDpp)()unwrap in 4 sites where wasm-dpp's CJS default is called directly (Platform.initializeDppModule,createIdentityFixtureInAccount, two contract specs).webpack.config.js,webpack.base.config.js,karma/.platform-test-suite
package.json:"type": "module", dropstest:browsers, browser polyfills,karma-*..jsfiles converted to ESM with.jsextensions on relative imports.lib/test/bootstrap.jsrewritten as ESM withfileURLToPath-based__dirname..mocharc.ymlusesimport:for ESM root hooks.karma.conf.js,lib/test/karma/.Test plan
yarn workspace @dashevo/dapi-client run test:unit— 315 passing ✅yarn workspace @dashevo/wallet-lib run test:unit— 377 passing ✅yarn workspace dash run test:unit— 60 passing ✅ (3 pending pre-existing)yarn workspace @dashevo/platform-test-suite run lint— ESM bootstrap loads.env(pre-existing constraint, unrelated to this conversion)Breaking changes
require('dash')andrequire('@dashevo/platform-test-suite')no longer work..jsextensions.Platform.names.register(...)gained an optional trailingrandomBytesparameter. Existing callers unaffected; subclasses or replacements need to know.winston,bs58,node-inspect-extractedfrom deps where unused.Monorepo state after this stack lands
@dashevo/dapi-client— ESM,Uint8ArrayAPI, browser-bundler-friendly (Vite/esbuild/webpack 5)@dashevo/wallet-lib— ESMdash— ESM TypeScript@dashevo/platform-test-suite— ESM@dashevo/dashmate— was already ESM, works as-is@dashevo/dapi-grpc— patched in PR 1; rest of package unchangedStack