get-convex / convex-helpers

A collection of useful code to complement the official packages.
Apache License 2.0
101 stars 18 forks source link

Unable to pass customQuery to CRUD #250

Closed panzacoder closed 1 month ago

panzacoder commented 2 months ago

I worked on a project over the summer for a client using Convex, and have just started to pick things back up after a few months of low-to-no activity. After updating deps, I seem to have some odd TS error blocking builds & updates.

All of my CustomQueries are incompatible with the CRUD helper. Am I missing something obvious?

Here's the error:

ts: Argument of type 'CustomBuilder<"query", {}, { user: { _id: Id<"users">; _creationTime: number; phone?: string | undefined; location?: { name?: string | undefined; zipCode?: string | undefined; address?: string | undefined; country: string; state: string; city: string; } | undefined; ... 20 more ...; onboardingStep: number; }; } | { ...' is not assignable to parameter of type 'QueryBuilder<{ agencies: { document: { _id: Id<"agencies">; _creationTime: number; listed?: boolean | undefined; shortName?: string | undefined; logo?: string | undefined; websiteUrl?: string | undefined; ... 5 more ...; name: string; }; fieldPaths: "_id" | ExtractFieldPaths<...>; indexes: { ...; }; searchIndexes: {...'.
  Types of parameters 'func' and 'query' are incompatible.
    Type '{ args?: ArgsValidator | undefined; returns?: ReturnsValidator | undefined; handler: (ctx: GenericQueryCtx<{ agencies: { document: { _id: Id<"agencies">; _creationTime: number; ... 9 more ...; name: string; }; fieldPaths: "_id" | ExtractFieldPaths<...>; indexes: { ...; }; searchIndexes: { ...; }; vectorIndexes: {}; ...' is not assignable to type '{ args?: void | PropertyValidators | undefined; returns?: ReturnsValidator | undefined; handler: (ctx: Overwrite<GenericQueryCtx<{ agencies: { document: { _id: Id<"agencies">; ... 10 more ...; name: string; }; fieldPaths: "_id" | ExtractFieldPaths<...>; indexes: { ...; }; searchIndexes: { ...; }; vectorIndexes: {}; ...'.
      Type '{ args?: ArgsValidator | undefined; returns?: ReturnsValidator | undefined; handler: (ctx: GenericQueryCtx<{ agencies: { document: { _id: Id<"agencies">; _creationTime: number; ... 9 more ...; name: string; }; fieldPaths: "_id" | ExtractFieldPaths<...>; indexes: { ...; }; searchIndexes: { ...; }; vectorIndexes: {}; ...' is not assignable to type '{ args?: void | PropertyValidators | undefined; returns?: ReturnsValidator | undefined; handler: (ctx: Overwrite<GenericQueryCtx<{ agencies: { document: { _id: Id<"agencies">; ... 10 more ...; name: string; }; fieldPaths: "_id" | ExtractFieldPaths<...>; indexes: { ...; }; searchIndexes: { ...; }; vectorIndexes: {}; ...'.
        Type '{ args?: ArgsValidator | undefined; returns?: ReturnsValidator | undefined; handler: (ctx: GenericQueryCtx<{ agencies: { document: { _id: Id<"agencies">; _creationTime: number; ... 9 more ...; name: string; }; fieldPaths: "_id" | ExtractFieldPaths<...>; indexes: { ...; }; searchIndexes: { ...; }; vectorIndexes: {}; ...' is not assignable to type '{ args?: void | PropertyValidators | undefined; returns?: ReturnsValidator | undefined; handler: (ctx: Overwrite<GenericQueryCtx<{ agencies: { document: { _id: Id<"agencies">; ... 10 more ...; name: string; }; fieldPaths: "_id" | ExtractFieldPaths<...>; indexes: { ...; }; searchIndexes: { ...; }; vectorIndexes: {}; ...'.
          Types of property 'args' are incompatible.
            Type 'ArgsValidator | undefined' is not assignable to type 'void | PropertyValidators | undefined'.
              Type 'ArgsValidator' is not assignable to type 'void | PropertyValidators | undefined'.
                Type 'void | PropertyValidators | Validator<any, any, any>' is not assignable to type 'void | PropertyValidators | undefined'.
                  Type 'VId<any, any>' is not assignable to type 'void | PropertyValidators | undefined'.
                    Type 'VId<any, any>' is not assignable to type 'PropertyValidators'.
                      Index signature for type 'string' is missing in type 'VId<any, any>'. [2345]

I made these dep updates with Convex:

Am I missing something obvious?

Edit: It was late, adding some additional color to the problem.

Here is my custom query:

export const authQuery = customQuery(
  query,
  customCtx(async (ctx) => {
    try {
      return { user: await getUserOrThrow(ctx) }
    } catch (err) {
      return { user: null }
    }
  })
)

And here is where the error shows up (on line 3. If I replace authQuery with the convex-generated query func, the error moves to line 4):

1 export const { create, update, destroy } = crud(
2   Agencies,
3   authQuery,
4   authMutation
5 )
ianmacartney commented 2 months ago

Yeah I noticed this too. I suspect when we either add return type validators, or when the validator types changed (1.13) the resulting change made TypeScript see the resulting custom function as not conforming to typeof query.

I have a fix in convex-helpers@0.1.57-alpha.1 Can you verify it works for you @panzacoder?

panzacoder commented 2 months ago

Yes that worked!!! I have been poking at the types for a bit in my node_modules but couldn't find the right adjustment. Thank you @ianmacartney!

ianmacartney commented 1 month ago

Sweet - let me know if there are any issues at runtime - I'll put it out in the next major release.

By the way, I am planning to update the API to just take in the schema and table name:

import schema from "./schema";

crud(schema, "users", authQuery, mutationWithRLS);

So you don't have to use Table to use it.

It's in convex-helpers@0.1.57-alpha.2

This would be a breaking change, so I'm curious whether this would be too disruptive for you / if there's a reason you'd prefer it the old way? I could make a new version and deprecate the old version if it'd be a problem.

panzacoder commented 1 month ago
import schema from "./schema";

crud(schema, "users", authQuery, mutationWithRLS);

Interesting. Seems easier for sure, although I would definitely just wrap it in another function so I don't have to import and pass the schema everywhere (if I'm understanding the change right).

To answer the question directly though, not a big deal to refactor this.

ianmacartney commented 1 month ago

Yeah you could have a helper that you give the schema once and only takes in the table name for other declarations. It also has the second two arguments optional (defaults to internalQuery / internalMutation) - so for making internal crud apis it's only

export const { read, update } = crud(schema, "users");

On Tue, Sep 10, 2024 at 6:27 PM Jake Hebert @.***> wrote:

import schema from "./schema"; crud(schema, "users", authQuery, mutationWithRLS);

Interesting. Seems easier for sure, although I would definitely just wrap it in another function so I don't have to import and pass the schema everywhere (if I'm understanding the change right).

To answer the question directly though, not a big deal to refactor this.

— Reply to this email directly, view it on GitHub https://github.com/get-convex/convex-helpers/issues/250#issuecomment-2342435760, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACZQW3HHJTVAL6FLI5ZPW3ZV6L77AVCNFSM6AAAAABN6H5OX2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNBSGQZTKNZWGA . You are receiving this because you modified the open/close state.Message ID: @.***>

ianmacartney commented 1 month ago

I'm going to go ahead with it at some point, then. Thanks for validating

On Tue, Sep 10, 2024 at 10:58 PM Ian Macartney @.***> wrote:

Yeah you could have a helper that you give the schema once and only takes in the table name for other declarations. It also has the second two arguments optional (defaults to internalQuery / internalMutation) - so for making internal crud apis it's only

export const { read, update } = crud(schema, "users");

On Tue, Sep 10, 2024 at 6:27 PM Jake Hebert @.***> wrote:

import schema from "./schema"; crud(schema, "users", authQuery, mutationWithRLS);

Interesting. Seems easier for sure, although I would definitely just wrap it in another function so I don't have to import and pass the schema everywhere (if I'm understanding the change right).

To answer the question directly though, not a big deal to refactor this.

— Reply to this email directly, view it on GitHub https://github.com/get-convex/convex-helpers/issues/250#issuecomment-2342435760, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACZQW3HHJTVAL6FLI5ZPW3ZV6L77AVCNFSM6AAAAABN6H5OX2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNBSGQZTKNZWGA . You are receiving this because you modified the open/close state.Message ID: @.***>

ianmacartney commented 1 month ago

I published the version with schema in 0.1.58 at convex-helpers/server/crud and marked the old one as deprecated. I decided it was a good time to move it to its own entrypoint anyways, and not break old clients when I can avoid it

ianmacartney commented 1 month ago

also there's a Stack post now: https://stack.convex.dev/crud-and-rest that lightly touches on it