✨ Make the web3 constructor parameters optional #5139
Conversation
nazarhussain
requested review from
avkos,
jdevcs,
luu-alex,
nikoulai and
spacesailor24
nazarhussain
force-pushed
the
nh/5125-optional-constructor
branch
from
8634f35 to
d7a9d08
Compare
…inSafe/web3.js into nh/5125-optional-constructor
…inSafe/web3.js into nh/5125-optional-constructor
| it('should be able use functionality with web3 object not dependant on provider', () => { | ||
| web3 = new Web3(); | ||
|
|
||
| expect(web3.utils.hexToNumber('0x5')).toBe(5); |
There was a problem hiding this comment.
Seems ok, but I was thinking we should have test that goes through multiple scenarios without provider. Like we should add ABI encoding without provider :
const web3 = new Web3();
const contract = new web3.eth.Contract(ContractABI);
await contract.methods.Func(12).encodeABI(); // and see it works
This will cover more scenarios, web3 instance creation, contract instance creation, encoding etc
There was a problem hiding this comment.
Such behaviour is better candidate for unit tests, not the integration. I had added one test case though in the integration tests.
There was a problem hiding this comment.
Only encoding param can be yes, but for scenarios that involve multiple packages, integration tests are more good. like abi encoding for eth_call of specific function of contract without provider or creating tx and offline signing etc.
There was a problem hiding this comment.
These functions pure functions, have no dependency on any related code that can effect the behavior. So these functions can be better tested in unit tests with all possible input/output. If those functions had any state which are preserved in the class then we would have gone for integration tests.
Description
Fixes #5130
Type of change
Checklist:
npm run dtslintwith success and extended the tests and types if necessary.npm run test:covand my test cases cover all the lines and branches of the added code.npm run buildwith success.dist/web3.min.jsin a browser.CHANGELOG.mdfile in the root folder.