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: Fix typings export for web3-providers-ws #3215

Closed
wants to merge 1 commit into from

Conversation

daffl
Copy link
Contributor

@daffl daffl commented Nov 15, 2019

The WebsocketProvider class is set as module.exports in https://github.com/ethereum/web3.js/blob/1.x/packages/web3-providers-ws/src/index.js#L417 which makes it a default export.

Currently, if you use the typings published in v1.2.4 you will get an error like TypeError: WebsocketProvider is not a constructor when trying to use it.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 84.347% when pulling d806423 on bidalihq:bidalihq/ws-provider into 5c78c2b on ethereum:1.x.

@nivida nivida added 1.x 1.0 related issues Types Incorrect or missing types labels Nov 18, 2019
@nivida nivida self-requested a review November 28, 2019 13:49
@nivida nivida added the Review Needed Maintainer(s) need to review label Nov 28, 2019
@alcuadrado
Copy link

Hey @fmiras, does this fix what you mentioned me some time ago?

@fmiras
Copy link

fmiras commented Dec 10, 2019

Hey @fmiras, does this fix what you mentioned me some time ago?

Partially yes, but this same thing is happening with some other imports of Web3. In fact, with this we will have some modules without default export and other bunch of them within it.

@alcuadrado
Copy link

alcuadrado commented Dec 12, 2019

Thanks for your reply @fmiras !

@nivida @cgewecke, FYI, what is happning right now is that type declarations use named exports, and the actual js files just reassign module.exports. They should use this pattern instead: https://www.typescriptlang.org/docs/handbook/declaration-files/templates/module-class-d-ts.html

Note that fixing this is a breaking change for TS users, but their current situation is kinda shitty. They are forced to do things like this unless they use esModuleInterop:

import * as Web3 from 'web3';
import { Eth } from 'web3-eth';

const client: Eth = new (Web3 as any)(clientHost).eth;

@nivida
Copy link
Contributor

nivida commented Dec 13, 2019

@alcuadrado Yes, but we have the required configurations documented here. It will run without doing anything special if those configuration properties are defined as documented.

But yes you are right it’s probably worth to update our exports from our type definitions if such a change doesn't break existing DApps with those documented configuration properties set.

@joshstevens19 What do you think about as creator of those type definitions?

@fmiras
Copy link

fmiras commented Dec 16, 2019

IMHO web3 should work out of the box with ts-node (no tsconfig.json)

@github-actions
Copy link

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.

@github-actions github-actions bot added the Stale Has not received enough activity label Jul 27, 2020
@github-actions github-actions bot closed this Aug 3, 2020
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 Review Needed Maintainer(s) need to review Stale Has not received enough activity Types Incorrect or missing types
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants