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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support TypeChain in deployProxy function #535

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

Conversation

frimoldi
Copy link

@frimoldi frimoldi commented Mar 9, 2022

As discussed in #325, this change allows deployProxy to returned a typed Contract instance, so the user can use things like Typechain without loosing contract's types definition.

Usage example in a hardhat script 馃憞馃徑

import { ethers, upgrades } from "hardhat";
import { Greeter__factory } from "../typechain";

async function main() {
  const signer = (await ethers.getSigners())[0];
  const factory = new Greeter__factory(signer);
  const greeter = await upgrades.deployProxy<Greeter__factory>(factory, {
    constructorArgs: ["Fran"],
  });

  await greeter.deployed();

  // this is now type safe
  await greeter.greet();
}

@frimoldi
Copy link
Author

@frangio please let me know your thoughts on this.

Copy link
Contributor

@frangio frangio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @frimoldi! This looks solid. I'd love to have a test for this but our tests in this package are written in JavaScript so I don't think we can set one up.

packages/plugin-hardhat/src/deploy-proxy.ts Outdated Show resolved Hide resolved
@frangio frangio changed the title feat: add generic type arg to deployProxy func Support TypeChain in deployProxy function Mar 21, 2022
@frangio frangio self-requested a review March 21, 2022 23:23
Copy link
Contributor

@frangio frangio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay. I realized we also have to cover the other functions like upgradeProxy, deployBeaconProxy, etc. Do you want to take a look at those?

@frimoldi
Copy link
Author

@frangio sure, I'll take a look as soon as I can.

@frangio
Copy link
Contributor

frangio commented Apr 5, 2022

@frimoldi Let us know if you can carry on with this. 馃檪

@frimoldi
Copy link
Author

frimoldi commented Apr 5, 2022

@frangio Yes. Sorry for the delay, I'll find some time this week.

@0xakihiko
Copy link

Would love this

@0xWhiteleaf
Copy link

Any update on this?

@bulbazavr1024
Copy link

@frimoldi Hey! How it's going ?)

@frankbonnet
Copy link

What's the status of this?

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

6 participants