-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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 Web3 provider typings #3824
Conversation
|
Hi there! Thank you for opening the PR! I'm not incredibly familiar with Typescript, so forgive my ignorance, but it seems to be that named exports are more preferable and should work all the same? |
They are but the corresponding JavaScript code is assigning Changing those to a named export would be a breaking change. |
@daffl Can you update this with your latest changes? This lgtm @GregTheGreek Do we need to remove the CLA bot from this? |
Cla is disabled so we should be able to merge without it |
Rebased with latest, I don't think any other changes were necessary. |
Pull Request Test Coverage Report for Build 478477210
💛 - Coveralls |
If you could just update the CHANGELOG.md, this would be good to go |
Correct me if I am wrong but if you change it to default exports now the changes are breaking for people using TS already right? I am not sure if we can do this in a minor release. |
Right now you can't use these modules directly with TS at all without ignoring the typings: import WebsocketProvider from 'web3-providers-ws';
new WebsocketProvider(connect, options) Gives you a compile time error like
And import { WebsocketProvider } from 'web3-providers-ws';
new WebsocketProvider(connect, options) Throws a runtime error with
|
Temporary solution |
This PR has been automatically marked as stale beacause it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. If you believe this was a mistake, please comment. |
hi guys, is this solved ? |
Description
This pull request fixes the typings of the
web3-provider-*
packages. The modules use the default (module.exports =
) export instead of a named export. The typing tests were only passing because the configuration aliased to the deprecatedweb3-providers
module. This means the provider modules could not be used as intended in TypeScript because the named export does not exist and caused a runtime error and the default export was not typed.Type of change
Checklist:
npm run dtslint
with success and extended the tests and types if necessary.npm run test:unit
with success.npm run test:cov
and my test cases cover all the lines and branches of the added code.npm run build
and testeddist/web3.min.js
in a browser.CHANGELOG.md
file in the root folder.