trufflesuite / truffle

:warning: The Truffle Suite is being sunset. For information on ongoing support, migration options and FAQs, visit the Consensys blog. Thank you for all the support over the years.
https://consensys.io/blog/consensys-announces-the-sunset-of-truffle-and-ganache-and-new-hardhat?utm_source=github&utm_medium=referral&utm_campaign=2023_Sep_truffle-sunset-2023_announcement_
MIT License
14.02k stars 2.32k forks source link

Types are not readily usable on Contract object #4825

Closed wbt closed 2 years ago

wbt commented 2 years ago

Issue

When attempting to use @truffle/contract in Typescript, the default types don't permit use as documented, and have assorted other redundancies and issues.

Steps to Reproduce

In a new folder/project, npm i @truffle/contract Create a file example.ts with the following contents:

import TruffleContract from '@truffle/contract';
const getDeployedInstanceFromContract = function(truffleContract: TruffleContract.Contract) {
    return new Promise(function(resolve, reject) {
        truffleContract.deployed()
        .then(function(instance) {
            //Initial processing here which motivates use of a wrapper omitted for minimal example.
            resolve(instance);
        }).catch(function(err: unknown) {
            //Error processing here omitted for minimal example.
            reject(err);
        });
    });
};

Expected Behavior

Based on the documentation, I would expect this to compile and run without a problem.

Actual Results

The call to .deployed() has a compilation error:

This expression is not callable. No constituent of type 'string | number | boolean | any[] | { [k: string]: any; }' is callable.ts(2349)

The next line also has an error:

Parameter 'instance' implicitly has an 'any' type.ts(7006)

The second is more easily overcome but raises the issue that there should be a separate type for this (e.g. TruffleContract.Instance) and there doesn't appear to be one, though the "Contract Instance API" is broken out under a separate heading in documentation that presents a separate set of available functions. The documentation there also notes that a function call returns an object that "looks like this" but doesn't have a Type specification available in the Truffle package.

Cause Analysis

The typings for @truffle/contract create a namespace TruffleContract with just one member: 'Contract', which is a renamed version of @truffle/contract-schema/ContractObject which is a passthrough from @truffle/contract-schema/spec/ContractObject. The index.d.ts file there disables the linter and says the following:

This file was automatically generated by json-schema-to-typescript. DO NOT MODIFY IT BY HAND. Instead, modify the source JSONSchema file, and run yarn prepare inside your local packages/contract-schema

This typing file is not very useful. It has a lot of repetition in typings, for example from ABI:

//... 
 | (
      | {
          [k: string]: any;
        }
      | {
          [k: string]: any;
        }
    )
  | (
      | {
          [k: string]: any;
        }
      | {
          [k: string]: any;
        }
    )
  | {
      type: "receive";
      stateMutability: "payable";
    }
  | (
      | {
          [k: string]: any;
        }
      | {
          [k: string]: any;
        }
    )

These typings also aren't very granular, e.g. Abi doesn't allow you to have a NormalFunction vs ConstructorFunction vs. EventABI type even though those are broken out in the spec. Some are over-broad; e.g. the Solidity 'Type' string could be more strictly typed into an enum, though the tuple representation found in the json is more compact and having some scripting would help.

The absence of the documented functions is also an issue as noted above.

As a solution, I would suggest shifting away from the current script package used to generate the types file, and replacing it with either another script and/or manual generation of the file.

Environment

wbt commented 2 years ago

I should note I'm willing to contribute a first draft without regeneration script, but only if there's buy-in from the community that the autogeneration can/should be replaced.

MicaiahReid commented 2 years ago

@wbt, thanks for opening this issue. We'll do some investigating on our end, but we would be thrilled to have you write up a first draft.

wbt commented 2 years ago

As some further points of support, see e.g. in other repos comments like "The types in this package are broken, so we have to require it." Despite the documentation saying it 'works well with ES6' it doesn't appear to on this point for that reason. This StackOverflow question asks if it is possible to use @truffle/contract in typescript without require and appears to get a negative answer.

The main reason I haven't written up a fuller first draft yet is because I myself haven't yet quite figured out how to do that, and it's not 100% clear if that's because I'm missing something or if that's because the types are really so broken in this package. Do you know if anyone else has found a way to make that work?

gnidan commented 2 years ago

Oof. Looks like we're exporting the wrong types entirely. Would it be better if we just removed this file? i.e., leave it to TypeChain to provide types for @truffle/contract? At least until we can get around to properly porting that package to TS.

wbt commented 2 years ago

I'm not sure what TypeChain is doing for this project but if it's generating the code quoted in the OP, I wouldn't recommend that. I don't know what all the impacts would be of removing that file.

I do have a very hacky workaround to loading TruffleContract in global scope (and ensuring the sequential loading of dependencies like web3 and BN before that) but it's pretty bad and definitely not a simple solution.

cds-amal commented 2 years ago

@wbt we're wranggling our understanding of this issue to determine a path forward.

kevinweaver commented 2 years ago

Through a conversation with @gnidan, we decided to just remove the types for now. In the future, we'll make sure to implement the types correctly.

lsqproduction commented 2 years ago

Removed in https://github.com/trufflesuite/truffle/releases/tag/v5.5.16, will work on adding usable types at a later time.

wbt commented 2 years ago

For those who might come across this disappointed, also check out TypeChain.

benjamincburns commented 2 years ago

@wbt I'm really sorry about the experience that you've been having here with @truffle/contract and TypeScript. You may want to follow #5370 for updates on improvements, as well as the issues linked to the wider TypeScript support epic, #5194.

TypeScript support is now my primary focus. If you're willing, I'd be very appreciative if you could chime in on those issues to share your experiences and pain points that you've encountered. I want to make sure that you and others don't keep experiencing problems like the ones you've reported here once these changes get released.