A way to disable 'preventAwait'

[wirekang]

Although @koskimas closed #693 as 'wontfix', I'm suggesting a way to disable the behavior of preventAwait, with a small footprint. We often returning non-Promise value in async function or then() chains. It would be great if Kysely have an option for those situation, for people who aren't Knex-newbie or know how to execute query.

The name 'allowNoopAwait' is a little bit temporary, it can be renamed as 'allowAwait', 'ignoreAwait', 'disablePreventAwait', 'iKnowHowToExecute' or something you suggest.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
kysely	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Sep 29, 2024 9:31am

[igalklebanov]

Hey 👋

We often returning non-Promise value in async function or then() chains. It would be great if Kysely have an option for those situation

I didn't get the necessity quite honestly. Can you elaborate on that?

[wirekang]

I didn't get the necessity quite honestly. Can you elaborate on that?

The code from original issue #693, and I'm not sure I can find better example.. Yeah it is not "often" but "rarely", sorry for overstating.
But it worth considering turning off the behavior that throwing error without actual logical reason.

[koskimas]

But it worth considering turning off the behavior that throwing error without actual logical reason.

There is a logical reason. People coming from knex and objection would make the mistake of awaiting queries without execute. Kysely is heavily inspired by knex which naturally leads people thinking it behaves the same.

[wirekang]

There is a logical reason. People coming from knex and objection would make the mistake of awaiting queries without execute. Kysely is heavily inspired by knex which naturally leads people thinking it behaves the same.

@koskimas I didn't denied it. I meant "programming logic error". Anyways, It was a light suggestion, never mind.

[koskimas]

I think it wouldn't hurt to add this. But remove the delete clazz.prototype.then thing. Having a simple then method that simply values through when prevent = false shouldn't have any measurable effect on performance.

Edit: Or actually, will that cause infinite recursion? Does resolve call .then?

[wirekang]

@koskimas Yes. That was the reason I have to delete then.

[matthew-holder-revvity]

preventAwait actually causes issues with anybody trying to return partially built queries from async functions. Not sure if the builders originally implemented the thenable protocol, but it's obvious it shouldn't be used as a means to execute the queries. It does mean that any return of queries or an attempt at query composition in async functions cannot work with preventAwait on the builders.

[marceloverdijk]

I'm hitting this as well to use a async function to enhance a query...

I'm trying to create a function that enhances the query (SelectQueryBuilder) with pagination, sorting and filtering options that are derived from a HTTP request.

This function looks like:

  async applyQueryParams<DB, TB extends keyof DB, O>(qb: SelectQueryBuilder<DB, TB, O>): Promise<SelectQueryBuilder<DB, TB, O>> {
    const data = await this.getValidatedData<PaginationSchema & SortSchema & FilterSchema>();
    const page = .. // derive page
    const pageSize = .. // derive page size
    const sort = .. // derive sort
    const filter = .. // derive filter

    console.log("Query builder before pagination:", qb);

    qb = qb.limit(pageSize);
    qb = qb.offset((page - 1) * pageSize);

    console.log("Query builder after pagination:", qb);

    return qb;
  }

this function needs to be async as the getValidatedData() is also async.

in a handler I call applyQueryParams like:

async handle(c: C) {

    let qb = c.var.mydb
      .selectFrom('customer')
      .selectAll('customer');

    console.log("Before applying query params:", qb);

    qb = await this.applyQueryParams(qb);

    console.log("After applying query params:", qb);

    const customers = await qb.execute();

but this fails with:

Before applying query params: SelectQueryBuilderImpl {}
Query builder before pagination: SelectQueryBuilderImpl {}
Query builder after pagination: SelectQueryBuilderImpl {}
✘ [ERROR] Error: don't await SelectQueryBuilder instances directly. To execute the query you need to call `execute` or `executeTakeFirst`.

I understand the rationale behind the preventAwait for knex users, but Typescript users - not coming from Knex - are now confused and limited.

[koskimas]

I'll remove preventAwait if you guys promise the handle all incoming issues it will cause. I'll just tag you and ignore the issue. Sounds good?

[koskimas]

Surely without a thenable interface on the queries, there should be no issue; other than folks from knex wondering why awaiting the query doesn't work, right?

And objection and a bunch of other ORMs. That's a big part of Kysely's users. What's your point?

[koskimas]

Yet another issue about this. Let's just remove preventAwait. Let's see what happens in terms of issues.

@wirekang If you have time and interest, could you remove preventAwait everywhere in this PR and rebase it on top of the v0.28 branch? Just let me know if not and let's close this and I'll remove it later.

[wirekang]

@koskimas Just removed all preventAwait related lines from v0.28 branch. Is it what you meant? (This PR is for master branch).

[wirekang]

New PR for v0.28 branch: #1160