feat(cli): add additionalIdls config option - #1018
Conversation
🦋 Changeset detectedLatest commit: c6c170d The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 packages
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 |
| additionalPrograms.push(...getAllPrograms(additionalRootNode)); | ||
| } | ||
|
|
||
| return rootNode(mainRootNode.program, additionalPrograms); |
There was a problem hiding this comment.
I think the version could be lost here. (worth checking other fields as well)
getRootNodeFromIdl returns a RootNode one of two ways:
- If the IDL already is a Codama root node, it returns it as-is, preserving the whatever specified version.
- Otherwise it's an Anchor IDL, and
rootNodeFromAnchor(idl)builds a root node via therootNode().
| const parsed = await getParsedConfig({}); | ||
|
|
||
| expect(parsed.rootNode.program.name).toBe('main'); | ||
| expect(parsed.rootNode.additionalPrograms.map(p => p.name)).toEqual(['a', 'b']); |
There was a problem hiding this comment.
I would double check that version and other fields are preserved after additionalPrograms merge.
Thank you!
|
Good catch, the merge rebuilt the root node and reset version and standard. It now spreads the main root node instead, with test assertions for both. |
lorisleiva
left a comment
lorisleiva
left a comment
There was a problem hiding this comment.
Thanks, I like the idea! I've just made a comment on the execution of this idea to make it more async.
| const mainRootNode = await getRootNodeFromIdl(idlContent); | ||
| const rootNode = await mergeAdditionalIdls(mainRootNode, config.additionalIdls ?? [], configPath); |
There was a problem hiding this comment.
I would rather we implemented this as:
- Promise.all await all conversions from IDL string to
RootNodesuch that you end up with an array of RootNodes where the first one is the main one. That way all async operations are parallel. - Then have a
mergeRootNodeshelper function that destructs[head, ...tail]and adds all programs insidetailto the additional programs ofhead.
There was a problem hiding this comment.
Done — conversions now run in parallel via Promise.all and a mergeRootNodes helper merges [head, ...tail]. Thanks!
| const idlContent = await importModuleItem({ identifier: 'IDL', from: idlPath }); | ||
| const rootNode = await getRootNodeFromIdl(idlContent); | ||
| const rootNode = await mergeAdditionalIdls(idlContent, config.additionalIdls ?? [], configPath); |
There was a problem hiding this comment.
Thanks, much better. I still think it would be nice if all RootNodes were recovered in the same way. Here the main RootNode has its own importModuleItem whereas the others at in the parallel execution. Do you know what I mean?
There was a problem hiding this comment.
Good point — the main IDL now goes through the same loadIdl helper as the others, so all root nodes are recovered identically inside the same Promise.all.
lorisleiva
left a comment
lorisleiva
left a comment
There was a problem hiding this comment.
Thank you! I've just made a few final nit comments but I'm happy with the implementation. 🍺
| const visitors = (config.before ?? []).map((v, i) => parseVisitorConfig(v, configPath, i, null)); | ||
|
|
||
| return { configPath, idlContent, idlPath, rootNode, scripts, before: visitors }; | ||
| return { configPath, idlContent: mainIdl.content, idlPath, rootNode, scripts, before: visitors }; |
There was a problem hiding this comment.
out-of-scope for this PR: I don't actually think idlContent is being used. Perhaps we should consider removing it altogether or making it match the config by adding a additionalIdlContents?
|
Addressed the nits: dropped
|
4 participants
Closes #983.
Implements the
additionalIdlsapproach from your comment @lorisleiva: the config can list additional Anchor IDLs, and their programs get merged into the root node'sadditionalPrograms, so onecodama.jsongenerates clients for every program in an Anchor workspace.Includes a README entry, a test, and a changeset.