Add v.object() unknownKeys: "strip" mode#348
Add v.object() unknownKeys: "strip" mode#348robelest wants to merge 5 commits intoget-convex:mainfrom
Conversation
robelest
force-pushed
the
feat/v.object-optional-validators
branch
from
b07bf3e to
ad062c7
Compare
ianmacartney
left a comment
ianmacartney
left a comment
There was a problem hiding this comment.
didn't go through all of it, but I think dropping the new type parameter will simplify a lot
| Expand<Omit<Type, K>>, | ||
| Expand<Omit<Fields, K>>, | ||
| IsOptional, | ||
| string, |
There was a problem hiding this comment.
I think this loses the extracted field paths - maybe we can use the ObjectFieldPaths helper?
| Expand<Pick<Fields, K>>, | ||
| IsOptional | ||
| IsOptional, | ||
| string, |
| { [K in keyof Fields]: VOptional<Fields[K]> }, | ||
| IsOptional | ||
| IsOptional, | ||
| string, |
| Expand<Fields & NewFields>, | ||
| IsOptional | ||
| IsOptional, | ||
| string, |
| }[keyof Fields] & | ||
| string, | ||
| FieldPaths extends string = ObjectFieldPaths<Fields>, | ||
| UnknownKeys extends "strict" | "strip" = "strict", |
There was a problem hiding this comment.
until we have "passthrough" - I don't think we need this to be reflected in types, which would help with the auto-assigning of FieldPaths (not having to duplicate it or pass "string"). we can cross the "passthrough" bridge later (if ever)
| const validatedArgs = jsonToConvex(validateArgsResult.args[0]) as any; | ||
| const functionResult = await functionDefinition.handler(ctx, validatedArgs); |
There was a problem hiding this comment.
was this a bug before? I don't think this is doing what you want- it'll turn a bigint into { $bigint: "AAAA..." }
| return new VObject<ObjectType<T>, T, "required", any, "strict" | "strip">({ | ||
| isOptional: "required", | ||
| fields, | ||
| unknownKeys: options?.unknownKeys ?? "strict", | ||
| }); | ||
| }) as { | ||
| <T extends PropertyValidators>( | ||
| fields: T, | ||
| ): VObject<ObjectType<T>, T>; | ||
| <T extends PropertyValidators, UK extends "strict" | "strip">( | ||
| fields: T, | ||
| options: { unknownKeys: UK }, | ||
| ): VObject<ObjectType<T>, T, "required", ObjectFieldPaths<T>, UK>; |
There was a problem hiding this comment.
hopefully dropping the UnknownKeys type will simplify this
Rust: Convert ObjectValidator tuple to named struct with fields + unknown_keys.
Add UnknownKeysMode enum (Strict, Strip). Strict preserves existing behavior
(reject unknown fields); Strip silently removes them during validation.
- Add check_fields(), check_value(), strip_unknown_fields() methods
- Update is_subset() to account for mode compatibility
- Add strip_value() on DocumentSchema, strip_new_document() on DatabaseSchema
- Wire stripping into transaction.rs (insert/patch/replace), import_facing.rs,
and UDF arg validation (validation.rs)
- JSON serialization: unknownKeys field omitted for Strict (backward compatible)
TypeScript: Add unknownKeys property to VObject as a concrete runtime type
(not a type parameter). v.object(fields, { unknownKeys: "strip" }) API.
Propagate mode through asOptional, omit, pick, extend, partial.
FieldPaths auto-compute from Fields default (no regression on combinators).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
robelest
force-pushed
the
feat/v.object-optional-validators
branch
from
ad062c7 to
b145232
Compare
ianmacartney
left a comment
ianmacartney
left a comment
There was a problem hiding this comment.
The API looks good.
I want to see more usage of this in JS however - e.g. having some JS that defines these looser object validators as args / returns / schema and runs them "for real" - from Rust-based integration test runner or otherwise.
I'm going to add @Nicolapps to review the Rust side and provide guidance on testing approaches here.
It also makes me realize we should update convex-test to support this around the same time, so folks writing tests see the same behavior. Can you install a local build of convex in convex-test (you can use npm pack to make it) and put up a tentative PR there?
Also convex-helpers has validate and parse functions that should take this into account.
ianmacartney
dismissed
their stale review
LGTM, deferring to Nicolas from here
Add end-to-end coverage for unknownKeys: "strip" across isolate args validation, application returns validation, and database schema enforcement. These tests verify that extra object fields are stripped where expected while type validation errors are still raised, and that schema export includes unknownKeys in serialized table definitions.
robelest
force-pushed
the
feat/v.object-optional-validators
branch
2 times, most recently
from
4665440 to
5d940b7
Compare
|
Resolved merge conflicts with Preserved the unknownKeys strip behavior and strict-first union matching. Ran targeted isolate/application/database strip-mode tests locally; all passed. |
Reformat validator types and tests touched by strip-mode work so formatting checks on PR get-convex#348 pass cleanly.
2 participants
Summary
unknownKeysoption tov.object()validators, controlling how extra fields are handled"strict"(default): rejects documents with unknown fields (existing behavior)"strip": silently removes unknown fields during validation, before persistingDesign for future Passthrough
The
UnknownKeysModeenum is designed so adding aPassthroughvariant is straightforward:strips_unknown_fields()would returnfalsefor Passthrough (like Strict — don't modify the document)is_subsetalready handles mode compatibility via the boolean checkcheck_fieldsskips the extra-field rejection whenstrips_unknown_fields()is true — Passthrough would need a similar gate onallows_extra_fields()(re-added at that point)FromStr| "passthrough"addedBackward compatibility
Stricteverywhere — no behavior change for existing schemasunknownKeysfield for Strict — old schemas parse correctlyTest plan
cargo buildpassestest_scheduled_jobs_garbage_collectionis test unrelated to this PR (GC timing issue, zero diff from main)By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.