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

A way to disable 'preventAwait' #748

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

wirekang
Copy link
Contributor

@wirekang wirekang commented Oct 26, 2023

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
Copy link

vercel bot commented Oct 26, 2023

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 Jan 14, 2024 9:25am

@igalklebanov
Copy link
Member

igalklebanov commented Nov 7, 2023

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?

@igalklebanov igalklebanov added the enhancement New feature or request label Nov 7, 2023
@wirekang
Copy link
Contributor Author

wirekang commented Nov 8, 2023

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
Copy link
Member

koskimas commented Nov 9, 2023

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.

@koskimas koskimas closed this Nov 9, 2023
@koskimas koskimas reopened this Nov 9, 2023
@wirekang
Copy link
Contributor Author

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
Copy link
Member

koskimas commented Jan 13, 2024

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
Copy link
Contributor Author

wirekang commented Jan 14, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using sql helper in promise chains causes unexpected preventAwait errors
3 participants