-
Notifications
You must be signed in to change notification settings - Fork 325
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
Automatic TypeScript definition generation from JSG RTTI #110
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mrbbot
requested review from
kentonv,
jasnell,
harrishancock,
mikea,
jclee,
a-robinson,
bretthoerner and
byule
as code owners
October 15, 2022 18:57
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
I have read the CLA Document and I hereby sign the CLA |
mrbbot
force-pushed
the
bcoll/types
branch
2 times, most recently
from
October 16, 2022 10:02
c585165
to
38d6bde
Compare
jasnell
approved these changes
Oct 16, 2022
penalosa
approved these changes
Oct 17, 2022
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.
This is very very cool!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hey! 👋 This PR adds support for generating TypeScript types from JSG RTTI, replacing the internal
autodecl
script. This forms the basis for the nextworkers-types
version. These scripts are located in this repository rather thanworkers-types
as they depend on Bazel outputs, and we'd like to be able to share the Bazel cache in CI. Going forward, DevProd should probably be theCODEOWNER
for everything in thetypes
directory.To generate types, run:
A gist containing the generated types can be found here. This also includes a copy of the types before TypeScript transformers are applied.
(fyi, about half of the additions are from
pnpm-lock.yaml
)Implementation Notes and Questions
jsg::fullyQualifiedTypeName()
method. This behaves likejsg::typeName()
, but includes namespaces and template arguments in the returned name. Namespaces are required to differentiate nested types with the same name (e.g.DurableObjectStorageOperations::GetOptions
,KvNamespace::GetOptions
andR2Bucket::GetOptions
). Template arguments are required to differentiatejsg::IteratorBase...
types generated byJSG_ITERATOR
.workerd::jsg::rtti::Builder
, replaced the symbol key fromjsg::typeName()
tojsg::fullyQualifiedTypeName()
for the reasons above. This also affects which values should be passed as parameters toworkerd::jsg::rtti::Builder::structure()
.rtti.capnp
schema. Generally, I tried to evolve the schema in a backwards-compatible way, but it would be cleaner if we're still able to make breaking changes. /cc @mikeafullyQualifiedName
fields toStructure
andStructureType
schemas for the reasons above. I was hesitant to makename
fully-qualified since I wasn't sure which other code depended on RTTI.jsg::LenientOptional
to RTTI. When generating TypeScript definitions, we also need to be able to distinguish between optionals expectingnull
(kj::Maybe
) and others expectingundefined
. A newname
field has been added to theMaybeType
schema, similar toNumberType
andStringType
.kj::Array
,kj::ArrayPtr
andjsg::Sequence
, so a newname
field has been added to theArrayType
schema. It may be better to make this and the previous maybe field enums instead?name
field in a group withnested
members. This ensures members coming fromJSG_NESTED_TYPE_NAMED
macros have the correct names.iterator
andasyncIterator
fields to theStructure
schema. Whilst there are alreadyiterable
andasyncIterable
boolean fields, we need to know the full method types (especially the returned(Async)Iterator
type) for[Symbol.iterator]
/[Symbol.asyncIterator]
.api-encoder.c++
entrypoint that spits out RTTI to a file. All TypeScript generation scripts are written in TypeScript, so we can use the official TypeScript compiler API for creating/processing/printing AST nodes.aspect-build/rules_js
requires us to use pnpm. We should be able to use this setup for packaging/publishingworkerd
npm
packages too. /cc @penalosaTODOs
(to follow in later PRs)
autodecl
😅), they're not as accurate as they could be.param0
,param1
, ... . Ideally, we'd use the actual C++ parameter names here, so we need some way of getting these into the RTTI.