[parcel 2, regression] parcel optimizes away parentheses and makes valid code invalid / makes terser fail

[danieltroger]

🐛 bug report

I am lacking words for this issue as I don't know what the features are called. But basically code where the result of an assignment to a variable is awaited is correctly built in parcel@2.0.0-nightly.511 but breaks shortly thereafter and is broken in the newest version.

🎛 Configuration (.babelrc, package.json, cli command)

Please checked attached .zip file, run npm install and then npm run build

🤔 Expected Behavior

the code should transpile to this or similar, like it does in version 511:

! function() {
  var t = {
    promise: () => {}
  };
  console.log(async function() {
    return await (t.promise = (await fetch("/")).text())
  }(), t)
}();

😯 Current Behavior

Parcel removes parantheses around the definition of the variable, breaking the code:

daniel@iMac:/tmp/parcel-pleeeaase-wtf$ npm run build

> build
> parcel build src/main.ts --log-level verbose --detailed-report 100 --target modern

🚨 Build failed.
@parcel/optimizer-terser: Invalid assignment
/private/tmp/parcel-pleeeaase-wtf/src/main.ts:3:31
  2 | async function main(){
> 3 |   return await (obj.promise = (await fetch("/")).text())
>   |                               ^ Invalid assignment
  4 | }
  5 | 
It's likely that Terser doesn't support this syntax yet.
@parcel/optimizer-terser: Invalid assignment
  5 |   async function $244ca7e764f37acfc6c3db0b97d89bba$var$main() {
> 6 |     return await $244ca7e764f37acfc6c3db0b97d89bba$var$obj.promise = (await fetch("/")).text();
>   |                                                                   ^ Invalid assignment
  7 |   }
  8 |   console.log($244ca7e764f37acfc6c3db0b97d89bba$var$main(), $244ca7e764f37acfc6c3db0b97d89bba$var$obj);
It's likely that Terser doesn't support this syntax yet.
npm ERR! code 1
npm ERR! path /private/tmp/parcel-pleeeaase-wtf
npm ERR! command failed
npm ERR! command sh -c parcel build src/main.ts --log-level verbose --detailed-report 100 --target modern

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/daniel/.npm/_logs/2021-02-03T10_59_06_681Z-debug.log

When running parcel with --no-optimize it successfully builds to this:

(function () {
  var $244ca7e764f37acfc6c3db0b97d89bba$var$obj = {
    promise: () => {}
  };
  async function $244ca7e764f37acfc6c3db0b97d89bba$var$main() {
    return await $244ca7e764f37acfc6c3db0b97d89bba$var$obj.promise = (await fetch("/")).text();
  }
  console.log($244ca7e764f37acfc6c3db0b97d89bba$var$main(), $244ca7e764f37acfc6c3db0b97d89bba$var$obj);
})();

But that code is illegal and throws Uncaught SyntaxError: invalid assignment left-hand side when being executed.

💁 Possible Solution

Revert back to what the old version did which is preserving the parantheses and not breaking the code.

🔦 Context

I moved all my css imports to use bundle-text after I got to know this workaround which makes the css minified. When building all my scripts again to check the transpiled code for unminified css one project failed to build. I investigated and opened this bug report.

💻 Code Sample

parcel-parantheses-bug.zip

This is the line it can't deal with: return await (obj.promise = (await fetch("/")).text())

🌍 Your Environment

Software	Version(s)
Parcel	2.0.0-nightly.566+dba96de0
Node	v15.6.0
npm/Yarn	7.5.2
Operating System	macOS High Sierra 10.13.6

[mischnic]

A bug in astring: davidbonnet/astring#435

[danieltroger]

Thanks @mischnic. I checked that it wasn't babel but didn't think of astring.

[danieltroger]

Would it be a good idea to add a test case for it anyways so someone notices if it breaks again?

[mischnic]

astring itself has tests here: https://github.com/davidbonnet/astring/tree/master/src/tests

But after fixing it in astring, we should add a test here as well: https://github.com/parcel-bundler/parcel/blob/v2/packages/shared/babel-ast-utils/test/fixtures/await.js

[davidbonnet]

Clearly overlooked the precedence of await and yield operators four years ago 🤦‍♂️: davidbonnet/astring@a7b3dd2

In case you're using EXPRESSIONS_PRECEDENCE overrides, note that the precedence of AwaitExpression has changed:
davidbonnet/astring@d6166a2#diff-48d731d988c1539f085b428de2166d696b64ab21eabe0b3273492ee90232457cR82-L87

Edit: The precedence of YieldExpression has indeed not changed.

[mischnic]

@davidbonnet Thank you for the quick fix and release!

[davidbonnet]

First fix introduced a regression regarding the YieldExpression, now fixed in v1.6.2.

[danieltroger]

Thanks a lot guys for the quick fixes! Hope the PR makes it into the next nightly.