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

Export additional utility types from generated query builder code #1052

Open
tomnz opened this issue Jul 6, 2024 · 5 comments
Open

Export additional utility types from generated query builder code #1052

tomnz opened this issue Jul 6, 2024 · 5 comments

Comments

@tomnz
Copy link

tomnz commented Jul 6, 2024

I'm attempting to write generic functions that accept query builder expressions and wrap them with additional functionality before calling them. (In our particular case, the intent is to combine with Effect to perform logging/tracing/etc and turn the expression call into an "effect-ful" call, but I could imagine loads of other use cases).

However, I am occasionally having difficulty crafting type signatures that satisfy TS. For example, this works:

import { type $expr_Insert } from "@internal/edgedb/edgeql/insert";
import {
  type ObjectType,
} from "@internal/edgedb/edgeql/typesystem";

export const tryInsert = async <T extends ObjectType>(
  statement: $expr_Insert<T>,
): Promise<string> => {
  try {
    const result = (await statement.run(client)) as unknown as { id: string };
    return result.id;
  } catch (error) {
    console.error(error);
    throw error;
  }
};

For whatever reason, statement.run() has an any return type, in spite of being an $expr_Insert - so the clunky cast is required, but works fine. Also, this is obviously a fairly contrived example, but picture additional things happening in the wrapper.

However, the following seems to be impossible:

import { type $expr_Insert } from "@internal/edgedb/edgeql/insert";
import {
  type $expr_WithParams,
  // INVALID: type is NOT exported
  type paramsToParamArgs,
  // INVALID: type is NOT exported
  type ParamsRecord,
} from "@internal/edgedb/edgeql/params";
import {
  type ObjectType,
} from "@internal/edgedb/edgeql/typesystem";

export const tryInsertWithParams = async <
  T extends ObjectType,
  // INVALID: ParamsRecord is not available
  P extends ParamsRecord,
>(
  statement: $expr_WithParams<P, $expr_Insert<T>>,
  // INVALID: paramsToParamArgs is not available
  args: paramsToParamArgs<P>,
): Promise<string> => {
  try {
    const result = (await statement.run(client, args)) as unknown as {
      id: string;
    };
    return result.id;
  } catch (error) {
    console.error(error);
    throw error;
  }
};

The two utility types paramsToParamArgs and ParamsRecord are not exported from the generated code. Therefore, it does not seem possible to correctly type generic parameters to wrap a parameterized expression execution.

I tried copy/pasting the definitions across alongside the above code, but it complains about args being a different type than expected:

Argument of type 'paramsToParamArgs<Params>' is not assignable to parameter of type 'paramsToParamArgs<Params>'.
  Type 'paramsToParamArgs<Params>' is not assignable to type '{ [key in keyof Params as Params[key] extends ParamType ? key : never]: Params[key] extends ParamType ? Readonly<BaseTypeToTsType<Params[key], true>> : never; }'.ts(2345)

(First type is the copied type, second is the type in the generated code).

Open to other ideas about how to achieve this, but it seems the easiest solution would be to export additional types from the generated code.

@scotttrinh
Copy link
Collaborator

Hey @tomnz ! Super excited to have people with Effect experience looking at EdgeDB, I'm a big fan myself, although I've typically used EdgeQL rather than query builder expressions in my own experiments with Effect+EdgeDB.

I'd be open to exporting more types from the generated code, and probably even spending a little time crafting easier-to-use types in the public API to make it a little easier to write generic code like this. I don't have any ETA on when we'd have to resources to directly do this work, but I'm happy to assist any effort toward this end. I'm afraid the query builder code base takes a little time to onboard to, but if you make any progress on this and hit any roadblocks, feel free to ping me here. Otherwise, consider this a strong vote from me toward wanting to make stuff like this more possible.

@tomnz
Copy link
Author

tomnz commented Jul 17, 2024

I'd be open to exporting more types from the generated code, and probably even spending a little time crafting easier-to-use types in the public API to make it a little easier to write generic code like this.

I mean, if there are things to be done with the types that have this sort of use case in mind, that would be amazing! But even just access to more of the existing types would unlock a lot.

I don't have any ETA on when we'd have to resources to directly do this work, but I'm happy to assist any effort toward this end.

It's unlikely I will have time to pick this up either 🙃 but will keep that in mind. Thanks!

@riwsky
Copy link

riwsky commented Jul 18, 2024

@scotttrinh when you say "typically used EdgeQL rather than query builder expressions", are you referring to https://docs.edgedb.com/libraries/js/queries ? or are you passing EdgeQL some other way?

@scotttrinh
Copy link
Collaborator

@riwsky Oh, just using the query* functions and supplying the type manually, but the queries generator is even better since it does the parameter and return type generation itself, and checks syntax at generate-time.

But basically something like:

      Effect.gen(function* (_) {
        const created = yield* _(
          Effect.tryPromise({
            try: () =>
              data.client.queryRequiredSingle<User>(
                `\
with
  identity_id := <uuid>$identity_id,
  new_identity := (select ext::auth::Identity filter .id = identity_id),
insert User {
  identities := new_identity
};`,
                { identity_id }
              ),
            catch: (err) => new DataError((err as Error).message),
          })
        );

        return created;
      })

@zerosym
Copy link

zerosym commented Aug 20, 2024

I've been dipping my toes into using Effect as well. This is my hacked together attempt at integrating Edgedb with it:

type ExtractExpr<T> = T extends $expr_WithParams<infer _Params, infer _Expr> ? T : never
type ExprRun<T> = ExtractExpr<T>['run']

const EdgedbImpl = (client: edgedb.Client) => ({
	runQuery: <T>(query: ExtractExpr<T>, params: Parameters<ExprRun<T>>['1']) =>
		pipe(
			Effect.tryPromise({
				try: () => {
					return query.run(client, params) as Promise<$infer<T>>
				},
				catch: () => {
					// Do something...
				},
			}),
			Effect.tap(() => Effect.logInfo('Edgedb query')),
			Effect.withLogSpan('duration')
		) 
})

Update: unfortunately this seems to run into Type instantiation is excessively deep and possibly infinite. with more complicated queries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants