Repository navigation
Add a TypeScript binding emitter - #9
Conversation
A native emitter, written in TypeScript, turns a normalized binding model into a typed npm package: src/bindings.ts and src/index.ts, package.json, tsconfig.json, and the structural binding manifest. It follows the Python emitter's command contract: emit, verify, and api. - Declarations are functions taking a per-kind marker interface. Object shapes are classes that declare the marker method, so a field belonging to another kind, or to another package's kind, does not type-check. String domains are enums, arrays are ReadonlyArray, and maps are index-signature types, which stay valid for recursive schemas. - Names derive from exact source names with Pascal and camel case. A reserved word gains a trailing underscore; a TypeScript global, a fixed symbol, or an unresolvable collision stops emission without renaming. - The model is read with the Python emitter's restrictions and validated against the shared binding-model schema. - verify parses the generated source with the TypeScript compiler API and compares its public API and the manifest with a fresh rendering. - The shared binding-manifest schema accepts the typescript language, with minimumNodeVersion, bindings.ts, and the boolean, number, and null builtins. Go and Python manifests validate unchanged. - toolchain.lock.yaml records the TypeScript, Prettier, and Node pins under languages.typescript, and a test keeps the emitter's pins equal to it. - The extension binding tooling workflow gains a TypeScript emitter job. Tests cover the shared conformance models and the committed first-party models: three byte-identical emissions, Prettier stability, strict tsc compilation with provider stubs, a generated exercise of every type, and negative diagnostics, loader, target, naming, and CLI cases. Signed-off-by: itvi-1234 <rjsumit71@gmail.com>
0c29123 to
cca11d8
Compare
colinjlacy
left a comment
colinjlacy
left a comment
There was a problem hiding this comment.
In general, looks good. Big picture, the thing I'd like to see changed is to remove hard-coded versions and see if we can inject those dynamically. The idea of tracking versions in code means that we'd have to update the code to the right version before we can actually tag the version. And that becomes a bit of a mess.
| export const EMITTER_VERSION = "0.1.0"; | ||
| export const TYPESCRIPT_VERSION = "5.9.3"; | ||
| export const PRETTIER_VERSION = "3.9.10"; |
There was a problem hiding this comment.
Is there a reason we'd want these hard-coded here? Seems like we'd have to maintain this every time we make a change and push a new version, use a new version of TS, etc. I wonder if there's a way to pull this in dynamically instead.
| const [major, minor] = value.split("-", 1)[0]!.split(".").map(Number) as [number, number]; | ||
| const next = major === 0 ? `0.${minor + 1}.0` : `${major + 1}.0.0`; |
There was a problem hiding this comment.
Oof, that is a really big assumption. It would be good to be able to set this dynamically.
| { | ||
| "compilerOptions": { | ||
| "target": "ES2022", | ||
| "module": "NodeNext", | ||
| "moduleResolution": "NodeNext", | ||
| "rootDir": "src", | ||
| "outDir": "dist", | ||
| "declaration": true, | ||
| "strict": true, | ||
| "noUncheckedIndexedAccess": true, | ||
| "isolatedModules": true, | ||
| "verbatimModuleSyntax": true, | ||
| "skipLibCheck": false, | ||
| "types": [] | ||
| }, | ||
| "include": ["src"] | ||
| } |
There was a problem hiding this comment.
Instead of hard-coding this here, I wonder if there's a way to configure this externally with a template file.
| export const KEY_PATTERN = /^[a-z0-9]+(?:-[a-z0-9]+)*$/; | ||
| export const PACKAGE_NAME_PATTERN = /^(?:@[a-z0-9][a-z0-9-]*\/)?[a-z0-9][a-z0-9-]*$/; | ||
| export const SHA_PATTERN = /^[0-9a-f]{64}$/; | ||
| const MINIMUM_NODE = [20, 0] as const; |
There was a problem hiding this comment.
This is another one I wouldn't want to hard-code, if we can avoid it
| properties: | ||
| type: {$ref: '#/$defs/nativeIdentifier'} | ||
| builtin: {enum: [string, bool, int64, float64, 'struct{}', any, str, int, float, 'None', JSONValue]} | ||
| builtin: {enum: [string, bool, int64, float64, 'struct{}', any, str, int, float, 'None', JSONValue, boolean, number, 'null']} |
There was a problem hiding this comment.
This is fine for now, so don't worry about changing this yet. I'm worried about this growing into every language-specific type. I wonder if we need to start thinking about some sort of Karpenter style YAML join templating to dynamically compile a schema for binding generation. And if we do that, are we really working with a schema? 🤔
There was a problem hiding this comment.
Do we need this file? Looks more like a README. If that's what it is, let's change the name. I'd imagine your agent saw https://github.com/runtimeconditions/extensions/blob/main/tooling/extension-bindings/emitters/python/CONTRACT.md and thought that this is a paradigm it needed to follow. I need to rename and update that file to be more of a README than what it currently is, which is an LLM artifact from the process of setting up the origin binding generation workflows.
Review feedback asked for the emitter to stop tracking versions and settings in code. - The emitter name and version come from its package.json, and the TypeScript and Prettier versions come from the installed packages. The Prettier version check against a constant is gone because it compared the tool with itself. - A dependency's npm range defaults to ^<version>, so the next-breaking rule is npm's own, and a dependency can set versionRange to override it. - The generated package.json and tsconfig.json are filled from templates in templates/, and the Prettier options are read from templates/. - The package target is validated against package-target.schema.yaml instead of patterns repeated in code. Only the checks that need two fields stay in code. - CONTRACT.md is removed. Signed-off-by: itvi-1234 <rjsumit71@gmail.com>
Keep the resolved type of scalar, any, and chained definitions instead of discarding it, so a property that references one no longer gets an undefined type. Compare package.json and tsconfig.json during verification, read inputs through a bounded reader so an oversized file is rejected before it is held in memory, and compile root, middle, and leaf packages together. Make the npm publication workflow state that it is a placeholder, reject dry_run false, and record in its summary that nothing was verified or published. Signed-off-by: itvi-1234 <rjsumit71@gmail.com>
| } | ||
| } | ||
|
|
||
| for (;;) { |
There was a problem hiding this comment.
does this ever exit? I'm not seeing it...
e6cb4a1 to
4cbe392
Compare
Replace for (;;) and its inner break with a while loop whose condition names what is left to plan, and explain why it ends: each pass removes the definitions it plans from pendingDefinitions, so the set only shrinks. Signed-off-by: itvi-1234 <rjsumit71@gmail.com>
4cbe392 to
d943981
Compare
No description provided.