Update typed FieldMask in sdk-core#101
Merged
Merged
Conversation
|
Please ensure that the NEXT_CHANGELOG.md file is updated with any relevant changes. |
fe1ee1c to
20e3093
Compare
Open
20e3093 to
9662ed8
Compare
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 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.
🥞 Stacked PR
Use this link to review incremental changes.
Summary
Update the
FieldMask<T>in@databricks/sdk-core/wkt.FieldMask.buildvalidates paths against a per-message schema, translates them to wire format, and stores the result privately.toString()serializes to the comma-separated wire string the server expects.Why
Databricks accepts field-mask paths in proto snake_case, but generated TS interfaces use camelCase. The existing
FieldMaskdidn't translate or validate — typos went silently to the server. This is the runtime foundation the generator needs; the follow-up PR emits the per-message schemas and factories that callFieldMask.build.Why validation is at runtime, not compile time
A compile-time
FieldPaths<T>conditional type (computing the string-literal union of valid paths for each generated interface) was explored and rejected. TypeScript's conditional-type evaluation has a hard recursion depth limit — any proto message that references itself or forms a cycle through another message (common in Databricks' schemas:Node { child: Node },A → B → A) triggersTS2589: Type instantiation is excessively deep and possibly infinite. Verified by probing against the real sdk-jstsconfig. Because we can't express a complete set of valid paths at the type level for the schemas we actually ship,FieldMask.buildwalks the schema at runtime and throws on a mismatch instead. The schema itself is a plain runtime object with lazy() => FieldMaskSchemachild references, which keeps recursive and mutually-recursive protos describable as finite graphs.What changed
Interface changes
FieldMask<T = unknown>— private constructor,@internal static build<T>(paths, schema)as the sole entry point,toString(): stringreturning the joined wire paths. PhantomTdistinguishes masks across messages at compile time.FieldMaskSchema,FieldMaskSchemaField— new exports. The shape the generator emits per message;children?: () => FieldMaskSchemakeeps recursive/mutual-recursive protos cycle-safe.FieldPaths<T>,FieldMask.of(...),FieldMask.append(...). See the "Why validation is at runtime" note above for the rationale.Behavioral changes
FieldMask.buildthrows on an unknown path segment or descent past a scalar leaf. Before, any string was accepted.Internal changes
walkFieldMaskPathandnormalizeare module-private inpackages/core/src/wkt/fieldmask.ts.How is this tested?
Table-driven tests (
it.each) inpackages/core/tests/wkt/fieldmask.test.ts: