[pull] master from clockworklabs:master#548
Merged
Merged
Conversation
# Description of Changes
Updates the TypeScript code generator to emit correct `.name()` values
on table fields, and updates the three affected Case Conversion Test
snapshots.
**Problem:** For source identifiers with mixed/PascalCase casing (e.g.,
`Player1Id`), the in-process codegen path used by `cargo test` and
`check-diff.sh` emitted `.name()` calls using the pre-conversion
identifier (`name('Player1Id')`) instead of the canonical database
column name (`name('player_1_id')`). The CLI `spacetime generate` path
was already correct because its `extract-schema` round-trip
canonicalizes names before codegen. The two paths disagreed, causing
`check-diff` CI failures.
**Root cause / Fix:** `write_object_type_builder_fields` in
`crates/codegen/src/typescript.rs` was using the `TypespaceForGenerate`
pre-conversion identifier for `.name()`. It now accepts an optional
`&[ColumnDef]` and emits `column.name` for table fields. The camelCase
accessor key (e.g., `player1Id`) is unchanged; only the underlying
wire/database column name is corrected to snake_case.
The three snapshot files in
`crates/bindings-typescript/case-conversion-test-client/src/module_bindings/`
are updated to match the corrected output.
# API and ABI breaking changes
No API or ABI changes at the Rust crate or C ABI level.
For TypeScript users, this is a bug fix for the in-process generation
path; the CLI path was already correct.
# Expected complexity level and risk
1 - Trivial
# Testing
- [X] Local testing
- Verified the in-process `cargo test -p spacetimedb-sdk --
case_conversion` path on `master` generated incorrect PascalCase
`.name()` values (`name("Player1Id")`, missing `.name()` for
`currentLevel2`/`status3Field`.
- Verified the same in-process path on the PR branch generates correct
snake_case `.name()` values (`name("player_1_id")`,
`name("current_level_2")`, `name("status_3_field")`), matching the
updated snapshots.
- Verified the CLI `spacetime generate --lang typescript` path produces
identical snake_case output on both `master` and the PR branch.
- Created a standalone TypeScript test that confirms the generated
bindings produce SQL with canonical snake_case column names
(`"current_level_2"`, `"player_1_id"`, `"player_ref"`) via the SDK
`toSql()` function.
- [X] CI passing
---------
Signed-off-by: Ryan <r.ekhoff@clockworklabs.io>
Co-authored-by: clockwork-labs-bot <clockwork-labs-bot@users.noreply.github.com>
Co-authored-by: Zeke Foppa <196249+bfops@users.noreply.github.com>
# Description of Changes Instead of baking it into the query plan. The query plan should parameterize anonymous views just like sender-scoped views. We need this for the next step which is plan sharing for subscriptions. # API and ABI breaking changes None # Expected complexity level and risk 1 # Testing Existing coverage
…lumnBuilder (#4389) ## Summary - Codegen unconditionally emits `.primaryKey()` on all primary key columns, including enum types - `SumBuilderImpl` (used for **all** object-form enums via `__t.enum("Name", { ... })`) only implemented `Defaultable` and `Nameable` — not `PrimaryKeyable` or `Indexable` - This causes a runtime `TypeError: SC.primaryKey is not a function` whenever an enum type is used as a primary key column ## Fix Add `Indexable` and `PrimaryKeyable` interfaces and their methods to: - `SumBuilderImpl` (the base class for all enum type builders) - `SumColumnBuilder` (the column builder returned by `SumBuilderImpl` methods) This matches the existing pattern already present in `SimpleSumBuilderImpl` and `SimpleSumColumnBuilder`. ## Why SumBuilderImpl and not just SimpleSumBuilderImpl? Codegen always generates object-form enums: ```typescript export const PlatformModuleType = __t.enum("PlatformModuleType", { Standard: __t.unit, Control: __t.unit, }); ``` The runtime dispatch in `type_builders.ts` (line ~3656) routes object-form enums to `SumBuilder` (backed by `SumBuilderImpl`), never to `SimpleSumBuilder`. So `SimpleSumBuilderImpl`'s existing `primaryKey()`/`index()` methods are effectively dead code for codegen output. ## Test plan - [x] Verify enum types used as primary keys no longer throw `TypeError: SC.primaryKey is not a function` - [ ] Verify existing `SimpleSumBuilderImpl`/`SimpleSumColumnBuilder` behavior is unchanged (subclass overrides still work) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Zeke Foppa <196249+bfops@users.noreply.github.com>
# Description of Changes An evolution of #4707 where i explored adding a `db_read_only` method to `DbContext`. (hence i would appreciate your thoughts @gefjon ) This was the wrong appraoch in retrospect because there are still some annoyances with this. Furthermore there is really no benefit to adding the associated type `DbView` and is just making the ergonomics worse. So here is the new approach which solved all my problems: Making a trait for every capability which the various contexts are providing because the Databse is only one of them. These capabilities are (and pretty much the whole relevant div for this pr because the impl blocks are trivial) and should be self explanatory: ```rust pub trait CtxDbRead { fn db_read_only(&self) -> &LocalReadOnly; } pub trait CtxDbWrite: CtxDbRead { fn db(&self) -> &Local; } pub trait CtxWithSender { fn sender(&self) -> Identity; } pub trait CtxWithTimestamp { fn timestamp(&self) -> Timestamp; } pub trait CtxWithHttp { fn http(&self) -> &HttpClient; } ``` ## Why is this relevant? Lets look at an example building on the previous pr: You have abstracted your code in a trait which you can call for every context e.g. authorization. Now this does not work because the sender method is not available on `DbContext`. ```rust impl<Db: CtxDbRead> Authorization for Db { fn test(&self,args:Args) { self.db_read_only().do_i_have_perms().find(self.sender()); //ERROR: no sender method } ``` Now this is really annoying since you now have to pass additional parameters to the method. Instead we can now specific these capabilities in the type system: ```rust impl<Db: CtxDbRead+CtxWithSender> Authorization for Db { fn test(&self,args:Args) { self.db_read_only().do_i_have_perms().find(self.sender()); //WORKS NOW YAY } ``` Additonally there could be also a `+ CtxWithTimestamp` if you wanted to for example store a last logged in date or smth (you get the idea) Now this is far better because `.sender` is available for `ViewContext` for example so you can authorize with the same method. ## Alternatives/Bikeshedding I chose the names CtxWith because really the `Contexts` are the common denominator and not the `Database`. Thats also the reason why its `CtxDbRead` because you are expressing: "All context where i get read access to the databse (e.g. everything). Other names have felt worse. Also the deprecation can be removed but i think this approach is strictly superior and i dont think there are currently many people relying on it. # API and ABI breaking changes None. one deprecation for the old `DbContext` but this can also be removed if desired. # Expected complexity level and risk 1. Additive change with extremly minimal surface # Testing - [x] Works for my project --------- Signed-off-by: Kilian Strunz <93079615+kistz@users.noreply.github.com> Co-authored-by: Phoebe Goldman <phoebe@goldman-tribe.org>
## Summary
`XForwardedFor::decode` requires a comma and fails to decode
`X-Forwarded-For: 1.2.3.4`.
Under axum 0.8 this now surfaces as HTTP 400 on every single-hop-proxy
request.
## Fix
Accept comma-separated or single-IP values — take the first entry either
way.
## Reproducer
Any browser going through a proxy that does `.insert("x-forwarded-for",
client_ip)`
(standard single-hop pattern) sees 400 Bad Request on WebSocket
subscribe.
## Version regression
Bug latent since the axum migration in 2023-06 (commit b4dae74). axum
0.7
silently dropped the decoder error via `Option<TypedHeader<T>>`; axum
0.8
(#2713) promotes it to a 400 rejection.
Co-authored-by: Zeke Foppa <196249+bfops@users.noreply.github.com>
This file contains hidden or 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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )