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

Incorrect transform with async/await #565

Closed
CyriacBr opened this issue Feb 16, 2020 · 14 comments · Fixed by #968 or #958
Closed

Incorrect transform with async/await #565

CyriacBr opened this issue Feb 16, 2020 · 14 comments · Fixed by #968 or #958
Labels
increased scope Increases project scope, or is out of scope. known issue

Comments

@CyriacBr
Copy link

microbundle uses the babel plugin babel-plugin-transform-async-to-promises which has this serious bug as it changes the behavior of your code once transpilled.
Sharing that here as this could save a few hours of headache and insanity to someone. I wasn't that lucky.

@developit
Copy link
Owner

For sure. I think short term we can detect switch cases within async functions and throw an error.

@Djaler
Copy link

Djaler commented Apr 17, 2020

@developit so what about it?

@developit
Copy link
Owner

@Djaler this is an upstream issue that needs to be fixed in the Babel plugin.

@Djaler
Copy link

Djaler commented Apr 17, 2020

@Djaler this is an upstream issue that needs to be fixed in the Babel plugin.

I mean this :

we can detect switch cases within async functions and throw an error

@Djaler
Copy link

Djaler commented Apr 19, 2020

Also, you can just stop using this plugin

@developit
Copy link
Owner

@Djaler the value proposition of microbundle is that output is unabashedly minimal. Switching to regenerator output would be a very large step in the wrong direction. That being said, depending on how modern output moves forward, it might be possible to sacrifice some of the output quality in legacy builds as long as most package consumers end up with the modern version. Currently that's not the case, but it may be soon.

@Djaler

This comment has been minimized.

@marvinhagemeister marvinhagemeister changed the title microbundle users should be aware of this issue Incorrect transform with async/await May 6, 2020
@PeterBurner
Copy link
Contributor

is there a workaround for now?
will things break if we pin babel-plugin-transform-async-to-promises to v0.8.14?
or do we have to downgrade to microbundle v0.11.0 ?

@Nick-Lucas
Copy link

Nick-Lucas commented Jul 23, 2020

the value proposition of microbundle is that output is unabashedly minimal. Switching to regenerator output would be a very large step in the wrong direction

Given that the upstream package hasn't had a commit since October 2019, and this critical bug was reported way back then, wouldn't it be a good idea to look at alternatives from shipping a broken dependency? If there's a working version that could be pinned like @PeterBurner suggests, then that's great, but regenerator is very common in the wild, so it doesn't really seem like a bad choice if it's actually stable.

@developit
Copy link
Owner

The workaround is to avoid direct combined usage of async/await and switch. We can't produce acceptable output for it with or without regenerator, so consider it unsupported by Microbundle.

The fact that regenerator is common does not make it the right solution. Regenerator is routinely duplicated tens or hundreds of times in bundles because a large percentage of npm modules bundle it for the sake of correctness. Unfortunately, no tools exist that can fix that situation, which leaves us with two bad options: ship regenerator and bloat 16,000 npm modules, or ship async-to-promises and advise Microbundle users to avoid the problem syntax. We only transpile first-party code, so this only applies to code written by the person configuring the bundler - the code that is easiest and most valuable to change.

Ultimately, Microbundle isn't webpack or Rollup - it's not a general-purpose tool and it doesn't cater to the same breadth of use-cases. It exists purely to provide the smallest output for the subset of modern input that we be compiled with minimal overhead. This particular async/await issue is gnarly, sure, but keep in mind Microbundle and indeed most meta-bundlers run Babel in loose mode anyway, which has far more edge cases in order to produce output of an acceptable size.

I have yet to find any reasonable solution to this aside from advocating for modern JS output that is (thankfully) immune to the problem, which is exactly what we are doing. Once enough bundlers (or really bundler configs) support modern npm modules, this problem goes away entirely. At that time, we can happily switch to generating larger more accurate ES5 bundles for the rare cases in which someone would end up using them.

@developit developit added increased scope Increases project scope, or is out of scope. known issue labels Dec 18, 2020
@MartinDawson
Copy link

Once enough bundlers (or really bundler configs) support modern npm modules, this problem goes away entirely. At that time, we can happily switch to generating larger more accurate ES5 bundles for the rare cases in which someone would end up using them.

Just curious but roughly when do you think this will be?

@make-github-pseudonymous-again

Upstream allegedly fixed with rpetrich/babel-plugin-transform-async-to-promises#49 18 days ago.

@PeterBurner
Copy link
Contributor

Upstream allegedly fixed with rpetrich/babel-plugin-transform-async-to-promises#49 18 days ago.

Is there some action required on microbundle side? Eg. updating dependencies.
Or can this issue be closed?

@developit
Copy link
Owner

@PeterBurner I believe a new publish of Microbundle would automatically include this as long as we run npm update. Might be better to submit a PR with the part release of this transform bumped though.

PeterBurner pushed a commit to PeterBurner/microbundle that referenced this issue Jun 3, 2022
* Update babel-plugin-transform-async-to-promises to v0.8.18
PeterBurner pushed a commit to PeterBurner/microbundle that referenced this issue Jun 3, 2022
* Update babel-plugin-transform-async-to-promises to v0.8.18
PeterBurner added a commit to PeterBurner/microbundle that referenced this issue Jun 3, 2022
* Update babel-plugin-transform-async-to-promises to v0.8.18
PeterBurner added a commit to PeterBurner/microbundle that referenced this issue Jun 3, 2022
* Update babel-plugin-transform-async-to-promises to v0.8.18
rschristian pushed a commit that referenced this issue Jun 4, 2022
* fix: Incorrect transform with async/await (#565)

* Update babel-plugin-transform-async-to-promises to v0.8.18

* Add changeset

* Update jest test snapshots

Co-authored-by: Björn Saja <bjoern@codeatelier.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
increased scope Increases project scope, or is out of scope. known issue
Projects
None yet
7 participants