perun-network / erdstall-ts-sdk

TypeScript client SDK to interact with Erdstall.
Apache License 2.0
5 stars 2 forks source link

Change return value of `Asset.typeTag(): string` to an exhaustively checkable type at compile time #71

Closed ndzik closed 2 years ago

ndzik commented 3 years ago

Currently TypeTags is defined as an interface with the keys Amount and Tokens.

The corresponding Asset.typeTag() function returns a simple string. It would be beneficial to constrain the return type of Asset.typeTag() to be a sum type of the possible typeTags, currently "uint" | "idset".

This sum type should be compiler derived from the TypeTags type, which limits the number of changes necessary until the compiler can throw all necessary errors to make refactoring a simple task of following these errors and extend implementation. In the future all we would have to do is extend the TypeTags type and the rest gets derived on its own from tsc.

This would allow users to exhaustively switch over the Asset.typeTag() function enabling more rigorous implementations and gives confidence when updating the SDK:

    const resolveAssetAmount = (a: Asset): TypeTagsValue => {
        switch (asset.typeTag()) {
            case TypeTags.Amount:
                return utils.formatEther((a as Amount).value);
            case TypeTags.Tokens:
                return (a as Tokens).value.length.toString();
            // no default case. User gets compile time error here when an
            // updated SDK version has extended the `TypeTags` type.
        }
    };

Users are forced to use a default case currently because asset.typeTag() returns a string. Any update to the SDK will not lead to an error here and just fails at runtime, which is inconvenient.

sebastianst commented 2 years ago

solved by #109