[2.x] Adapt nodes to Codama v2 - #1171
lorisleiva wants to merge 1 commit into
Conversation
|
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
trevor-cortex
left a comment
There was a problem hiding this comment.
Summary
Adapts the hand-written side of @codama/nodes to the v2 node model. The src delta is small and mechanical: four helpers whose v1 concepts no longer exist are deleted (InstructionArgumentNode.ts, NestedTypeNode.ts, NumberTypeNode.ts, getAllInstructionArguments), isScalarEnum is re-expressed as "no variant carries data", the dead ENUM_VARIANT_TYPE_NODES alias is dropped, and a single new helper addTypeNodeTransforms is added as the flat-transforms replacement for re-wrapping a mapped type. The bulk of the diff is tests: 20 files for removed nodes deleted, 31 adapted (identifier, string-encoded numerics, options bags, single-string docs, flat transforms), and new coverage for every new v2 node plus two new kind-parity typetests.
I read every src diff and every test diff, and cross-checked addTypeNodeTransforms against the generated TypeNode/StandaloneTypeNode/DefinedTypeLinkNode types — every member of the TypeNode union carries transforms?, so the generic access is sound, and the "first is innermost" ordering in the docblock matches the generated field docs. The v1 → v2 mapping in the tests is faithful, the three intentional assertion changes are exactly the ones called out in the description, and the surviving InstructionNode.ts helpers (getAllInstructionsWithSubs, parseOptionalAccountStrategy) still line up with the v2 InstructionNode shape.
Things worth a look
addTypeNodeTransformsdouble-apply hazard — the helper is correct for its stated use (carrying transforms onto a fresh node), but a consumer that derives the replacement via spread ({ ...old, fields }) already hasold.transformsand would end up with them twice. Left an inline nit suggesting a one-line warning in the docblock, since this is exactly the pattern thetransformNestedTypeNodemigration in visitors will hit.packages/nodes/README.mdis now stale — it still usesnumberTypeNode('u64')/NumberTypeNodeas its worked example and links to thev1/docsspec pages. Not introduced by this PR, but this PR is what makes the example wrong for the package. Fine as a follow-up alongside the other v2 doc touch-ups, or a trivial fix here.- Changeset — removing public exports and adding
addTypeNodeTransformsis user-facing. If the 2.x line is covered by the seeded major changeset described inCONTRIBUTING.mdthen nothing is needed; otherwise a short changeset listing the removed helpers and the new one would make the eventual CHANGELOG much more useful to people migrating. I couldn't list.changeset/from here, so flagging rather than asserting.
Notes for subsequent reviewers
- The
CODAMA_ERROR__INVALID_BRANDED_STRINGassertions in the new value-node tests match on the exact human-readableexpectedstring (repeated acrossConstantNode,AccountDataValueNode,DataValueNode,EnumVariantTypeNode,IntegerValueNode,FloatValueNode). They pass today, but a wording tweak in the brand validators will fan out to six test files. Acceptable coupling for now; just be aware when the brands get touched. - Several new tests (skip-when-empty arrays, default
endian/prefix/strategy, etc.) exercise generated constructors rather than hand-written code. That's consistent with the pre-existingProgramNode.test.tspattern and cheap documentation of behaviour, so no objection — just don't expect them to catch generator regressions that CI's regenerate-and-diff check wouldn't already. - The follow-up list in the description is accurate:
CODAMA_ERROR__UNEXPECTED_NESTED_NODE_KINDis orphaned as of this PR, andunwrapTupleEnumWithSingleStructVisitor/setNumberWrappersVisitortarget kinds that no longer exist.
63686a6 to
8ca495f
Compare

This PR adapts the hand-written helpers and tests of
@codama/nodesto the v2 node model. The package is fully green: it type-checks, builds, tree-shakes, and 1113/1113 tests pass.srcDeleted helpers whose v1 concept no longer exists (all with zero surviving consumers, or consumers being rewritten when their own package is adapted):
InstructionArgumentNode.ts—structTypeNodeFromInstructionArgumentNodes/structFieldTypeNodeFromInstructionArgumentNode. Instruction arguments are nowinstructionNode.datadirectly.NestedTypeNode.ts—resolveNestedTypeNode/transformNestedTypeNode/isNestedTypeNode/assertIsNestedTypeNode. Wrapper type nodes are replaced by the flattransformsarray, so resolving is the identity and the guards collapse toisNode/assertIsNode.NumberTypeNode.ts—isSignedInteger/isUnsignedInteger/isInteger/isDecimal. Subsumed by theintegerTypeNode/floatTypeNodesplit.getAllInstructionArguments(inInstructionNode.ts) — read the removedarguments/extraArguments. Its three consumers are rewritten in their own packages to readinstructionNode.data.Reworked:
EnumTypeNode.isScalarEnum— the three variant kinds collapsed intoenumVariantTypeNodewith optionaldata; a scalar enum is one whose variants all have nodata.index.ts— dropped the exports above and the deadENUM_VARIANT_TYPE_NODESalias (its union no longer exists). The other eight legacy plural aliases stay:spec-generatorsemits them into the generated visitors.Added:
TypeNode.ts—addTypeNodeTransforms<T extends TypeNode>(typeNode, transforms): T. The v2 counterpart of re-wrapping a type in its original wrapper nodes: appendstransformsafter any the node already carries (first is innermost), omits the attribute when empty, returns a frozen copy. Lets a consumer that rebuilds a type node carry the original's transforms across without a double spread.testidentifier, string-encoded numeric values, flat transforms, singledocsstring,options-bag ergonomics). Three assertions changed intent because the v1 behaviour is gone: identifier casing is now preserved rather than camelCased (with a companion "rejects invalid identifiers" test);docsis a single string;instructionRemainingAccountsNodeis built from an identifier, notargumentValueNode.integerTypeNode,floatTypeNode,fixedPointTypeNode,durationTypeNode,enumVariantTypeNode,integerValueNode,floatValueNode,textNode,dataValueNode,accountDataValueNode,sentinelCountNode,unitNumberDisplayNode, the seven transform nodes) plusaddTypeNodeTransforms. The string-encoded value nodes assert that malformed input throwsCODAMA_ERROR__INVALID_BRANDED_STRING.Follow-ups
getAllInstructionArguments(validators,fillDefaultPdaSeedValuesVisitor, the visitors-core resolver → readinstructionNode.datafields);resolveNestedTypeNode(identity — use the node);transformNestedTypeNode(→addTypeNodeTransformswhere a fresh node replaces one that had transforms);structTypeNodeFromInstructionArgumentNodes(→node.data).CODAMA_ERROR__UNEXPECTED_NESTED_NODE_KINDin@codama/errorsis orphaned now thatassertIsNestedTypeNodeis gone — remove in a later errors touch-up.unwrapTupleEnumWithSingleStructVisitorandsetNumberWrappersVisitortarget removed kinds wholesale; the visitors PR decides delete-vs-rewrite.