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 having to cast web3.eth.Contract as unknown before casting to generated type #802

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

sigmachirality
Copy link

@sigmachirality sigmachirality commented Jan 23, 2023

Previously, when using typechain generated types, one would have to cast to unknown or any before casting to the generated Typechain type. For example:

import { ERC20 } from '../typechain';

const contract1 = new this.web3.eth.Contract(erc20Artifact as AbiItem[], tokenAddress) as unknown as ERC20;
// or:
const contract2 = new this.web3.eth.Contract(erc20Artifact as AbiItem[], tokenAddress) as any as ERC20;

This is because Typescript did not consider eth.Contract and the generated type (ERC20 in this case) overlapping. There are two reasons for this.

  • Instances of a class do not have the constructor as a member - the constructor is a member of the static Class factory. (See https://blog.logrocket.com/writing-constructor-typescript/ for more information.)
  • resultValues in an event object are typed as { [string:key]: any } in web3-v1, which does not contain the properties of generated events. For example:
export type Approval = ContractEventLog<{
  owner: string;
  spender: string;
  value: string;
  0: string;
  1: string;
  2: string;
}>;

// The expected shape of this type is something like this:
type ExpectedShape = {
  returnValues: {
    owner: string;
    spender: string;
    value: string;
    0: string;
    1: string;
    2: string;
  }
} 

// This means Typescript will throw the error: `{ [key:string]: any }` does not contain properties `owner`, `spender`, etc...
// This is because the shape given by web3-v1 implements this
type ExpectedShape = {
  returnValues: { [key:string]: any }
} 
// Which to the type system, might as well be this.
type ExpectedShape = {
  returnValues: {}
}
// This also applies for Record<string, any>. We could change web3-v1's resultValues type to `any` or `object`, but that's upstream of this repo.

Proposed Changes
First, remove the constructor from the interface implemented by class instances, and move it into a separate interface meant for typing the static Class:

import { ERC20, ERC20Constructor } from '../typechain';

const erc20Factory = this.web3.eth.Contract as ERC20Constructor
const contract1 = new erc20Factory(erc20Artifact, tokenAddress);

// Or,
const contract2 = new this.web3.eth.Contract(erc20Artifact as AbiItem[], tokenAddress) as ERC20;

The factory/constructor interface may come in handy for mocking objects in testing, typing higher order functions, etc.

Second, we want to resolve the type mismatch in event callbacks, specifically in the Once methods -
For now, we can solve this issue by making the return values optional like this:

interface ContractEventLog<T> extends EventLog {
  returnValues: Partial<T>
}

The above is a stopgap solution until I can either confirm or be denied from changing web3-v1's types upstream. It's not ideal, as we know for a fact that the event WILL contain the properties, but are forced into making them partial to satisfy web3-v1's narrow types.

A benefit of this PR is that users will be able to use the satisfies keyword going forward.

const contract2 = new this.web3.eth.Contract(erc20Artifact as AbiItem[], tokenAddress) satisfies ERC20;

This keyword provided no benefit before as casting to unknown/any destroys the underlying information of the type.

Related PRs/Issues: web3/web3.js#2461 (we may be able to get this type changed again to any or object type, which would allow us to not have to make returnValues as Partial.

Other TODOs:
Bump the version of web3-v1 required for this PR to 1.8?

@changeset-bot
Copy link

changeset-bot bot commented Jan 23, 2023

🦋 Changeset detected

Latest commit: e2694cb

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@typechain/web3-v1 Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

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.

None yet

1 participant