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 idl enum fields which are snake case have proper typing to camel case #2378

Merged
merged 5 commits into from Feb 1, 2023

Conversation

pplisd
Copy link
Contributor

@pplisd pplisd commented Feb 1, 2023

solves #2374

fixes:
ts: idl enum field types properly convert from snake case to camel case

@vercel
Copy link

vercel bot commented Feb 1, 2023

@nojob1 is attempting to deploy a commit to the coral-xyz Team on Vercel.

A member of the Team first needs to authorize it.

@Henry-E
Copy link
Contributor

Henry-E commented Feb 1, 2023

Looks great, would you mind to

  • Add a change log
  • Add a small test. Just add some enums to a program tests/misc and try to load them in a typescript test it() to verify that the naming is correct.
  • Run prettier on the ts code

For my own curiosity, just so that I can understand why this is broken but the struct stuff isn't broken, would you be able to share a link to where the same parsing is done correctly for structs? I'd just like to verify they now have the same logic.

@pplisd
Copy link
Contributor Author

pplisd commented Feb 1, 2023

for why the struct ones are fine I'm not 100% sure but I think it's just because the idl json generated for structs converts the fields to camelcase whereas the enum one does not. so they were parsing the idl basically the same (see https://github.com/coral-xyz/anchor/blob/master/ts/packages/anchor/src/coder/borsh/idl.ts#L121) but the idl itself was just different.

in misc tests it doesn't even build for me because some errors which i guess fixing might be against your guys maintainer stuff but can just decide after.

@Henry-E
Copy link
Contributor

Henry-E commented Feb 1, 2023

Here's the program for misc, you could try adding some custom types maybe? I don't think you need to go all the way to actually add them to an account or load data into them since this is just to test the IDL.

program: https://github.com/coral-xyz/anchor/tree/master/tests/misc/programs/misc/src
ts test: https://github.com/coral-xyz/anchor/blob/master/tests/misc/tests/misc/misc.ts

@pplisd
Copy link
Contributor Author

pplisd commented Feb 1, 2023

i think the test uses an idl that gets built (it's not committed) im fixing it so i can add.

@Henry-E
Copy link
Contributor

Henry-E commented Feb 1, 2023

We want a test though in which the IDL gets built? Please don't commit the IDL itself.

Also the test just failed because the ts code needs prettier run on it

@vercel
Copy link

vercel bot commented Feb 1, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated
anchor-docs ⬜️ Ignored (Inspect) Feb 1, 2023 at 10:23AM (UTC)

@pplisd
Copy link
Contributor Author

pplisd commented Feb 1, 2023

idk the issue i was having was that the program had a bunch of UncheckedAccount or similar without /// Check: so i couldn't build the idl for misc. i just added the check and added my enum account and it worked. I wasn't trying to commit the idl

@Henry-E
Copy link
Contributor

Henry-E commented Feb 1, 2023

Ah right, you need to run with anchor test --skip-lint

@pplisd
Copy link
Contributor Author

pplisd commented Feb 1, 2023

ah well, i guess not anymore

seems tests worked

@Henry-E
Copy link
Contributor

Henry-E commented Feb 1, 2023

Great, looks good to me. Thanks for the PR!

@Henry-E Henry-E merged commit cb68080 into coral-xyz:master Feb 1, 2023
@Henry-E
Copy link
Contributor

Henry-E commented Feb 1, 2023

closes: #2374

@pplisd
Copy link
Contributor Author

pplisd commented Feb 2, 2023

oh cool thanks.

ohh i put my changelog in the comment not in the file lmao didn't know about that. Issue was just for the types not the actual object but i guess it's close enough.

@Henry-E
Copy link
Contributor

Henry-E commented Feb 6, 2023

So what would be a more accurate changelog?

  • ts: Fields inside enum variants weren't being converted to camelCase correctly

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

2 participants