Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Doc validator #427

Merged
merged 7 commits into from
Jan 31, 2025
Merged

Doc validator #427

merged 7 commits into from
Jan 31, 2025

Conversation

ianmacartney
Copy link
Collaborator

adds doc and typedV validator helpers


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@ianmacartney ianmacartney requested a review from sshader January 30, 2025 22:20
Copy link
Contributor

@sshader sshader left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚢

@@ -106,6 +113,10 @@ export const systemFields = <TableName extends string>(
_creationTime: v.number(),
});

export type SystemFields<TableName extends string> = ReturnType<
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going to claim that we already had this in the convex package, but turns out we don't

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah it makes sense to add to the package imo

expect(userDoc.fields._creationTime).toBeDefined();
const unionDoc = doc(schema, "unionTable");
expect(unionDoc.kind).toBe("union");
if (unionDoc.kind !== "union") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aren't these redundant with the line above?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I had to do this for typescript to stop complaining. Would be nice if there was a type assertion/ refinement as part of vitest. let me know if you know other ways to do this

Expand<V["type"] & ObjectType<Fields>>,
V["isOptional"],
V["fieldPaths"] &
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it possible to get a VObject<Fields> and somehow extract out field paths from there?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe? It's not a property, but fields are, but afaict we don't have a fields -> FieldPaths type utility - it's just inlined in the VObject type def. Maybe I could do ObjectType<Fields> extends ObjectType<any, any, any, infer FieldPaths> ? FieldPaths : never or something - is that better?

@@ -129,6 +140,112 @@ export const withSystemFields = <
} as Expand<T & typeof system>;
};

export type AddFieldsToValidator<
V extends Validator<any, any, any>,
Fields extends PropertyValidators,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens when Fields contains fields that are also in V?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now it's just being used for _id & _creationTime, but it's a good q. I think b/c of & it'll either work if they're the same, or be never?

>,
>(
tableName: TableName,
): AddFieldsToValidator<
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was going to ask if you could use something similar to the Doc type, but nope this is a validator.

I think you could maybe assemble a Validator out of the data model type directly (especially if we're ok losing the structure of the original validator), but seems just as hard as this

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I'd love to have the original validator so you could do omit(v.doc("users").fields, ["password"]) or v.doc("users").fields.address and still have the structure / types work out

@ianmacartney
Copy link
Collaborator Author

ianmacartney commented Jan 31, 2025 via email

@ianmacartney ianmacartney merged commit 1767492 into main Jan 31, 2025
1 check passed
@ianmacartney ianmacartney deleted the doc-validator branch January 31, 2025 01:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants