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

lang, ts: Add map type #916

Closed
wants to merge 2 commits into from
Closed

lang, ts: Add map type #916

wants to merge 2 commits into from

Conversation

joaompfe
Copy link

IDL support for Map types. Depends on project-serum/serum-ts#181.

Because BTreeMap has two generic types I was forced to use syn to extract the two types. Without syn, only with recursive regex, which isn't available. Since I was using syn, I refactored the complete method (if that's not desired I can put it as was).

@joaompfe
Copy link
Author

CI will fail because of project-serum/serum-ts#181.

let inner_ty = syn_type_to_idl_type(&type_args[0])?;
IdlType::Option(Box::new(inner_ty))
}
"Vec" | "VecDeque" | "LinkedList" => {
Copy link
Author

Choose a reason for hiding this comment

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

I also added support to VecDeque and LinkedList since they are serialized by borsh in the same way as Vec.

@kevinheavey
Copy link
Contributor

@armaniferrante I'm happy to write a simple integration test for this if we can get the borsh PR merged: project-serum/serum-ts#181

@armaniferrante
Copy link
Member

@armaniferrante I'm happy to write a simple integration test for this if we can get the borsh PR merged: project-serum/serum-ts#181

Sounds great. Let's write a test.

@joaompfe
Copy link
Author

joaompfe commented Dec 5, 2021

@kevinheavey maybe I am late but when writing the integration tests keep in mind that Javascript Map does key comparisons by reference (see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map#key_equality), which works fine for primitive types or values which are the same but here we end up comparing objects which are deep equal but not the same most of the time (e.g., BN objects).

@kevinheavey
Copy link
Contributor

kevinheavey commented Dec 7, 2021

@joaompfe lol actually if you're still around do you wanna add an integration test? Turns out adding to someone else's PR is messier than I thought

@LesDomen
Copy link

@joaompfe, can I help with this? I would really like to see this feature merged.

@beautyfree
Copy link

@joaompfe could be a useful feature

@joaompfe joaompfe closed this by deleting the head repository Sep 6, 2023
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

5 participants