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

make useScaffoldWriteContract & useDeployedContractInfo backward compatible #1015

Merged
merged 4 commits into from
Dec 17, 2024

Conversation

technophile-04
Copy link
Collaborator

@technophile-04 technophile-04 commented Dec 17, 2024

Description:

This allows developers to consume useScaffoldWriteContract and useDeployedContractInfo hook in both ways until we make breaking changes release :

Example of useScaffoldWriteContract

Old:

  const { writeContractAsync: depeRecatedWriteAsync } = useScaffoldWriteContract("YourContract");

When using the Old way the IDE should give the warning :

Screenshot 2024-12-17 at 2 12 08 PM

New:

  const { writeContractAsync: newWriteAsync } = useScaffoldWriteContract({
    contractName: "YourContract",
    chainId: 31337,
  });

To test:

Copy this in page.tsx
"use client";

import type { NextPage } from "next";
import { useAccount } from "wagmi";
import { Address } from "~~/components/scaffold-eth";
import { useDeployedContractInfo, useScaffoldReadContract, useScaffoldWriteContract } from "~~/hooks/scaffold-eth";

const Home: NextPage = () => {
  const { address: connectedAddress } = useAccount();
  const { data: depecatedContractInfo } = useDeployedContractInfo("YourContract");
  const { data: newContractInfo } = useDeployedContractInfo({ contractName: "YourContract", chainId: 31337 });

  console.log({ depecatedContractInfo, newContractInfo });

  const { writeContractAsync: depeRecatedWriteAsync } = useScaffoldWriteContract("YourContract");

  const { writeContractAsync: newWriteAsync } = useScaffoldWriteContract({
    contractName: "YourContract",
    chainId: 31337,
  });

  const { data: currGreeting } = useScaffoldReadContract({
    contractName: "YourContract",
    functionName: "greeting",
  });

  const handleWriteWithDeperecated = async () => {
    try {
      await depeRecatedWriteAsync({
        functionName: "setGreeting",
        args: ["With deprecated write!"],
      });
    } catch {}
  };

  const handleNewWrite = async () => {
    try {
      await newWriteAsync({
        functionName: "setGreeting",
        args: ["With new write!"],
      });
    } catch {}
  };

  return (
    <>
      <div className="flex items-center flex-col flex-grow pt-10">
        <div className="px-5">
          <div className="flex justify-center items-center space-x-2 flex-col sm:flex-row">
            <p className="my-2 font-medium">Connected Address:</p>
            <Address address={connectedAddress} />
          </div>
        </div>
        <p>{currGreeting}</p>
        <div className="flex flex-col space-y-3">
          <button onClick={handleWriteWithDeperecated} className="btn btn-primary">
            Write with Deperecated
          </button>

          <button onClick={handleNewWrite} className="btn btn-primary">
            Write with New
          </button>
        </div>
      </div>
    </>
  );
};

export default Home;

Question:

Shall we also console.warn("Warning: Using useScaffoldWriteContract with a string parameter is deprecated. Please use the object parameter version instead") log this warning in browser? if people use string param?

@technophile-04 technophile-04 changed the title allow backward compatibilty for useScaffoldWriteContract make useScaffoldWriteContract & useDeployedContractInfo backward compatible Dec 17, 2024
@technophile-04 technophile-04 changed the title make useScaffoldWriteContract & useDeployedContractInfo backward compatible make useScaffoldWriteContract & useDeployedContractInfo backward compatible Dec 17, 2024
Copy link
Member

@carletex carletex left a comment

Choose a reason for hiding this comment

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

Good stuff @technophile-04 this is great!

Left a tiny comment.

Shall we also console.warn("Warning: Using useScaffoldWriteContract with a string parameter is deprecated. Please use the object parameter version instead") log this warning in browser? if people use string param?

No strong opinion here. Up to you all.

P.S. I was trying to understand why we needed the explicit return type (introduced in #931) and not sure if we needed it there, but we definitely need it here because of the overloads (requires a function declaration syntax). So all good!

@technophile-04
Copy link
Collaborator Author

P.S. I was trying to understand why we needed the explicit return type (introduced in #931) and not sure if we needed it there, but we definitely need it here because of the overloads (requires a function declaration syntax). So all good!

Ohh we didn't add any explicitly return type there, no? In https://github.com/scaffold-eth/scaffold-eth-2/pull/931/files#diff-9cc9e3c8fd6916932f4a4dd3a27f0147d2753318c962178abfa880cab12cc79e we just created a type for args obj UseScaffoldWriteConfig<TContractName> but maybe I missed the point

@damianmarti
Copy link
Member

@technophile-04 This is great!! The code looks good and it's working as expected.

Question:

Shall we also console.warn("Warning: Using useScaffoldWriteContract with a string parameter is deprecated. Please use the object parameter version instead") log this warning in browser? if people use string param?

I think the IDE messages are great, but it would be useful to give more visibility (and it's more annoying to see) by showing the warning log with the deprecated messages, too.

@rin-st
Copy link
Member

rin-st commented Dec 17, 2024

Lgtm! For vscode/cursor it even cross out deprecated variants!
image

Shall we also console.warn("Warning: Using useScaffoldWriteContract with a string parameter is deprecated. Please use the object parameter version instead") log this warning in browser? if people use string param?

No strong opinion too, but agree with Damu

@technophile-04
Copy link
Collaborator Author

Added the warning log feel free to suggest a better message!

Copy link
Member

@damianmarti damianmarti left a comment

Choose a reason for hiding this comment

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

@technophile-04 The console warn message looks good to me. Thanks!!

@carletex carletex merged commit 476010a into main Dec 17, 2024
1 check passed
@carletex carletex deleted the writeHook-backward-compatible branch December 17, 2024 14:56
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.

4 participants