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
await QueryBuilder without execute #4876
Comments
I guess we could have a flag for this, that way you could also revert it back if you want to. |
That will do, |
It's actually more complicated, I can't find a way to do this conditionally in the existing |
What if you wrapped the builder in another Promise? Alternatively, make the
|
await the result will execute the query |
@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()". |
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). |
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. |
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. |
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 |
// 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. |
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? |
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. |
currently the async is for |
Is your feature request related to a problem? Please describe.
I want to write a builder helper
the workaround is
Describe the solution you'd like
Maybe
builder.untouch()
?The text was updated successfully, but these errors were encountered: