pure annotations incorrectly output, leading to bad bundles

[kzc]

Bug Report

Current Behavior

Pure annotations of calls within compound call expressions are not emitted correctly. They lack the necessary parentheses required to maintain the intent of the annotations.

Input Code

$ cat pure.js

/*@__PURE__*/ statement.should().be().dropped();
( /*@__PURE__*/ statement.should().be() ).kept();

Expected output

/*@__PURE__*/ statement.should().be().dropped();
( /*@__PURE__*/ statement.should().be() ).kept();

Actual incorrect generated code

$ babel pure.js -o pure.out.js && cat pure.out.js
"use strict";

/*@__PURE__*/
statement.should().be().dropped();

/*@__PURE__*/
statement.should().be().kept();

Pure annotations are greedy - without parentheses they apply to the longest subsequent call (or new) expression. When this babel output is passed through uglify-js, terser or rollup it produces an incorrect result:

$ uglify-js -V
uglify-js 3.6.0

$ cat pure.out.js | uglify-js -c
"use strict";

$ terser -V
terser 3.11.0

$ cat pure.out.js | terser -c
"use strict";

$ rollup -v
rollup v1.19.3

$ rollup pure.out.js -f es --silent
(!) Generated an empty bundle

You can see above that statement.should().be().kept(); was dropped due to the unintended altered pure annotation.

This is the desired result, as demonstrated by Rollup on the original input:

$ rollup pure.js -f es --silent
( /*@__PURE__*/ statement.should().be() ).kept();

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

{
    "presets": [
        [
            "@babel/preset-env",
            {
                "useBuiltIns": "entry",
                "corejs": 3
            }
        ]
    ],
    "plugins": [
        [
            "@babel/plugin-transform-runtime",
            {
                "corejs": false,
                "helpers": false,
                "regenerator": true,
                "useESModules": false
            }
        ]
    ]
}

Modules installed:

    "@babel/cli": "7.5.5",
    "@babel/core": "7.5.5",
    "@babel/plugin-transform-runtime": "7.5.5",
    "@babel/preset-env": "7.5.5",
    "@babel/runtime": "7.5.5",
    "core-js": "3.1.4",

Environment

• Babel version(s): 7.5.5 (@babel/core 7.5.5)
• Node/npm version: yarn@1.12.3
• OS: n/a
• Monorepo: n/a
• How you are using Babel: cli

Possible Solution

Parentheses could be output around pure annotated calls if nested within a call expression or new expression. Alternatively, an unnecessary pure annotation could be removed from a call subexpression from within a larger call expression.

A more complex example with an immediately invoked bind of an async function

$ cat async.js 
console.log(1);
(async function() {
    console.log((await this).toString());
}).bind("PASS")();
console.log(2);

Expected result:

$ cat async.js | node-v12.1.0 
1
2
PASS

Babel generated output:

$ babel async.js -o async.out.js && cat async.out.js
"use strict";

var _regenerator = _interopRequireDefault(require("@babel/runtime/regenerator"));

function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { "default": obj }; }

function asyncGeneratorStep(gen, resolve, reject, _next, _throw, key, arg) { try { var info = gen[key](arg); var value = info.value; } catch (error) { reject(error); return; } if (info.done) { resolve(value); } else { Promise.resolve(value).then(_next, _throw); } }

function _asyncToGenerator(fn) { return function () { var self = this, args = arguments; return new Promise(function (resolve, reject) { var gen = fn.apply(self, args); function _next(value) { asyncGeneratorStep(gen, resolve, reject, _next, _throw, "next", value); } function _throw(err) { asyncGeneratorStep(gen, resolve, reject, _next, _throw, "throw", err); } _next(undefined); }); }; }

console.log(1);

/*#__PURE__*/
_asyncToGenerator(
/*#__PURE__*/
_regenerator["default"].mark(function _callee() {
  return _regenerator["default"].wrap(function _callee$(_context) {
    while (1) {
      switch (_context.prev = _context.next) {
        case 0:
          _context.t0 = console;
          _context.next = 3;
          return this;

        case 3:
          _context.t1 = _context.sent.toString();

          _context.t0.log.call(_context.t0, _context.t1);

        case 5:
        case "end":
          return _context.stop();
      }
    }
  }, _callee, this);
})).bind("PASS")();

console.log(2);

If the babel generated output is run directly with node, it's correct:

$ cat async.out.js | node
1
2
PASS

However, the result is incorrect if the babel generated code is run through terser:

$ cat async.out.js | terser --toplevel -bc | node
1
2

$ cat async.out.js | terser --toplevel -bc
"use strict";

var obj;

(obj = require("@babel/runtime/regenerator")) && obj.__esModule;

console.log(1), console.log(2);

The result is also incorrect when the babel generated output is run through rollup without minification:

$ rollup async.out.js -f es --silent | node
1
2

$ rollup async.out.js -f es --silent
var _regenerator = _interopRequireDefault(require("@babel/runtime/regenerator"));

function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { "default": obj }; }

console.log(1);

console.log(2);

Here's how the babel generated output ought to look with the correct parentheses inserted:

$ cat async.out.corrected.js 
"use strict";

var _regenerator = _interopRequireDefault(require("@babel/runtime/regenerator"));

function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { "default": obj }; }

function asyncGeneratorStep(gen, resolve, reject, _next, _throw, key, arg) { try { var info = gen[key](arg); var value = info.value; } catch (error) { reject(error); return; } if (info.done) { resolve(value); } else { Promise.resolve(value).then(_next, _throw); } }

function _asyncToGenerator(fn) { return function () { var self = this, args = arguments; return new Promise(function (resolve, reject) { var gen = fn.apply(self, args); function _next(value) { asyncGeneratorStep(gen, resolve, reject, _next, _throw, "next", value); } function _throw(err) { asyncGeneratorStep(gen, resolve, reject, _next, _throw, "throw", err); } _next(undefined); }); }; }

console.log(1);

( /*#__PURE__*/
_asyncToGenerator(
/*#__PURE__*/
_regenerator["default"].mark(function _callee() {
  return _regenerator["default"].wrap(function _callee$(_context) {
    while (1) {
      switch (_context.prev = _context.next) {
        case 0:
          _context.t0 = console;
          _context.next = 3;
          return this;

        case 3:
          _context.t1 = _context.sent.toString();

          _context.t0.log.call(_context.t0, _context.t1);

        case 5:
        case "end":
          return _context.stop();
      }
    }
  }, _callee, this);
}))).bind("PASS")();

console.log(2);

The babel output with modifications now produces the desired result when run through terser:

$ cat async.out.corrected.js | terser -bc | node
1
2
PASS

...and with rollup:

$ rollup async.out.corrected.js -f es --silent | node
1
2
PASS

The diff between the babel output and the corrected babel output:

$ diff -u async.out.js async.out.corrected.js
--- async.out.js	2019-08-07 14:21:29.000000000 -0400
+++ async.out.corrected.js	2019-08-07 14:43:35.000000000 -0400
@@ -10,7 +10,7 @@
 
 console.log(1);
 
-/*#__PURE__*/
+( /*#__PURE__*/
 _asyncToGenerator(
 /*#__PURE__*/
 _regenerator["default"].mark(function _callee() {
@@ -33,6 +33,6 @@
       }
     }
   }, _callee, this);
-})).bind("PASS")();
+}))).bind("PASS")();
 
 console.log(2);

Additional information on pure annotations:

rollup/rollup#2429 (comment)

[babel-bot]

Hey @kzc! We really appreciate you taking the time to report an issue. The collaborators on this project attempt to help as many people as possible, but we're a limited number of volunteers, so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack community that typically always has someone willing to help. You can sign-up here for an invite.

[nicolo-ribaudo]

This is because in JavaScript parentheses aren't meaningful: the spec defines statement.should().be().kept() and ( statement.should().be() ).kept() as being the exact same expression.

If you need Babel to preserve parentheses, you can enable the createParenthesizedExpressions parser option.

[kzc]

I thought comments in Babel were already associated with specific AST nodes by default - i.e.: isPureAnnotated(node) - and this is just a paren output issue. Or are these annotations pointing to the wrong AST node without that parser option?

[kzc]

Adding parserOpts: { createParenthesizedExpressions: true }, to .babelrc allows the first test case to be parsed, emitted and correctly retained by uglify-js, terser and rollup:

$ babel pure.js
"use strict";

/*@__PURE__*/
statement.should().be().dropped();
(
/*@__PURE__*/
statement.should().be()).kept();

$ babel pure.js | terser -c
"use strict";statement.should().be().kept();

Unfortunately, running babel on the second test case with this new .babelrc produces this error:

$ babel async.js
Error: unknown Expression of type "ParenthesizedExpression"
    at Emitter.Ep.explodeExpression (node_modules/regenerator-transform/lib/emit.js:946:13)

[kzc]

I managed to get the async.js test case working with this patch:

diff -u node_modules/regenerator-transform/lib/emit.js-orig node_modules/regenerator-transform/lib/emit.js
--- node_modules/regenerator-transform/lib/emit.js-orig	2019-08-08 19:24:16.000000000 -0400
+++ node_modules/regenerator-transform/lib/emit.js	2019-08-08 19:23:28.000000000 -0400
@@ -942,6 +942,9 @@
       self.mark(after);
       return self.contextProperty("sent");
 
+    case "ParenthesizedExpression":
+      return finish(t.parenthesizedExpression(self.explodeExpression(path.get("expression"))));
+
     default:
       throw new Error("unknown Expression of type " + JSON.stringify(expr.type));
   }

$ babel async.js | terser -bc | node
1
2
PASS

$ babel async.js
"use strict";

var _regenerator = _interopRequireDefault(require("@babel/runtime/regenerator"));

function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { "default": obj }; }

function asyncGeneratorStep(gen, resolve, reject, _next, _throw, key, arg) { try { var info = gen[key](arg); var value = info.value; } catch (error) { reject(error); return; } if (info.done) { resolve(value); } else { Promise.resolve(value).then(_next, _throw); } }

function _asyncToGenerator(fn) { return function () { var self = this, args = arguments; return new Promise(function (resolve, reject) { var gen = fn.apply(self, args); function _next(value) { asyncGeneratorStep(gen, resolve, reject, _next, _throw, "next", value); } function _throw(err) { asyncGeneratorStep(gen, resolve, reject, _next, _throw, "throw", err); } _next(undefined); }); }; }

console.log(1);
(
/*#__PURE__*/
_asyncToGenerator(
/*#__PURE__*/
_regenerator["default"].mark(function _callee() {
  return _regenerator["default"].wrap(function _callee$(_context) {
    while (1) {
      switch (_context.prev = _context.next) {
        case 0:
          _context.t0 = console;
          _context.next = 3;
          return this;

        case 3:
          _context.t1 = (_context.sent).toString();

          _context.t0.log.call(_context.t0, _context.t1);

        case 5:
        case "end":
          return _context.stop();
      }
    }
  }, _callee, this);
}))).bind("PASS")();
console.log(2);

The patch is probably wrong, but it's a good start for someone who knows what they're doing to put together a proper PR for regenerator. I won't work on this further.

This problem wasn't trivial to debug. Is there a chance that Babel could enable the parser option createParenthesizedExpressions by default in future releases?

Thanks for your help @nicolo-ribaudo.

[kzc]

This old PR could be applicable to the async.js test case: #7044. It creates a pure annotation within a sequence expression ( 0, /*@__PURE__*/f() ) which ought to work regardless of the parser setting createParenthesizedExpressions.

AFAICT PR #7044 would not remedy the first test case in the top post when createParenthesizedExpressions is not enabled. Only cases where Babel itself inserts pure annotations.

[kzc]

Closing out issue since parentheses output behavior with respect to comments appears to be by design and the default is not likely going to change.