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

Using 'export =' instead of 'export default' in main typescript definition for fixing #1121 #1184

Merged
merged 1 commit into from
Nov 30, 2017

Conversation

matrushka
Copy link
Contributor

@matrushka matrushka commented Nov 19, 2017

At the moment if you are using TS and importing web3 with:

  • import Web3 from 'web3'; => TS definitions work but you get TypeError: web3_1.default is not a constructor in runtime.
  • import Web3 = require('web3'); or import * as Web3 from 'web3'; => the code works but TS definitions doesn't work and you end up with error TS2351: Cannot use 'new' with an expression whose type lacks a call or construct signature. in TS compilation.

The cause for this is TS expects a ES6 module definitions inside the index.js file. Although it's returning the class itself through CommonJS (this is the standard node behaviour, normally preprocessors like Babel can work around this but in TS it's optional and can cause some problems with environments which are not using any loaders). With this fix the typescript definitions point out the right output type (which is the class not the module) therefore it can be imported properly by using import Web3 = require('web3');. I've tried to make it work as import * as Web3 from 'web3'; which is more appropriate but typescript doesn't allow classes as direct and throws out error TS2309: An export assignment cannot be used in a module with other exported elements.

TL;DR TS doesn't allow exporting a class as the module hence creating various problems with a module compiled in CommonJS but imported as ES6. With this PR it's possible to use all the type definitions with the import Web3 = require('web3'); import instruction just like mentioned in #1121.

@coveralls
Copy link

coveralls commented Nov 19, 2017

Coverage Status

Coverage remained the same at 85.566% when pulling f653da2 on matrushka:typescript-export into eda3f5e on ethereum:1.0.

@frozeman
Copy link
Contributor

Does any of the typescripters have an opinion on that?

@connectdotz
Copy link

@frozeman this particular fix is right if we consider supporting the syntax

import Web3 = require("web3")

The way web3.js is written, in typescript term, is a module-class, which indeed should export the class with export = (there are other ways to import it in a typescript module but we don't need to get into that here)

There are many other problems with the current type definition, but it is probably beyond the scope of this PR... I think it is ok to merge this as it is, we could address other issues in a separate PR...

@frozeman frozeman merged commit 380642f into web3:1.0 Nov 30, 2017
@frozeman
Copy link
Contributor

Thanks!

@eepstein
Copy link

I don't think this is sufficient.
There's currently no clean way to import this module using es2015 settings in TypeScript.

nachomazzara pushed a commit to nachomazzara/web3.js that referenced this pull request Jun 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants