Move top-level await out of experimental

[guybedford]

Now that TLA is Stage 3 and the Chrome implementation is currently in the process of being merged, it is coming around the time where the experimental flag is no longer necessary.

I would personally see no risk in landing this, but if there was a desire to be more conservative it could be worth waiting a couple of months for the Chrome implementation to at least land first too, although I'm not sure I see any benefit in waiting at this point either.

[codecov]

Codecov Report

Merging #3089 into master will increase coverage by 0.04%.
The diff coverage is 90%.

@@            Coverage Diff             @@
##           master    #3089      +/-   ##
==========================================
+ Coverage   93.17%   93.22%   +0.04%     
==========================================
  Files         172      172              
  Lines        6040     6038       -2     
  Branches     1800     1800              
==========================================
+ Hits         5628     5629       +1     
+ Misses        221      218       -3     
  Partials      191      191

Impacted Files	Coverage Δ
src/ast/nodes/LogicalExpression.ts	98.7% <ø> (ø)	⬆️
src/rollup/index.ts	96.96% <ø> (ø)	⬆️
src/ast/nodes/ConditionalExpression.ts	96.15% <ø> (ø)	⬆️
src/watch/index.ts	85.32% <100%> (ø)	⬆️
src/utils/PluginContext.ts	100% <100%> (ø)	⬆️
src/utils/renderNamePattern.ts	100% <100%> (ø)	⬆️
src/utils/deconflictChunk.ts	100% <100%> (ø)	⬆️
cli/run/watch.ts	43.58% <66.66%> (+1.61%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f4af5fe...0472cee. Read the comment docs.

[lukastaegert]

To my knowledge, our implementation is rather non-spec-compliant, which is why I would avoid this unless we reimplement it.

[guybedford]

@lukastaegert are you referring to the timing issues we discussed before? Significant effort was actually put into the specification here to ensure there are no subtle timing issues with optimized output.

For the case of multiple async dependencies in the same chunk they will execute serially under the Rollup output instead of in parallel.

So if I had:

a.js

console.log('sync part a');
await 0;
console.log('async part a');

b.js

console.log('sync part b');
await 0;
console.log('async part b');

Then the expected output should be "sync part a", "sync part b", "async part a", "async part b", while under the Rollup output it would be "sync part a", "async part a", "sync part b", "async part b".

Fixing the above would be basically hoisting within a group of async siblings in a chunk the sync statements up to the first await statement or expression.

Personally I think we should wait on this advanced feature until we have module splitting optimizations in place, but if you would rather unblock unflagging on that then let's set up an issue for sure.

[shellscape]

@lukastaegert this would be a big win. can you please revisit the PR when you have the chance?

[lukastaegert]

Actually instead of renaming the flag (which happens in this PR) I would rather remove the flag altogether and make this “always on”. My concern is that the current execution order is sub-optimal because sibling dependencies are not executed in parallel, but this could be improved later via some code-splitting changes.

[lukastaegert]

By sub-optimal I mean non-spec compliant

[shellscape]

@lukastaegert OK, I'll tackle that change. Would this require a major version?

[lukastaegert]

No, it cannot break anything, there would just be no error when using TLA

[lukastaegert]

This looks good to me know, I would release it in the next minor version ina few days, together with some of the other PRs floating around.

[shellscape]

Thanks for jumping on this before I was able to. My free time has been consumed by the plugins repo as of late.

LGTM - ship it!