Skip to content

[2.x] Adapt nodes to Codama v2 - #1171

Draft
lorisleiva wants to merge 1 commit into
09-21-adapt_errors_to_codama_v2from
09-22-adapt_nodes_to_codama_v2
Draft

lorisleiva wants to merge 1 commit into
09-21-adapt_errors_to_codama_v2from
09-22-adapt_nodes_to_codama_v2

Conversation

@lorisleiva

@lorisleiva lorisleiva commented Sep 22, 2026

Copy link
Copy Markdown
Member

This PR adapts the hand-written helpers and tests of @codama/nodes to the v2 node model. The package is fully green: it type-checks, builds, tree-shakes, and 1113/1113 tests pass.

src

Deleted helpers whose v1 concept no longer exists (all with zero surviving consumers, or consumers being rewritten when their own package is adapted):

  • InstructionArgumentNode.tsstructTypeNodeFromInstructionArgumentNodes / structFieldTypeNodeFromInstructionArgumentNode. Instruction arguments are now instructionNode.data directly.
  • NestedTypeNode.tsresolveNestedTypeNode / transformNestedTypeNode / isNestedTypeNode / assertIsNestedTypeNode. Wrapper type nodes are replaced by the flat transforms array, so resolving is the identity and the guards collapse to isNode/assertIsNode.
  • NumberTypeNode.tsisSignedInteger / isUnsignedInteger / isInteger / isDecimal. Subsumed by the integerTypeNode / floatTypeNode split.
  • getAllInstructionArguments (in InstructionNode.ts) — read the removed arguments/extraArguments. Its three consumers are rewritten in their own packages to read instructionNode.data.

Reworked:

  • EnumTypeNode.isScalarEnum — the three variant kinds collapsed into enumVariantTypeNode with optional data; a scalar enum is one whose variants all have no data.
  • index.ts — dropped the exports above and the dead ENUM_VARIANT_TYPE_NODES alias (its union no longer exists). The other eight legacy plural aliases stay: spec-generators emits them into the generated visitors.

Added:

  • TypeNode.tsaddTypeNodeTransforms<T extends TypeNode>(typeNode, transforms): T. The v2 counterpart of re-wrapping a type in its original wrapper nodes: appends transforms after 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.

test

  • Deleted the 20 test files for removed nodes/helpers.
  • Adapted the 31 surviving test files to v2 (identifier, string-encoded numeric values, flat transforms, single docs string, 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); docs is a single string; instructionRemainingAccountsNode is built from an identifier, not argumentValueNode.
  • Added tests for every new v2 node (integerTypeNode, floatTypeNode, fixedPointTypeNode, durationTypeNode, enumVariantTypeNode, integerValueNode, floatValueNode, textNode, dataValueNode, accountDataValueNode, sentinelCountNode, unitNumberDisplayNode, the seven transform nodes) plus addTypeNodeTransforms. The string-encoded value nodes assert that malformed input throws CODAMA_ERROR__INVALID_BRANDED_STRING.

Follow-ups

  • Consumers of the deleted helpers, rewritten as their packages are adapted: getAllInstructionArguments (validators, fillDefaultPdaSeedValuesVisitor, the visitors-core resolver → read instructionNode.data fields); resolveNestedTypeNode (identity — use the node); transformNestedTypeNode (→ addTypeNodeTransforms where a fresh node replaces one that had transforms); structTypeNodeFromInstructionArgumentNodes (→ node.data).
  • CODAMA_ERROR__UNEXPECTED_NESTED_NODE_KIND in @codama/errors is orphaned now that assertIsNestedTypeNode is gone — remove in a later errors touch-up.
  • unwrapTupleEnumWithSingleStructVisitor and setNumberWrappersVisitor target removed kinds wholesale; the visitors PR decides delete-vs-rewrite.

@changeset-bot

changeset-bot Bot commented Sep 22, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 8ca495f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Copy Markdown
Member Author

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.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@lorisleiva lorisleiva changed the title Adapt nodes to Codama v2 [2.x] Adapt nodes to Codama v2 Sep 22, 2026
@lorisleiva

Copy link
Copy Markdown
Member Author

@trevor-cortex

@trevor-cortex trevor-cortex left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

  • addTypeNodeTransforms double-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 has old.transforms and would end up with them twice. Left an inline nit suggesting a one-line warning in the docblock, since this is exactly the pattern the transformNestedTypeNode migration in visitors will hit.
  • packages/nodes/README.md is now stale — it still uses numberTypeNode('u64') / NumberTypeNode as its worked example and links to the v1/docs spec 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 addTypeNodeTransforms is user-facing. If the 2.x line is covered by the seeded major changeset described in CONTRIBUTING.md then 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_STRING assertions in the new value-node tests match on the exact human-readable expected string (repeated across ConstantNode, 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-existing ProgramNode.test.ts pattern 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_KIND is orphaned as of this PR, and unwrapTupleEnumWithSingleStructVisitor / setNumberWrappersVisitor target kinds that no longer exist.

Comment thread packages/nodes/src/TypeNode.ts
@lorisleiva
lorisleiva force-pushed the 09-22-adapt_nodes_to_codama_v2 branch from 63686a6 to 8ca495f Compare September 22, 2026 12:46
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