Skip to content
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

Update bindings for variants and output documentation for resources #5

Merged

Conversation

mjoerussell
Copy link

This PR changes some of the Typescript bindings for variant and resource types.

For resources, this outputs the type-level documentation that exists in the .wit files into the bindings. The original implementation didn't really track these (they were parsed, though), so a bit more modification went into these than some of the other documentation changes.

For variants, this changes the naming convention for their Typescript enum outputs. Specifically, it:

  • Renames {variant_name}Variant to {variant_name}Kind
  • Renames the variant class instance field from {variant_name}Kind to type.

This PR also adds some documentation for the common functions added to variant classes.

This PR will be used to solve NomicFoundation/slang#1166 and NomicFoundation/slang#1171.

@mjoerussell mjoerussell self-assigned this Dec 2, 2024
@mjoerussell mjoerussell requested review from a team as code owners December 2, 2024 23:12
Copy link

@OmarTawfik OmarTawfik left a comment

Choose a reason for hiding this comment

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

Left a few suggestions. Thanks for looking into this!

Copy link

@OmarTawfik OmarTawfik left a comment

Choose a reason for hiding this comment

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

One suggestion remaining about a lingering change.
Otherwise, LGTM. Thanks!

@mjoerussell mjoerussell force-pushed the feature/variant-resource-bindings branch from c054ff6 to bb78e92 Compare December 5, 2024 14:39
mjoerussell and others added 4 commits December 5, 2024 08:43
* Rename the generated enum type from "{variant_name}Variant" to "{variant_name}Kind"
* Rename the generated readonly field on variant classes from "{variant_name}Kind" to "type"
* Add documentation to the generated functions on variant classes
Co-authored-by: Omar Tawfik <15987992+OmarTawfik@users.noreply.github.com>
* Update documentation to include backticks around keywords/types
* Update documentation to use the word "type" instead of "kind"
@mjoerussell mjoerussell force-pushed the feature/variant-resource-bindings branch from bb78e92 to 5b19b7b Compare December 5, 2024 14:43
@mjoerussell mjoerussell added this pull request to the merge queue Dec 5, 2024
Merged via the queue into nomic-enhancements-release with commit f3b5e14 Dec 5, 2024
@OmarTawfik OmarTawfik deleted the feature/variant-resource-bindings branch December 5, 2024 17:38
github-merge-queue bot pushed a commit to NomicFoundation/slang that referenced this pull request Dec 12, 2024
Updates our JCO submodule to include the changes in
NomicFoundation/jco#5. Includes one other change to fully integrate
these changes, which is to change `NodeVariant` to `NodeType` in
`codegen/../cst/index.mts` so that the expected imports align with the
new generated exports.

* [x] Depends on #1169
* [x] Depends on NomicFoundation/jco#5

Closes #1166 
Closes #1171
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