await QueryBuilder without execute

[wenerme]

Is your feature request related to a problem? Please describe.

I want to write a builder helper

  async read():Promise<QueryBuilder<E>> {
    const builder = this.repo.qb();
    builder.andWhere(await this.em.applyFilters(this.Entity.name, {}, {}, 'read'));
    // await builder will cause execute
    return builder;
  }

the workaround is

  async read():Promise<{builder:QueryBuilder<E>}> {
    const builder = this.repo.qb();
    builder.andWhere(await this.em.applyFilters(this.Entity.name, {}, {}, 'read'));
    // await builder will cause execute
    return { builder };
  }

Describe the solution you'd like

Maybe builder.untouch() ?

[B4nan]

I guess we could have a flag for this, that way you could also revert it back if you want to.

[wenerme]

I guess we could have a flag for this, that way you could also revert it back if you want to.

That will do, setExecuteMode or somthing ? Only trigger execute when getXyz

[B4nan]

It's actually more complicated, I can't find a way to do this conditionally in the existing then method.

[rcfox]

What if you wrapped the builder in another Promise? Alternatively, make the read function non-async:

read(): Promise<QueryBuilder<E>> {
    return this.em.applyFilters(this.Entity.name, {}, {}, 'read').then(result => {
        const builder = this.repo.qb();
        builder.andWhere(result);
        return builder;
    });
}

[wenerme]

What if you wrapped the builder in another Promise? Alternatively, make the read function non-async:

read(): Promise<QueryBuilder<E>> {
    return this.em.applyFilters(this.Entity.name, {}, {}, 'read').then(result => {
        const builder = this.repo.qb();
        builder.andWhere(result);
        return builder;
    });
}

await the result will execute the query

[boenrobot]

@B4nan I have a potentially controversial proposal to solve this issue...

Remove the ability to await the query builder. That is, instead of then(), name that method anything else.... like maybe, toPromise().

IMO, anything with "builder" in its name should be inert (as in, no side effects), until explicitly materialized, but the mere presence of then() is the problem. I understand the convenience provided though (the method is not just an alias to another QB method...), so renaming it to something else like toPromise(), instead of then() would provide an easy migration path. Instead of "await qb", such lines would need to do "await qb.toPromise()".

[B4nan]

We could surely do that, but only in v7, as it's breaking. I'd also argue it wouldn't be a simple upgrade as this can't be easily done via find/replace (unlike the refactoring the other way around).

[boenrobot]

Maybe introduce the renamed method before then (say, 6.2), and make then() an alias to it, to give users a cross working version? And add a deprecation warning log, to help locate non-refactored usages.

[B4nan]

I'd rather find a solution instead of doing that (and I can imagine some, it's just quite a chore implementing them properly, all for a very small benefit). If we are to deprecate this, it can happen in some future release, when we start thinking about v7 (which could be sometime next year), no reason to be so fast with this.

[B4nan]

Maybe the actual solution to this would be to provide out of box support for respecting filters in the QB instead. That's probably the main use case for returning the QB from async methods? We could have some QB flag to do this before the execution. It wouldn't work when trying to get the query as string out of it, since that is a sync path and adding filters is an async operation, but for execution, it would work just fine. For people who want to get the string query first, they could still use the em.applyFilters API explicitly, or we could have some async variant of the qb.getQuery() method and throw from the sync one if this new flag would be enabled.

[wenerme]

applyFilters is the major usecase, but I also return qb when I want to open an possible extension point like

  // allow child class to add static conditions
  protected async createQueryBuilder(): Promise<{ builder: QueryBuilder<E> }> {
    return createQueryBuilder(this.em, this.Entity);
  }

but ok with the object return.

I do hope there is a consistant way to apply the filter.

[B4nan]

protected async createQueryBuilder(): Promise<{ builder: QueryBuilder<E> }> {

But what's the point in having that method async? What other use cases than the filters can you think of that would require async method?

[B4nan]

Opened an RFC #5588, I don't mind removing the awaitability but it feels like a pretty big BC, since it wont be easy to find/replace this. I don't see another way around this really.

[wenerme]

protected async createQueryBuilder(): Promise<{ builder: QueryBuilder }> {

But what's the point in having that method async? What other use cases than the filters can you think of that would require async method?

currently the async is for em.applyFilters

[B4nan]

People have spoken (#5588), I will remove the awaitability in v7.

[B4nan]

FYI I just added the qb.applyFilters shortcut (0aaaa4fe7087e874cdb97b81ddf6c9da90def259).