Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: _jsonInterfaceMethodToString export in types #5550

Merged

Conversation

zlumer
Copy link

@zlumer zlumer commented Oct 22, 2022

Found a TypeScript export error in web3.js v1.x

web3.utils.jsonInterfaceMethodToString is exported with underscore in JS. In TS types however the underscore is not present.

Steps to reproduce:

import Web3 from "web3"

let web3 = new Web3()

console.log(`web3.utils.jsonInterfaceMethodToString: `, web3.utils.jsonInterfaceMethodToString)
// outputs: "web3.utils.jsonInterfaceMethodToString:  undefined"

console.log(`web3.utils._jsonInterfaceMethodToString: `, (web3.utils as any)._jsonInterfaceMethodToString)
// outputs: "web3.utils._jsonInterfaceMethodToString:  [Function: _jsonInterfaceMethodToString]"

This PR fixes this error in web3.js 1.x

@coveralls
Copy link

coveralls commented Oct 22, 2022

Pull Request Test Coverage Report for Build 3471596286

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 436 unchanged lines in 7 files lost coverage.
  • Overall coverage decreased (-2.3%) to 72.233%

Files with Coverage Reduction New Missed Lines %
packages/web3-core-requestmanager/src/jsonrpc.js 2 82.76%
packages/web3-core-helpers/src/formatters.js 25 80.8%
packages/web3-core-helpers/src/errors.js 31 4.41%
packages/web3-utils/src/soliditySha3.js 55 5.13%
packages/web3-utils/src/index.js 58 36.89%
packages/web3-utils/src/utils.js 96 13.31%
packages/web3-eth-accounts/src/index.js 169 23.35%
Totals Coverage Status
Change from base Build 3438136584: -2.3%
Covered Lines: 3414
Relevant Lines: 4445

💛 - Coveralls

@zlumer
Copy link
Author

zlumer commented Nov 4, 2022

Can we merge this or is anything blocking atm?

@Muhammad-Altabba
Copy link
Contributor

Thanks @zlumer for your contribution.
To merge this you need to get all the tests to pass. So, could you please investigate why there is one failing?

@zlumer
Copy link
Author

zlumer commented Nov 8, 2022

From what I can see in the logs, the problem seems to be with the infura connection.

Not sure if this has something to do with a simple one-character typings change.

Do any of the other PRs have the same problem?

>>>>>>
HTTP:MAINNET getBlock
>>>>>>
Error: Failed to connect to Infura over websockets after 10 tries
    at getBlockWithRetry (D:\a\web3.js\web3.js\windows_test\basic_usage.js:31:15)
Error: Process completed with exit code 1.

@zlumer
Copy link
Author

zlumer commented Nov 8, 2022

I just checked the Dependabot PRs (e.g. #5551) and they have the same error in the logs.

I suspect it may be the effect of The Merge and the testnet deprecation by Infura:
https://blog.infura.io/post/deprecation-timeline-for-rinkeby-ropsten-and-kovan-testnets

Copy link
Contributor

@Muhammad-Altabba Muhammad-Altabba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @zlumer,
Yes, it seems to be an irrelevant issue in the pipeline. And I created an issue for this based on your comment.
And I think this MR is good to go once we have additional approval from the team.

@Muhammad-Altabba
Copy link
Contributor

Could you please update the CHAINLOG.md file?

@zlumer
Copy link
Author

zlumer commented Nov 10, 2022

Could you please update the CHAINLOG.md file?

Done 🙂

CHANGELOG.md Outdated
@@ -609,6 +609,7 @@ Released with 1.0.0-beta.37 code base.

- Fixed types for getPastEvents (#4955) (#5260)
- Fix Log type by adding missing `removed` property (#4877)
- Fixed types for `web3.utils._jsonInterfaceMethodToString` (#5550)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kindly rebase your branch and put this under unreleased 1.8.2 . Thanks

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Should I squash commits now or during the PR merge?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

squash commits will happen during pr merge

@Muhammad-Altabba Muhammad-Altabba changed the base branch from 1.x to fix/5550/import-community-commits November 16, 2022 07:44
@Muhammad-Altabba Muhammad-Altabba merged commit 2505f87 into web3:fix/5550/import-community-commits Nov 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.x 1.0 related issues Bug Addressing a bug Review Needed Maintainer(s) need to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants