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

Add useSendTransaction, refactor useContractFunction #207

Merged
merged 4 commits into from
May 17, 2021

Conversation

Szymx95
Copy link
Contributor

@Szymx95 Szymx95 commented May 14, 2021

No description provided.

@changeset-bot
Copy link

changeset-bot bot commented May 14, 2021

🦋 Changeset detected

Latest commit: ce9ff76

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@usedapp/core Patch
@usedapp/testing Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@Szymx95 Szymx95 force-pushed the Add_useSendTransaction_hook_and_tests branch 2 times, most recently from 99dcf83 to 9f4f972 Compare May 14, 2021 16:08
import { TransactionStatus } from '../../src'
import { useEthers } from './useEthers'

async function handleTransactionState(
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 Looks like you could use this function in useContractFunction

interface Options {
signer?: Signer
transactionName?: string
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's extract common TransactionHookOptions and use the same for type for useContractFunction

Copy link
Contributor

Choose a reason for hiding this comment

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

We can than reuse connectContractToSigner as well in both functions

Copy link
Contributor

@marekkirejczyk marekkirejczyk May 14, 2021

Choose a reason for hiding this comment

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

Idea: Can we extract one more generic function (kind of inside-out approach to the current proposal), i.e.:

useSendGenericTransaction(() => Promise<TransactionResponse>, options?: Options) {
  ...
}

So that useSendTransaction would be something like:

function useSendTransaction(...) {
  return useSendGenericTransaction(() => 
    signer.sendTransaction(transactionRequest)
  )
}

and useContractFunction would be something like:

function useContractFunction(...) {
  return useSendGenericTransaction(() => 
      contractWithSigner[functionName](...args)
  )
}

@Szymx95 Szymx95 force-pushed the Add_useSendTransaction_hook_and_tests branch from 4b799f0 to 4142523 Compare May 17, 2021 07:00
@Szymx95 Szymx95 changed the title Add useSendTransaction tests and hook Add useSendTransaction, refactor useContractFunction May 17, 2021
@Szymx95 Szymx95 force-pushed the Add_useSendTransaction_hook_and_tests branch from 4142523 to f005bcb Compare May 17, 2021 07:06
const [spender, receiver, secondReceiver] = mockProvider.getWallets()

it('success', async () => {
const spenderBalance = await receiver.getBalance()
Copy link
Contributor

Choose a reason for hiding this comment

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

receiver.getBalance() -> spender.getBalance()

@marekkirejczyk marekkirejczyk merged commit 75b6ea8 into master May 17, 2021
@marekkirejczyk marekkirejczyk deleted the Add_useSendTransaction_hook_and_tests branch May 17, 2021 08:03
@bitfede
Copy link

bitfede commented May 24, 2021

Hey Guys! not sure if this is related, but I'm not able to call a contract function because I get this error:

Screen Shot 2021-05-23 at 10 58 08 PM

And here a more detailed output:

Screen Shot 2021-05-23 at 11 56 09 PM

I literally followed your documentations step by step, I defined contract and everything else exactly like it's shown in the docs.. It should work but it does not :(

Signer should be an optional variable in Transaction... Why is it interrupting the function execution? Where do I even look for Signer? Why is all this not explained in the docs? There is only a link to the ethersjs documentation but I have no clue how that ethers object fits into the useDapp framework :/ Also why should I define and create another signer object with ethers if I already have useDapp that injected a wallet instance into the web app state?

We have to update the docs asap, can you please provide an example on how to call a contract function?

Thank you so much for this clarification!

@marekkirejczyk
Copy link
Contributor

Try:

const {activate, account, library}  = useEthers();
const {state, send}  = useContractFunction(contract, 'addNewOpera', library);
....

@Szymx95
Copy link
Contributor Author

Szymx95 commented May 24, 2021

Propably wallet is not connected did you try:

const { activateBrowserWallet } = useEthers();
activateBrowserWallet()

Before using send ?

@bitfede
Copy link

bitfede commented May 24, 2021

Thanks for your reply @marekkirejczyk

Unfortunately I still get the error:
Screen Shot 2021-05-24 at 8 26 16 AM

Terminal:
Screen Shot 2021-05-24 at 8 38 03 AM
I see undefined as log output..

I realized that the issue might be that I'm using NextJS, and the code is being run also on server-side.

What could be a nextJS-friendly approach?


@Szymx95 thanks for your reply! Good observation, yes I do have the activateBrowserWallet in another component

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