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: Utilize generic to better support Typechain types #325

Closed
wants to merge 2 commits into from

Conversation

phated
Copy link

@phated phated commented Apr 15, 2021

This adds a generic to the return result of the DeployFunction. This allows better integration with Typechain types, like so:

const WhitelistFactory = await hre.ethers.getContractFactory('Whitelist');

// Notice the `: Whitelist` here, which casts the Contract return to the Typechain
const whitelist: Whitelist = await hre.upgrades.deployProxy(WhitelistFactory, [args.address]);

await whitelist.deployed();

// We could then use custom RPC methods on Whitelist and have it fully typechecked here.

@frangio
Copy link
Contributor

frangio commented Apr 16, 2021

Thanks for the suggestion @phated! This is a great idea.

Are you sure that this will achieve the desired outcome though? I would imagine the return value should depend on the type of the factory argument.

@phated
Copy link
Author

phated commented Apr 16, 2021

I linked it into the project I'm helping on and it seemed to make the typechecker happy, but I could also see the need to cast the return value using as C (I had to do that for ethers.js but TS didn't complain here) 🤔

@frangio
Copy link
Contributor

frangio commented Apr 16, 2021

@phated I've pushed in your branch what I believe will give the right results. Can you try it out?

@phated
Copy link
Author

phated commented Apr 17, 2021

@frangio Hmm, I just linked that into my project and it doesn't work. The type inferred is still Contract. What else can we try?

@frimoldi
Copy link

frimoldi commented Mar 8, 2022

hey @frangio @phated , I was looking for this as well. The use of generic types looks good to me, but I think you also need to refactor the makeDeployProxy function, which is used to inject upgrades into the hre object.

@frimoldi
Copy link

frimoldi commented Mar 8, 2022

Something like:

export function makeDeployProxy(hre: HardhatRuntimeEnvironment): DeployFunction {
  return async function deployProxy<F extends ContractFactory>(
    ImplFactory: F,
    args: unknown[] | DeployProxyOptions = [],
    opts: DeployProxyOptions = {},
  ) {

And then cast the returning obj to the desired Contract type:

    const inst = ImplFactory.attach(proxyDeployment.address) as InstanceOf<F>;
    ...
    return inst;

@frangio
Copy link
Contributor

frangio commented Mar 8, 2022

@frimoldi That would be awesome. Can you somehow test it? If you can also submit a PR that would be great.

@frimoldi
Copy link

frimoldi commented Mar 8, 2022

@frangio yep, I will. Wanted to see if there was interest still. 👍🏽

@frimoldi
Copy link

frimoldi commented Mar 9, 2022

@frangio I've just created a new PR here #535

Tested on a hardhat project and deployProxy is now returning a typed Contract ✨

@phated
Copy link
Author

phated commented Mar 9, 2022

Closing in favor of #535

We no longer use this plugin, so I don't have a need for this anymore.

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.

3 participants