-
Notifications
You must be signed in to change notification settings - Fork 5.1k
5072 - updating bn 1.x #5074
5072 - updating bn 1.x #5074
Conversation
|
Your Render PR Server URL is https://web3-js-pr-5074.onrender.com. Follow its progress at https://dashboard.render.com/static/srv-ca75ec30tnummsfmafv0. |
Pull Request Test Coverage Report for Build 2455976180
💛 - Coveralls |
jdevcs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will also need to update https://github.com/ChainSafe/web3.js/blob/2c0324af2da467ee1acb72452f20000e916e4306/packages/web3-utils/src/utils.js#L273 and might need to remove number-to-bn dependency. also update ethers to 5.6.8
|
@jdevcs the dependancy |
|
The failing testcase that is commented is related to this issue #5071. will need to investigate further |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome!
In 4.x I saw something like that for the handling negative hex:
if (typeof value == "string" && value.includes("0x")) {
const [negative, hexValue] = value.toLocaleLowerCase().startsWith('-') ? ["-", value.slice(3)] : ["", value.slice(2)];
return new BN(negative + hexValue, 16);
}
else {
return new BN(value);
}
What would you say to refactor?
|
FYI, this turned out to be a breaking change on nonbreaking semver, as discussed in more detail in the above-linked comment, and possibly foreshadowed here. |
|
As @wbt noted, this was a breaking change. When exposing deps directly (such as |
Description
#5072
Please include a summary of the changes and be sure to follow our Contribution Guidelines.
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.