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

feat: exclusive enum type inference in typegen #2146

Merged
merged 4 commits into from Apr 23, 2024

Conversation

nedsalk
Copy link
Contributor

@nedsalk nedsalk commented Apr 23, 2024

This PR adds better type inference for all typegen enums. Now, if you provide one property of an enum, you can't provide the other ones. Here's the change in a nutshell: playground.

This PR is a step towards resolving #1891, but I set it as a separate PR because it touches upon enum type inference generally, not just the Identity enum. As @danielbate alluded to here, there's no reason to not apply the same type inference for all enums.

Copy link
Member

@arboleya arboleya left a comment

Choose a reason for hiding this comment

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

How come this PR is closing #1891?

AFAIU, the API will still be:

await contract.functions.new_game({ Address: { value: walletAddress });
await contract.functions.new_game({ ContractId: { value: contractId });

Instead of:

await contract.functions.new_game({ address: walletAddress });
await contract.functions.new_game({ contractId: contractId });

@nedsalk
Copy link
Contributor Author

nedsalk commented Apr 23, 2024

@arboleya Oof, good catch. Converting to draft. Got a bit elated with the new generic behavior and forgot about that 😅

@nedsalk nedsalk marked this pull request as draft April 23, 2024 14:11
@nedsalk
Copy link
Contributor Author

nedsalk commented Apr 23, 2024

@arboleya Actually, we can treat this PR as a pre-step that can be merged as-is because it touches upon all enums, not just Identity. I'll change the description and open it for review again.

@nedsalk nedsalk marked this pull request as ready for review April 23, 2024 14:38
@nedsalk nedsalk requested a review from arboleya April 23, 2024 14:38
arboleya
arboleya previously approved these changes Apr 23, 2024
packages/abi-typegen/src/templates/common/common.hbs Outdated Show resolved Hide resolved
@nedsalk nedsalk dismissed stale reviews from petertonysmith94 and arboleya via 63de66c April 23, 2024 15:51
Co-authored-by: Peter Smith <peter@blueoceancomputing.co.uk>
Copy link
Contributor

Coverage Report:

Lines Branches Functions Statements
79.32%(+0%) 69.3%(+0%) 77.32%(+0%) 79.47%(+0%)
Changed Files:

Coverage values did not change👌.

@nedsalk nedsalk merged commit dd1b62b into master Apr 23, 2024
17 checks passed
@nedsalk nedsalk deleted the ns/feat/exclusive-enum-type-inference branch April 23, 2024 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants