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

feat: typescript format #52

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hbriese
Copy link

@hbriese hbriese commented Nov 21, 2023

Description

Add typescript as a format option.

This allows importing the abi with the constant type in TypeScript, making the abi usable by viem.
Currently this is done either manually, using typechain or through bespoke generator scripts (example)

Example ABI

[
  {
    "inputs": [],
    "stateMutability": "nonpayable",
    "type": "constructor"
  }
]

becomes

export default [
  {
    "inputs": [],
    "stateMutability": "nonpayable",
    "type": "constructor"
  }
] as const;

Example usage (viem)

import { encodeFunctionData } from 'viem';
import ERC20 from '../abi/ERC20'; // Your exported contract abi

// Fully typed thanks to viem and the const abi
encodeFunctonData({
  abi: ERC20,
  functionName: 'transfer',
  args: ['0x...', 100n],
});

@ItsNickBarry
Copy link
Owner

This change is not compatible with the clear-abi task. For safety, files are verified to be valid ABIs before they're deleted. This is meant to prevent data loss if a user accidentally sets an incorrect ABI output directory.

Two specific problems in the clear-abi task:

  1. The .json file extension is hardcoded.
  2. The contents of the ABI are passed to the ethers Interface constructor for validation. Presumably the .ts format does not work here.

I'd rather not add branching logic for each special case, which must work consistently across multiple tasks. Testing that many branches can get rather difficult.

Maybe .ts support should be moved to a separate task, which would re-export the .json files to a separate directory. Though this probably wouldn't work with the pretty format option. Or maybe the need for .ts support is widespread enough that the extra complexity is worth it? I'm not sure, any thoughts?

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.

2 participants