kklas / anchor-client-gen

A tool for generating solana web3 clients from anchor IDLs.
MIT License
135 stars 21 forks source link

Pass programID as argument rather than saving it in a file #51

Closed y2kappa closed 1 year ago

y2kappa commented 2 years ago

some workflows require regenerating diff program ids depending on cluster etc, would be useful to not require it to be hardcoded in a file

kklas commented 2 years ago

You can always load the program ID through an env var for example. Alternatively you can generate multiple clients for different program IDs. I don't think that being able to change the program ID dynamically during the runtime is good enough reason to introduce the programID argument since in large majority of use cases it's fixed.

ronanyeah commented 2 years ago

You can always load the program ID through an env var for example

Can this be done after the code is generated? I also find myself needing to use different program ids with the same generated code, and it would be a lot easier if I could set it using the webpack/JS env. For example:

import { setProgramId } from "./codegen/programId";
kklas commented 2 years ago

Yes, the env vars are hard-coded into the bundle by the bundler. So if you're using CRA you can do for example:

// programId.ts
...

export const PROGRAM_ID: PublicKey = new PublicKey(process.env.REACT_APP_MY_PROGRAM_ID)

And if you build your app with:

$ REACT_APP_MY_PROGRAM_ID="<...>" npx react-scripts build

the program id you've specified with REACT_APP_MY_PROGRAM_ID will be hard-coded instead of process.env.REACT_APP_MY_PROGRAM_ID.

This way you can create different bundles for mainnet, devnet, etc.

ronanyeah commented 2 years ago

Ok thanks, would be good if there was a way to avoid manually editing the file after generation, such as anchor-client-gen --program-id-env MY_PROGRAM_ID ./target/idl/app.json ./codegen, but this will do for now.

kklas commented 2 years ago

Could be useful in some cases but I don't think it's super useful. A lot of people will probably do something like:

export const PROGRAM_ID: PublicKey = config.myProgramId

where you will have one config file with the env var logic.

Also, the PROGRAM_ID constant is not overwritten on subsequent generations so you have to edit it only the first time.

ronanyeah commented 2 years ago

Won't the importing/setup of that config module get overwritten and need to be added after every gen?

I use webpack.DefinePlugin to inject my variables, so I would have something like this:

import { PublicKey } from "@solana/web3.js";

// @ts-ignore
// eslint-disable-next-line no-undef
const MY_PROGRAM_ID: string = MY_PROGRAM_ID_;

// This constant will not get overwritten on subsequent code generations and it's safe to modify it's value.
export const PROGRAM_ID: PublicKey = new PublicKey(MY_PROGRAM_ID);

The setup lines I have in the middle are getting removed by the generator.

Maybe anchor-client-gen could have a flag to disable any programId.ts changes.

kklas commented 2 years ago

Can't you do ''' export const PROGRAM_ID: PublicKey = new PublicKey(MY_PROGRAMID); ''' ?

Either that or define all your constants in a separate ts file and import it here.

ronanyeah commented 2 years ago

The generator will strip my linting comments, and I really don't like the idea of having generated code import a project file, but whatever, I'll figure something out.

kklas commented 2 years ago

and I really don't like the idea of having generated code import a project file

Why not? I find it quite convenient.

But also making the generator not overwrite the comment could be one thing we could do.

ronanyeah commented 2 years ago

I like the idea of generated code being a deterministic dependency of a project. So the generation command is the only thing that is required when onboarding new team members. At the moment I'm gonna have to document the various subtleties of how programId.ts behaves, what it does/doesn't overwrite, and have instructions to git revert / don't commit programId.ts changes etc.

y2kappa commented 2 years ago

we need to change between devnet and mainnet which have diff program ids so this doesn't work for us ..

y2kappa commented 2 years ago

we need to change this at runtime, so ENV doesn't work either

y2kappa commented 2 years ago

I prefer it having a function of env which can default to devnet for example

ronanyeah commented 2 years ago

Yeah switching between different PROGRAM_IDs based on different RPCs seems like a pretty normal use case.

JoeHowarth commented 1 year ago

Having dynamic programIds would be very helpful

kklas commented 1 year ago

@ronanyeah @y2kappa @JoeHowarth will this work https://github.com/kklas/anchor-client-gen/pull/55 ?

y2kappa commented 1 year ago

I ended up manually implementing the same thing..

y2kappa commented 1 year ago

can we reopen? we have to maintain a forked programid.ts if we don't merge the above

kklas commented 1 year ago

@y2kappa I argue that: 1) if you have the same contract that is forked and deployed on another address, they should be treated as two different programs and codegen them separately 2) if you want to switch between the same program but deployed on mainnet / devnet, there are other ways to work around it like REACT_APP_SOLANA_NETWORK=devnet or REACT_APP_SOLANA_NETWORK=mainnet to differentiate between program ids 3) having the ability to switch between the program deployed on different networks via e.g. a select on the UI dynamically is not much better than doing 2) and doing npm start:devnet npm start:mainnet anyways

So not sure why there's a need to do this dynamically. It's just more complex for no benefit.

I'm not against it in general but nobody has produced strong enough arguments to convince me that this is necessary considering above points.

sol-mocha commented 1 year ago

I've created https://github.com/kklas/anchor-client-gen/pull/62 to address the issues in this PR.

If it is not accepted in this repo, feel free to use the fork at dcaf labs

kklas commented 1 year ago

Closed with #62