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

allow await in invalid expression wrapper #3146

Merged
merged 4 commits into from
Nov 24, 2022

Conversation

ekwoka
Copy link
Contributor

@ekwoka ekwoka commented Sep 7, 2022

Solves: #3118

Problem: expressions that are identified as invalid right hand expressions are wrapped in an IIFE, which brakes any await calls.

Solution: make IIFE async

Expressions are normally treated as async functions, but this was one situation in which they were not.

@ekwoka ekwoka changed the title Fix expression wrapper allow await in invalid expression wrapper Sep 7, 2022
@calebporzio
Copy link
Collaborator

Yeah, this makes sense... my only hesitation is that now the function won't run synchronously right? it will be pushed to the microtask queue? Which in theory changes the behavior however so slightly.

So maybe hit me with opinions on if this is OK?

@joshhanley
Copy link
Collaborator

@ekwoka have pushed an empty commit to trigger CI, as I think the failing test is unrelated, but just want to make sure

@joshhanley
Copy link
Collaborator

@SimoTod if you get the chance, can you comment on Caleb's concern above?

@ekwoka
Copy link
Contributor Author

ekwoka commented Nov 23, 2022

It shouldn't change the behavior for anything else, in this case.

Async functions, when called, run synchronously up to the first await, and then the resolution is handled in the queue, but the promises from the outer AsyncFunction and this inner async()={} should virtually collapse together.

@joshhanley joshhanley merged commit 2369462 into alpinejs:main Nov 24, 2022
@joshhanley
Copy link
Collaborator

Yep makes sense to me. Thanks for the PR!

@ekwoka ekwoka deleted the fix-expression-wrapper branch November 25, 2022 01:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants