Skip to content

[2.x] Rename node name attribute to identifier - #1162

Merged
lorisleiva merged 1 commit into
mainfrom
09-18-rename_node_name_attribute_to_identifier
Sep 30, 2026
Merged

lorisleiva merged 1 commit into
mainfrom
09-18-rename_node_name_attribute_to_identifier

Conversation

@lorisleiva

@lorisleiva lorisleiva commented Sep 18, 2026 •

Copy link
Copy Markdown
Member

This PR renames the v2 name node attribute to identifier across all hand-written package source — a 1:1 rename with no behavioural changes.

What changed

  • .name member accesses on renamed node kinds → .identifier (programNode, accountNode, instructionNode, errorNode, eventNode, pdaNode, constantNode, definedTypeNode, instructionAccountNode, instructionRemainingAccountsNode, providedNode, structFieldTypeNode, enumVariantTypeNode, variablePdaSeedNode, the value nodes, and all link nodes).
  • name: object-literal keys constructing those nodes → identifier:.
  • update*Visitor config reads (updates.name, argUpdate.name) → identifier, since those config types are Partial<…NodeInput> which use identifier in v2.
  • The 'name' in node narrowing sites — which type-check against the v2 Node union but are runtime no-ops (NodeSelector.ts, NodePath.ts, getDebugStringVisitor.ts default branch) — switched to 'identifier' in node / node.identifier, plus the selector example in the README.

Deliberately left untouched (not the name attribute)

  • Raw Anchor IDL reads (idl.*.name, 'name' in field), commander .name(), error-context name: keys, projected plain-object keys, and anything inside removed-node guards (argumentValueNode, resolverValueNode, instructionArgumentLinkNode) — handled when each package is adapted to v2.
  • NodeSelector's camelCase(name) !== node.identifier comparison: v2 identifiers are no longer camelCase-normalised, so the comparison semantics need a case-fold rethink. Cast to string here to keep the rename compiling; the semantic fix is a selector follow-up.

Note on removed nodes

instructionArgumentNode is a removed node in v2, but its construction sites still had name: → identifier: renamed here (harmless — they become struct fields, which use identifier). Those sites are not fully migrated; the argument→data rework happens when each package is adapted. Similarly fieldDiscriminatorNode.name → path and accountFieldValueNode → accountDataValueNode are out-of-scope shape changes left for those PRs.

Status

Mechanical rename only; packages are not yet green (numeric/transform/removed-node work follows per package). Every remaining 'name' error in src/ sits inside a removed-node branch. test/ and generated files are untouched.

@changeset-bot

changeset-bot Bot commented Sep 18, 2026 •

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: f2549a7

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

@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

Mechanical name → identifier rename across hand-written src/ in the dynamic-*, nodes-from-anchor, validators, visitors-core and visitors packages. I went through all 87 files: every changed line is a faithful 1:1 swap, the update*Visitor config reads (updates.identifier, argUpdate.identifier) line up with the Partial<…NodeInput> types, and the value-node/link-node accesses I spot-checked (accountValueNode, accountBumpValueNode, pdaSeedValueNode, link nodes) all have identifier: IdentifierString in the regenerated @codama/node-types. Removed-node branches (argumentValueNode, resolverValueNode, instructionArgumentLinkNode) are consistently left on .name as described. Formatter re-wraps look right.

Two in-scope misses the TS check can't catch

The completeness claim in the description rests on 'name' does not exist compiler errors. There are two places that use the 'name' in node narrowing pattern, which compiles cleanly against the v2 Node union but silently returns false at runtime for every node:

  1. packages/visitors-core/src/NodeSelector.ts ~L36 — if (name && (!('name' in node) || camelCase(name) !== node.name)). This means every name-bearing selector ([programNode]foo, [errorNode]${name}, [definedTypeNode]bar, the selector argument of updateAccountsVisitor/updateDefinedTypesVisitor/updateProgramsVisitor, deleteNodesVisitor, …) will match nothing. Several files in this very PR now build selectors from .identifier (deduplicateIdenticalDefinedTypesVisitor, addPdasVisitor, updateAccountsVisitor) and would be no-ops. Needs 'identifier' in node / node.identifier. (Separately, and for a follow-up rather than this PR: camelCase(name) !== node.identifier will also need a decision now that v2 identifiers aren't camelCase-normalised — case-fold/underscore-strip compare per the brand docstring, or normalise both sides.)

  2. packages/visitors-core/src/getDebugStringVisitor.ts ~L115 — the default: branch return 'name' in node ? [node.name] : [];. This is the generic fallback that prints the name for accountNode, instructionNode, definedTypeNode, pdaNode, structFieldTypeNode, enum variants, variablePdaSeedNode, accountValueNode, pdaSeedValueNode, etc. — i.e. most of the debug output. Same fix. (Anchored a nit on the link-node branch a few lines above.)

Worth a quick rg "'name' in" across packages/*/src to confirm those are the only two.

Notes for the description / follow-ups

  • instructionArgumentNode is also a removed node in v2 — InstructionNode now carries data?: TypeNode and there's no InstructionArgumentNode in @codama/node-types (the remaining packages/nodes/src/InstructionArgumentNode.ts helper imports a type that no longer exists). This PR still renames name: → identifier: on instructionArgumentNode({...}) constructions and ix.arguments[].name reads (nodes-from-anchor InstructionNode.ts/InstructionArgumentNode.ts, createSubInstructionsFromEnumArgsVisitor, setInstructionDiscriminatorsVisitor, updateInstructionsVisitor, resolve-standalone-pda, encode-instruction-arguments, …). That's harmless — those sites will become struct fields, which do use identifier — but it's not in the description's renamed-kinds list and doesn't fit the "removed-node branches are left untouched" rule either. Might be worth a line in the description so the per-package PRs don't treat them as done.
  • Other non-rename shape changes I noticed while cross-checking, all correctly out of scope here but easy to lose track of: fieldDiscriminatorNode.name → path (dynamic-parsers/discriminators.ts, getDebugStringVisitor, nodes-from-anchor fieldDiscriminatorNode('discriminator')); accountFieldValueNode → accountDataValueNode (resolve-consumed-members.ts, resolve-injected-value.ts).

For subsequent reviewers

Nothing else needs deep attention — this is the rename it says it is. Once the two 'name' in node sites are fixed I'm happy to approve.

Comment thread packages/visitors-core/src/getDebugStringVisitor.ts
@lorisleiva
lorisleiva force-pushed the 09-18-rename_node_name_attribute_to_identifier branch from 3ac2a18 to 2804f7e Compare September 18, 2026 13:41
@lorisleiva lorisleiva changed the title Rename node name attribute to identifier [2.x] Rename node name attribute to identifier Sep 22, 2026
@lorisleiva
lorisleiva added this pull request to stack #1182 September 25, 2026 16:02
An error occurred while trying to automatically change base from 09-17-regenerate_the_node_types_from_the_v2_spec to main September 30, 2026 13:46
@lorisleiva
lorisleiva deleted the branch main September 30, 2026 13:50
@lorisleiva lorisleiva closed this Sep 30, 2026
@lorisleiva lorisleiva reopened this Sep 30, 2026
@lorisleiva
lorisleiva removed this pull request from stack #1182 September 30, 2026 13:53
@lorisleiva
lorisleiva changed the base branch from 09-17-regenerate_the_node_types_from_the_v2_spec to graphite-base/1162 September 30, 2026 13:54
@lorisleiva
lorisleiva force-pushed the 09-18-rename_node_name_attribute_to_identifier branch from 2804f7e to f2549a7 Compare September 30, 2026 13:54
@lorisleiva
lorisleiva changed the base branch from graphite-base/1162 to main September 30, 2026 13:55
@lorisleiva
lorisleiva marked this pull request as ready for review September 30, 2026 14:04
@lorisleiva
lorisleiva merged commit 440814e into main Sep 30, 2026
@lorisleiva
lorisleiva deleted the 09-18-rename_node_name_attribute_to_identifier branch September 30, 2026 14:04
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