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

Provide all exported variables from index.ts #221

Closed
MicahZoltu opened this issue Jul 1, 2018 · 8 comments
Closed

Provide all exported variables from index.ts #221

MicahZoltu opened this issue Jul 1, 2018 · 8 comments
Labels
discussion Questions, feedback and general information.

Comments

@MicahZoltu
Copy link

At the moment, importing requires pulling in from a number of different sources as seen in this snippet:

import {} 'ethers'
import { AbiCoder, BigNumber, toUtf8Bytes, keccak256 } from 'ethers/utils';
import { TransactionResponse, TransactionRequest, TransactionReceipt } from 'ethers/providers/provider';

It would be nice if instead I could just get all of it from ethers. Also note, I need the import {} from 'ethers' at the moment even though I don't pull anything from it because without it, TypeScript gets confused when trying to find the packages ethers/utils and ethers/providers/provider. I think this may be a bug in TypeScript, though I don't understand it will enough to be certain.

To simplify the process of exporting everything, in index.ts you can do the following:

export * from './utils'
export * from './providers'
...

This will allow the user to do:

import { bigNumberify, BigNumber, JsonRpcProvider } from 'ethers'
@ricmoo
Copy link
Member

ricmoo commented Jul 1, 2018

This seems like it would be an awfully noisy namespace, won't it? There are also collisions, such as base64.encode and rap.encode.

I will try it out in some TypeScript samples, but the way I would prefer it to work is:

import ethers from 'ethers';

// Common things:
let provider = ethers.providers.getDefaultProvider('rinkeby');
let contract = new ethers.Contract( ... )
let wallet = new ethers.Wallet( ... )

// If you need the utils a lot:
const utils = ethers.utils;

// If you need RLP a lot:
const RLP = ethers.utils.RLP;

I am a fairly heavy user of ethers, and usually the frequency with my use of entry points is very low; usually calling a Provider, Wallet and a Contract or two, with calls to formatEther and parseEther here and there. Everything else I generally need is available on those objects.

This hierarchy is also reflected in the documentation, which will be updated this week for v4. The top level is functionality everyone often needs, at least some time. Most of the utilities and things that are somewhat buries are fairly specialized for advanced functionality.

@ricmoo ricmoo added the discussion Questions, feedback and general information. label Jul 1, 2018
@MicahZoltu
Copy link
Author

I'm not against namespacing, especially if there are collisions. I believe the import ethers from 'ethers' syntax is considered a TypeScript anti-pattern and the favored style is import { providers } from 'ethers' or import { providers as ethersProviders } 'ethers'. However, this is a stylistic choice the library user can make so not really relevant to this conversation.

The root of my problem is that not everything can actually be imported directly from 'ethers'. For example, you cannot import TransactionResponse or TransactionRequest from ethers, even via a namespaced import.
import { providers } from 'ethers' for example doesn't give access to providers.provider namespace. Similarly, import { providers.provider } from 'ethers' isn't valid syntax.

If the problem of not everything being exposed in index.ts is addressed, then my problem would be resolved. Keeping the namespaces is more of a stylistic choice and one that I can appreciate (even if for my uses it ends up being noisy).

@ricmoo ricmoo added the v4.0 label Jul 3, 2018
@ricmoo
Copy link
Member

ricmoo commented Jul 3, 2018

I see the problem you mean now. I will try a few things and see which feels more natural.

The options, I think, seem to be to expose more of the related types in the relevant modules, or possibly having a ethers.types with those exposed. I will try them out and see what differences it actually causes in the JavaScript code... I suspect it should cause very little difference, although, I may make interfaces and have the classes implement them so I can hide the constructor better.

I don't really want external things calling the constructor on TransactionResponse, for example; but maybe that is okay? I just feel usually when people try to do that, they are expecting things that the library functions fill in, and are surprised they aren't filled in.

@MicahZoltu
Copy link
Author

All I need is an interface so if I create a function that takes an ethers TransactionResponse I can assign a correct type to the parameter and get auto-complete on it as well as static type checking. I think I would actually prefer an Interface because it makes mocking significantly easier when writing tests.

@ricmoo
Copy link
Member

ricmoo commented Jul 18, 2018

Have you tried the 4.0.0-beta.1 npm package yet? The ethers.types contains all types, interfaces and abstract classes to TypeScript.

I'm going to close this now, but if you have issues with it, please feel free to re-open. :)

@ricmoo ricmoo closed this as completed Jul 18, 2018
@onethread
Copy link

@ricmoo Sorry for bringing this back from the dead. First off, great library, been really enjoying it so far. But I've been trying to type some of my functions, and I want to get access to various types, such as TypedDataDomain.

I see there used to be an ethers.types but that seems to be gone. Is the recommended method back to the original issue? E.g., import { TypedDataDomain } from "@ethersproject/abstract-signer";?

@ricmoo
Copy link
Member

ricmoo commented Aug 10, 2022

@onethread Yupp, that’s the current recommended way if you are using the specific packages. I just checked and it looks like it isn’t exported in the umbrella package. I’ll add it for the upcoming 5.7 release.

ricmoo added a commit that referenced this issue Aug 10, 2022
@ricmoo
Copy link
Member

ricmoo commented Aug 19, 2022

The missing types for EIP-712 have been added in v5.7.0.

Thanks! :)

Woodpile37 pushed a commit to Woodpile37/ethers.js that referenced this issue Jan 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Questions, feedback and general information.
Projects
None yet
Development

No branches or pull requests

3 participants