feat(nodes): add ConstantNode support with Anchor parsing by tanmay4l · Pull Request #958 · codama-idl/codama

tanmay4l · 2025-12-12T12:17:04Z

changeset-bot · 2025-12-12T12:17:08Z

🦋 Changeset detected

Latest commit: caa2700

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 12 packages

Name	Type
@codama/visitors-core	Minor
@codama/nodes-from-anchor	Minor
@codama/node-types	Minor
@codama/nodes	Minor
@codama/cli	Patch
@codama/dynamic-codecs	Patch
@codama/dynamic-parsers	Patch
@codama/renderers-core	Patch
@codama/validators	Minor
@codama/visitors	Minor
@codama/errors	Minor
codama	Minor

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

lorisleiva · 2026-01-05T11:40:42Z

Hi, thanks for the PR!

This is looking pretty good. The only thing I'm unsure of is whether on not the ConstantNode should wrap the existing ConstantValueNode which is the existing way for us to describe a type + value combo.

On one hand, this would make the ConstantValueNode the single source of truth for handling constants (visitors just need to add an implementation for this Node and it'll work everywhere in the tree).

On the other hand, this make the ConstantNode a bit awkward because it'll access data by doing something like program.constants[0].constant.value. Maybe something that can be fixed with a better naming convention?

What do you think?

tanmay4l · 2026-01-05T12:47:40Z

Hi, thanks for the PR!

This is looking pretty good. The only thing I'm unsure of is whether on not the ConstantNode should wrap the existing ConstantValueNode which is the existing way for us to describe a type + value combo.

On one hand, this would make the ConstantValueNode the single source of truth for handling constants (visitors just need to add an implementation for this Node and it'll work everywhere in the tree).

On the other hand, this make the ConstantNode a bit awkward because it'll access data by doing something like program.constants[0].constant.value. Maybe something that can be fixed with a better naming convention?

What do you think?

hmm interesting point about ConstantValueNode. I kept them separate because program constants need name field, which ConstantValueNode doesn't have. also constants[0].constant.value feels awkward. But I see your point about reuse.
would you prefer I refactor it to wrap ConstantValueNode? happy to change if that's the direction you want

lorisleiva · 2026-03-30T19:00:35Z

Hey sorry for the late reply, I was actually gonna review this properly this week. Are you still okay keeping this PR open? ☺️

tanmay4l · 2026-03-30T19:06:48Z

Hey sorry for the late reply, I was actually gonna review this properly this week. Are you still okay keeping this PR open? ☺️

sure haha ( ill resolve conflict )

lorisleiva

Thanks for this PR and again sorry for the super later review.

I've made a few comments, lmk what you think.

I also think you're missing an important test file in the packages/visitors-core/test/nodes folder (e.g. look at the packages/visitors-core/test/nodes/DefinedTypeNode.test.ts file for reference).

P.S.: Don't forget to rebase. ☺️

lorisleiva · 2026-03-31T14:54:45Z

+    readonly type: TType;
+    readonly value: TValue;


Btw I've decided, I think it's better to keep the design as-is. 🙌

Wrapping a ConstantValueNode inside feels a bit dirty just for the sake of removing a little duplication.

lorisleiva · 2026-03-31T15:01:23Z

+    // so we use u8 to represent the type of each element
+    const type =
+        idl.type === 'bytes'
+            ? numberTypeNode('u8')


Are you sure this is correct? This represents a single u8 number. Should this not just be bytesTypeNode() since the parseConstantValue returns a bytesValueNode('base16', ...) anyway?

I also wonder if we should check the parsed value against the unparsed type to make sure they match together. Wdyt?

lorisleiva · 2026-03-31T15:01:44Z

+    // so we use u8 to represent the type of each element
+    const type =
+        idl.type === 'bytes'
+            ? numberTypeNode('u8')


Same question here.

lorisleiva · 2026-03-31T15:07:50Z

+        value: '[116, 101, 115, 116]', // "test" in bytes
+    });
+
+    expect(node).toEqual(constantNode('seedPrefix', numberTypeNode('u8'), bytesValueNode('base16', '74657374')));


Yeah see this should actually just use the bytesTypeNode() type here to match bytesValueNode(...).

The BytesTypeNode basically just means, it's a type representing raw-bytes without any size constraints. BytesValueNode helps describing the value for these bytes.

lorisleiva · 2026-03-31T15:08:23Z

+    // Type should be parsed, value should be string
+    expect(node.name).toBe('appName');
+    expect(node.value).toEqual(stringValueNode('MyApp'));


What's the type here?

lorisleiva · 2026-03-31T15:12:02Z

+### Numeric Constant
+
+```ts
+const maxSize = constantNode('MAX_SIZE', numberTypeNode('u32'), numberValueNode(100));


Let's stay consistent here with the casing so MAX_SIZE should be maxSize. Let's also use const node = to be consistent with the rest of the documentation.

lorisleiva · 2026-03-31T15:13:47Z

+```ts
+const maxItems = constantNode('MAX_ITEMS', numberTypeNode('u64'), numberValueNode(1000));
+// Note: Add docs via the ConstantNode interface if needed
+```


This example needs to use the 4th parameter to show how to document a constant.

lorisleiva · 2026-03-31T15:14:10Z

+// Note: Add docs via the ConstantNode interface if needed
+```
+
+## Parsing from Anchor IDL


Let's remove that section. We don't do that for any other node. Anchor parsing is bounded in its own package.

lorisleiva · 2026-03-31T15:16:49Z

        outExtension() {
            return { js: `.cjs` };
        },
+        treeshake: false,


Let's remove this change unless this is a blocker for this PR.

lorisleiva · 2026-03-31T15:17:58Z

+test('it preserves docs on constantNode', () => {
+    const node = constantNode('myConstant', numberTypeNode('u32'), numberValueNode(42), ['My docs']);
+    const result = visit(node, identityVisitor());
+    assertIsNode(result, 'constantNode');
+    expect(result.docs).toEqual(['My docs']);
+});


This is a bit too specific of a test for that file. I'd remove it altogether.

feat(nodes): add ConstantNode support with Anchor parsing

caa2700

tanmay4l marked this pull request as ready for review December 12, 2025 13:40

tanmay4l closed this Mar 30, 2026

tanmay4l reopened this Mar 30, 2026

lorisleiva requested changes Mar 31, 2026

View reviewed changes

Conversation

tanmay4l commented Dec 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

changeset-bot bot commented Dec 12, 2025

🦋 Changeset detected

Uh oh!

lorisleiva commented Jan 5, 2026

Uh oh!

tanmay4l commented Jan 5, 2026

Uh oh!

lorisleiva commented Mar 30, 2026

Uh oh!

tanmay4l commented Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lorisleiva left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tanmay4l commented Dec 12, 2025 •

edited

Loading

tanmay4l commented Mar 30, 2026 •

edited

Loading