-
Notifications
You must be signed in to change notification settings - Fork 18
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
Render a union type for parsed instructions in js-experimental #162
Render a union type for parsed instructions in js-experimental #162
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great, thank you! Just a few comments and LGTM.
You might want to write a small test on the programsPage.test.ts
file for that union type. It should be fairly similar to that one:
kinobi/test/renderers/js-experimental/programsPage.test.ts
Lines 108 to 127 in 761b969
test('it renders an enum of all available instructions for a program', (t) => { | |
// Given the following program. | |
const node = programNode({ | |
name: 'splToken', | |
publicKey: 'TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA', | |
instructions: [ | |
instructionNode({ name: 'mintTokens' }), | |
instructionNode({ name: 'transferTokens' }), | |
instructionNode({ name: 'updateAuthority' }), | |
], | |
}); | |
// When we render it. | |
const renderMap = visit(node, getRenderMapVisitor()); | |
// Then we expect the following program instruction enum. | |
renderMapContains(t, renderMap, 'programs/splToken.ts', [ | |
'export enum SplTokenInstruction { MintTokens, TransferTokens, UpdateAuthority };', | |
]); | |
}); |
Finally, don't forget to mark this PR as a patch
update by running changeset
and providing a description of the change (you can just copy the PR title).
🦋 Changeset detectedLatest commit: 3856336 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
a4052c4
to
aed3262
Compare
Thanks Loris! Updated those, and added a test and changeset :) |
aed3262
to
3856336
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfection 🫶
This PR adds a new union type to each program, where each variant is an
instructionType
from its instruction types enum together with the parsed type. Eg:This allows consumers to parse instructions and then pass them around as eg
ParsedMplTokenAuthRulesInstruction
. They can then useinstructionType
to distinguish the variants, and then access the known parsed type. Passing aroundParsedCreateRuleSetInstruction
is insufficient because it doesn't have an instruction type field that we can discriminate on.This gives more flexibility, particularly to consumers that are dealing with parsed instructions from multiple programs.