Skip to content

Chore/Refactor Arg Types#240

Merged
SamuelQZQ merged 1 commit intoThalaLabs:mainfrom
21Chani:chore/type-refactor
Apr 15, 2025
Merged

Chore/Refactor Arg Types#240
SamuelQZQ merged 1 commit intoThalaLabs:mainfrom
21Chani:chore/type-refactor

Conversation

@21Chani
Copy link
Copy Markdown
Contributor

@21Chani 21Chani commented Apr 8, 2025

This PR will:

Refactor types aiming a new cleaner approach to make life easier when messing with abi type conversion.

Motivation

At #237 I found myself dealing with a lot troubles having to manage a refactor in files that shouldn't be touched in the pr, because of that I am splitting those small tasks to it's own pr's like this one.

Approaches explanation

I wanted to remove type conversions like this one that was being "recreated" in 3 files:
image

So I have created a map type for it that will provide type reusability and also easier type reading.
image
But I had to make it generic with HighValue because I found that sometimes the api needs to use string instead of AnyNumber, so, instead of creating two separated types, you just need to provide the type in generics of the type of Number return you want to manage.

Example:

type Uint64 = MovePrimitiveMap<string>["u64"] 
// string

or

type Uint64 = MovePrimitiveMap<AnyNumber>["u64"] 
// AnyNumber

@21Chani
Copy link
Copy Markdown
Contributor Author

21Chani commented Apr 10, 2025

Hey @SamuelQZQ!
Sorry to bother, we need to release some features by 23th, if you can give some attention on those fixes by then would be lovely.
❤️

@SamuelQZQ
Copy link
Copy Markdown
Contributor

@21Chani ok, I will make sure to take a look today!

Copy link
Copy Markdown
Contributor

@SamuelQZQ SamuelQZQ left a comment

Choose a reason for hiding this comment

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

lgtm

@SamuelQZQ SamuelQZQ merged commit 5804b2f into ThalaLabs:main Apr 15, 2025
@SamuelQZQ
Copy link
Copy Markdown
Contributor

hey @21Chani , I merged you recent pr, but finally found that it cannot pass the yarn test. You can see that after that pr, the type inference of vector function arguments are broken

SamuelQZQ added a commit that referenced this pull request Apr 15, 2025
SamuelQZQ added a commit that referenced this pull request Apr 15, 2025
@SamuelQZQ
Copy link
Copy Markdown
Contributor

@21Chani I reverted this pr. Feel free to recreate it if you find out how to fix the bug.

@21Chani
Copy link
Copy Markdown
Contributor Author

21Chani commented Apr 15, 2025

@21Chani I reverted this pr. Feel free to recreate it if you find out how to fix the bug.

Lol really, i was sure that tests was passing...

Thank you sir, doing my cardio rigjt now, will take a look why vector is breaking after it, my bad.

@21Chani
Copy link
Copy Markdown
Contributor Author

21Chani commented Apr 15, 2025

@SamuelQZQ Just opened a new pr with the fix for it #246. I believe there will not be any issues anymore, if you can also run the tests just to confirm would be good. If we could allow the tests on pull requests from people that is not in the org would avoid this too.

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.

2 participants