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

ts: minor update for Provider.publicKey #2665

Open
kAsky53 opened this issue Oct 13, 2023 · 7 comments
Open

ts: minor update for Provider.publicKey #2665

kAsky53 opened this issue Oct 13, 2023 · 7 comments
Labels

Comments

@kAsky53
Copy link
Contributor

kAsky53 commented Oct 13, 2023

Summary of Changes

  • Change type of Provider.publicKey to PublicKey | null so that it can be compatible with @solana/wallet-adapter-react's WalletContextState.publicKey
  • Change type of first param of AccountClient.createInstruction from Signer into PublicKey since Signer is never used for TransactionInstruction
@acheroncrypto
Copy link
Collaborator

This is a breaking change so it's not really a "minor update".

@kAsky53
Copy link
Contributor Author

kAsky53 commented Mar 12, 2024

@acheroncrypto what do you think if we only update the first bullet?
PR is ready here - #2838

import { Provider } from "@coral-xyz/anchor";
import { useConnection, useWallet } from "@solana/wallet-adapter-react";

const { publicKey } = useWallet();
const { connection } = useConnection();

const provider: Provider = { connection, publicKey };

The above code throws this error due to mismatched type definition of publicKey.

Type 'PublicKey | null' is not assignable to type 'PublicKey | undefined'.
  Type 'null' is not assignable to type 'PublicKey | undefined'.

It would be beautiful if we could have these types matched together so that we do not create ugly code like below:

const provider: Provider = { connection, publicKey ?? undefined };

@kAsky53 kAsky53 changed the title ts: minor update for Provider.publicKey and AccountClient.createInstruction ts: minor update for Provider.publicKey Mar 12, 2024
@acheroncrypto
Copy link
Collaborator

Does this not work if you use useAnchorWallet instead of useWallet?

@kAsky53
Copy link
Contributor Author

kAsky53 commented Mar 12, 2024

Does this not work if you use useAnchorWallet instead of useWallet?

useAnchorWallet is just for the wallet is connected only. We also need to read public onchain data w/o having wallet connected.
I think this is why Provider interface has optional publicKey.

@acheroncrypto
Copy link
Collaborator

Please give an example on what you want to do.

@kAsky53
Copy link
Contributor Author

kAsky53 commented Mar 12, 2024

Please give an example on what you want to do.

For example I wanted to implment a model where it reads & writes data from and to onchain in object oriented manner.
I can initialize Program instance using useWallet() which it implemented Provider interface.
Without having wallet connected, readMyDataByAddress() works but createMyData() not because provider.publicKey is null, or wallet is not connected.

class MyData {
  constructor(readonly program: Program) {}
  get provider(): Provider {
    if (!provider.publicKey) throw Error("Please connect your wallet");
    return this.program.provider;
  }
  async readMyDataByAddress(address: PublicKey): Promise<MyDataState> {
    return this.program.account.myData.fetch(address);
  }
  async createMyData(): Promise<TransactionSignature> {
    ...
    this.provider.signTransaction(tx);
    return this.provider.sendTransaction(tx);
  }
}

Program instance can be inintialized as follows:

const program = new Program(idl, { ...useConnection(), ...useWallet() });

This will work for both wallet connected & not connected cases, while useAnchorWallet may only work for wallet connected cases.

@acheroncrypto
Copy link
Collaborator

Thanks for the example! However, I generally side with not making breaking changes if the change doesn't bring massive advantages.

I understood your problem but it doesn't seem like it would be too hard to fix it:

const wallet = useAnchorWallet();
const provider = wallet
  ? new AnchorProvider(connection, wallet, opts)
  : { connection };
const program = new Program(idl, provider);

I'm not exactly sure how much breakage #2838 causes, which is why I'm hesitant. I also prefer optional ? fields over explicit null in JS/TS in general.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants