fix: export Web3Account, Wallet and signature related types #7374
Conversation
krzysu
requested review from
Muhammad-Altabba,
avkos,
danforbes,
jdevcs and
luu-alex
as code owners
| transactionHash: string; | ||
| }; | ||
|
|
||
| export type SignTransactionFunction = ( |
There was a problem hiding this comment.
this is not used anywhere
| signature: string; | ||
| }; | ||
|
|
||
| export type SignFunction = (data: string, privateKey: string) => SignResult; |
There was a problem hiding this comment.
this is not used anywhere
Bundle StatsHey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
Changeset No files were changed View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger
Smaller
Unchanged
|
packages/web3/src/index.ts
Outdated
| */ | ||
| export * from 'web3-errors'; | ||
| export * from 'web3-types'; | ||
| export type { Web3Account, Wallet } from 'web3-eth-accounts'; |
There was a problem hiding this comment.
if there is any better place to export these, let me know!
There was a problem hiding this comment.
all account types are exported from web3-eth-accounts but not from web3. See https://github.com/web3/web3.js/blob/7341-export-types/packages/web3-eth-accounts/src/index.ts#L42
How to solve that?
There was a problem hiding this comment.
I moved the export of these to packages/web3/src/types.ts
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## 4.x #7374 +/- ##
=======================================
Coverage 94.51% 94.51%
=======================================
Files 218 218
Lines 8557 8557
Branches 2363 2363
=======================================
Hits 8088 8088
Misses 469 469
Flags with carried forward coverage won't be shown. Click here to find out more. |
| import { FeeMarketEIP1559TxData, AccessListEIP2930TxData, TxData } from './tx/types.js'; | ||
| import { AccessListEIP2930Transaction, FeeMarketEIP1559Transaction, Transaction } from './tx'; | ||
|
|
||
| export type SignatureObject = { |
There was a problem hiding this comment.
I'll suggest to move only shared types that are used in 1+ packages in web3-types package, and for moved types re-export those from web3-eth-accounts package for backward compatibility.
There was a problem hiding this comment.
the problem is that SignatureObject is used in web3-eth-accounts but is also part of the Web3BaseWalletAccount type in web3-types.
See the type details below:
export interface Web3BaseWalletAccount {
// ..
readonly signTransaction: (tx: Transaction) => Promise<SignTransactionResult>;
readonly sign: (data: Record<string, unknown> | string) => SignResult;
//
}Also, none of the web3-eth-accounts types are exported from the umbrella web3 package, but I will reexport these for web3-eth-accounts backwards compatibility.
krzysu
force-pushed
the
7341-export-types
branch
from
2f28971 to
b886b1c
Compare
Description
Export types that represent the return values of method calls but aren’t currently available as standalone types. Specifically, this includes
Web3Account,Wallet, and signature-related types likeSignatureObject,SignResult, andSignTransactionResult.Fixes #7341
Type of change
Checklist:
npm run lintwith success and extended the tests and types if necessary.npm run test:unitwith success.npm run test:coverageand my test cases cover all the lines and branches of the added code.npm run buildand testeddist/web3.min.jsin a browser.CHANGELOG.mdfile in the root folder.