feat: eql-types — TypeScript bindings (stacked on #236)#268
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
coderdan
force-pushed
the
dan/eql-types-ts-bindings
branch
7 times, most recently
from
29418a0 to
16ce202
Compare
coderdan
force-pushed
the
dan/eql-types-ts-bindings
branch
from
16ce202 to
73acb8e
Compare
coderdan
force-pushed
the
dan/eql-types-ts-bindings
branch
from
73acb8e to
0250c37
Compare
Stacks on the Rust-only eql-types crate. Every v3 domain type, the term newtypes, and Identifier gain a TS derive with bindings to crates/eql-types/bindings/v3/ (28 files, checked in). Term newtypes export as named TS aliases (export type Hmac256 = string) that every domain binding imports. mise types:generate clean-regenerates the bindings; types:check regenerates and fails on any diff or untracked output, wired into the rust-crates CI job as a freshness gate.
coderdan
force-pushed
the
dan/eql-types-ts-bindings
branch
from
0250c37 to
db988b3
Compare
freshtonic
left a comment
freshtonic
left a comment
There was a problem hiding this comment.
Approving — conditional on the three minor fixes flagged inline below. None are blockers: the substance of the codegen layer is correct and coverage is complete (30 structs → 30 #[ts(export)] attributes → 30 checked-in bindings under bindings/v3/), the generated TS is accurate, and the regenerate-and-swap script + CI freshness gate are sound.
Three minor items, posted inline:
serde-json-implts-rs feature appears unused (Cargo.toml).- Crate-naming suggestion:
eql-types→eql-types-postgres(Cargo.toml). - FYI on the
cargo test→bindings/write side effect (README.md).
One extra nit not tied to a line: the PR description says "28 files" but there are 30 .ts bindings under bindings/v3/ — worth correcting so reviewers' file counts line up.
| @@ -2,10 +2,11 @@ | |||
| name = "eql-types" | |||
There was a problem hiding this comment.
Naming suggestion: consider eql-types-postgres. The domain types here are prefixed with PostgreSQL type names (int2 / int4 / int8 / timestamptz / text), so the crate is implicitly Postgres-flavoured. Renaming it now leaves room for future database-vendor-native EQL type crates (e.g. eql-types-<vendor>) without a disruptive rename later, while there are still few external consumers.
This is fine as a separate follow-up — it needn't block or fold into this bindings PR. A rename ripples beyond these changes: the crate is path-referenced by eql-scalars' dev-dependencies, and the test:crates / types:generate / types:check mise tasks all hardcode -p eql-types. Keeping it as its own small PR keeps that churn out of this focused codegen change.
There was a problem hiding this comment.
Deserves a separate discussion. IMHO this crate defines the consumer shapes which should be consistent. If it turns out that XYZ DB needs a different shape I'd prefer to create an internal adapter crate and avoid changes to upstream consumers (stack, proxy etc).
…te in-place regen - Drop the `serde-json-impl` ts-rs feature: no v3 type carries a serde_json::Value/Map/Number field, so it only pulled serde_json into ts-rs's build graph for nothing. Bindings regenerate byte-identically without it (verified via `mise run types:check`). - README: note that a plain `cargo test` / `mise run test:crates` regenerates `bindings/` in place as a side effect — only `types:generate` isolates the write via the temp-dir swap.
feat: eql-types — JSON Schemas (stacked on #268)
…te in-place regen - Drop the `serde-json-impl` ts-rs feature: no v3 type carries a serde_json::Value/Map/Number field, so it only pulled serde_json into ts-rs's build graph for nothing. Bindings regenerate byte-identically without it (verified via `mise run types:check`). - README: note that a plain `cargo test` / `mise run test:crates` regenerates `bindings/` in place as a side effect — only `types:generate` isolates the write via the temp-dir swap.
feat: eql-types — TypeScript bindings (stacked on #236)
feat: eql-types — JSON Schemas (stacked on #268)
…te in-place regen - Drop the `serde-json-impl` ts-rs feature: no v3 type carries a serde_json::Value/Map/Number field, so it only pulled serde_json into ts-rs's build graph for nothing. Bindings regenerate byte-identically without it (verified via `mise run types:check`). - README: note that a plain `cargo test` / `mise run test:crates` regenerates `bindings/` in place as a side effect — only `types:generate` isolates the write via the temp-dir swap.
2 participants
Stacked on #236 (now merged). Adds the ts-rs layer to the eql-types crate:
TSderive +#[ts(export, export_to = "v3/")]on every v3 domain type, the term newtypes, andIdentifiercrates/eql-types/bindings/v3/*.ts(30 files). Term newtypes export as named TS aliases (export type Hmac256 = string) imported by every domain bindingmise run types:generate(clean-regenerate) andmise run types:check(regenerate + fail on diff/untracked), with a freshness-gate step in therust-cratesCI jobJSON Schemas follow in a further stacked PR.
Closes #287.