fix incorrectly versioned bn.js type export

[mrice32]

Description

The @types/bn.js package is not listed as a dependency in all packages that have an index.d.ts file that calls require("bn.js"). This causes downstream typescript users to resolve a type from a nested web3 dependency (ethereumjs-utils), which uses bn.js@5.X.X rather than bn.js@4.X.X as web3 does. The result is that web3 uses bn.js@4.X.X in js, but exports the bn.js@5.X.X type.

For users, this can result in compilation errors if using the bn.js library directly and using the objects to interact with web3. In the worst case, this can cause typescript users to make invalid calls into these BN objects due to the type mismatch.

Type of change

• Bug fix (non-breaking change which fixes an issue)

Checklist:

• I have selected the correct base branch.
• I have performed a self-review of my own code.
• I have commented my code, particularly in hard-to-understand areas.
• I have made corresponding changes to the documentation.
• My changes generate no new warnings.
• Any dependent changes have been merged and published in downstream modules.
• I ran npm run dtslint with success and extended the tests and types if necessary.
• I ran npm run test:unit with success.
• I ran npm run test:cov and my test cases cover all the lines and branches of the added code.
• I ran npm run build and tested dist/web3.min.js in a browser.
• I have tested my code on the live network.
• I have checked the Deploy Preview and it looks correct.
• I have updated the CHANGELOG.md file in the root folder.

[coveralls]

Pull Request Test Coverage Report for Build 1296720633

• 0 of 0 changed or added relevant lines in 0 files are covered.
• 264 unchanged lines in 7 files lost coverage.
• Overall coverage increased (+2.3%) to 74.836%

---

Files with Coverage Reduction	New Missed Lines	%
packages/web3-core-requestmanager/src/jsonrpc.js	1	70.0%
packages/web3-core-helpers/src/formatters.js	6	84.87%
packages/web3-core-helpers/src/errors.js	29	1.56%
packages/web3-utils/src/soliditySha3.js	34	3.43%
packages/web3-utils/src/index.js	43	31.38%
packages/web3-utils/src/utils.js	44	9.66%
packages/web3-eth-accounts/src/index.js	107	23.67%

Totals
Change from base Build 1292310514:	2.3%
Covered Lines:	3224
Relevant Lines:	4073

---

💛 - Coveralls

[spacesailor24]

Hey there! Thank you for opening this PR, it might actually be a solution to the failing Build / e2e (e2e_gnosis_dex) test

The gnosis dex repo which was renamed to gp-v1-ui uses the toWei method which supports a bn as input. It imports it from dex-js, and dex-js is just re-exporting it from web3-utils. Even though both gp-v1-ui and dex-js import and use bn.js, I didn't find it listed as a dependency in their package.jsons. However, dex-js does import dex-contracts which has bn.js listed as a dependency for version ^5.2.0

I'm not sure why this error would pop up now, but it sounds similar to the problem you were describing by not including bn.js as a dependency

[spacesailor24]

The files

• dist/web3.min.js
• dist/web3.min.js.map

shouldn't haven been updated, this only happens in release PRs

[philknows]

As mentioned in today's call, we should try to merge this for 1.6.1 @jdevcs to fix the failing tests.
@spacesailor24 will create a new issue if there's no response from PR author to request changes.

[mrice32]

As mentioned in today's call, we should try to merge this for 1.6.1 @jdevcs to fix the failing tests. @spacesailor24 will create a new issue if there's no response from PR author to request changes.

Sorry, I can make the changes requested now. Sorry for the delay!

[mrice32]

The files

• dist/web3.min.js
• dist/web3.min.js.map

shouldn't haven been updated, this only happens in release PRs

@spacesailor24 Thanks! This should be fixed now. Let me know if there's anything else that needs to be done before merging.

[spacesailor24]

Suggested change

	- - Added @types/bn.js dependency to additional packages (notably web3-utils) so the type export aligns with the js.
	- Added @types/bn.js dependency to additional packages (notably web3-utils) so the type export aligns with the js (#4418)

Extra - and missing PR number

[spacesailor24]

I'm going to take over this, some changes need to be applied, namely @types/bn.js was added as a dependency instead of a dev dependency

@mrice32 You'll still be apart of the git history, we're just trying to get the next RC out and want to merge this in to see if it fixes #4436

[mrice32]

@spacesailor24

I'm going to take over this

Sounds good!

some changes need to be applied, namely @types/bn.js was added as a dependency instead of a dev dependency

I totally could be wrong, but it was my impression that the issue was that it needed to be a regular dependency. If you are depending on this package and using typescript, you need the @types/bn.js package to be installed in the subtree below this package, and (as far as I know) dev deps are only installed during local development within this repo, not for downstream users when they install via npm. If it's not installed the require("bn.js") call within the index.d.ts file will resolve to ethereumjs-utils's version of @types/bn.js instead of the one web3 uses, which causes the issue we're seeing.

Does that make sense?

[spacesailor24]

@mrice32

Yep, I think you're right, corrected myself here. Just waiting for consensus around it from the rest of the team

Thank you for being so responsive and helpful with fixing this!