-
-
Notifications
You must be signed in to change notification settings - Fork 0
Add support for naming tuple members #33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Co-authored-by: streamich <[email protected]>
…ality working Co-authored-by: streamich <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sure to pass linter and formatter when done.
expect(schema).toStrictEqual({ | ||
kind: 'tup', | ||
types: [ | ||
{ kind: 'num' }, | ||
{ kind: 'str' } | ||
] | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Named fields should wrap the type node in a field node:
expect(schema).toStrictEqual({ | |
kind: 'tup', | |
types: [ | |
{ kind: 'num' }, | |
{ kind: 'str' } | |
] | |
}); | |
expect(schema).toStrictEqual({ | |
kind: 'tup', | |
types: [ | |
{ kind: 'field', key: 'x', value: { kind: 'num' } }, | |
{ kind: 'str' } | |
] | |
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the schema generation to wrap named fields in field nodes with the expected structure. The schema now correctly shows { kind: 'field', key: 'x', value: { kind: 'num' } }
for named tuple members. (957ed5c)
}); | ||
}); | ||
|
||
test('tuple toString shows field names when present', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the toString test as requested. (957ed5c)
Co-authored-by: streamich <[email protected]>
All linting and formatting checks now pass. Used yarn format:fix to auto-fix formatting issues and verified with yarn lint and yarn format. (957ed5c) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Resolve conflicts, push latest.
- Make sure tests, linter, and formatter pass.
Resolved conflicts and merged latest from master. All requirements are now met: ✅ Conflicts resolved: Successfully merged with master, updated to use Changes committed and pushed in 8d7dfc1. |
This PR implements support for naming tuple members using the same
t.prop()
syntax used for object properties, as requested in the issue.New Features
Named tuple members
You can now create tuples with named fields:
Mixed named and unnamed members
Tuples can mix named and unnamed members:
Full functionality support
Named tuples work with all existing functionality:
[1, 2]
"x": num
)Implementation Details
The implementation reuses the existing
ObjectFieldType<K, V>
class to maintain consistency with object field naming. TheTupleType
class was updated to accept mixedType | ObjectFieldType<any, any>
elements, with properinstanceof
checks throughout all codegen paths.Backward Compatibility
All existing tuple functionality remains unchanged:
Testing
Fixes #3.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.