Not-so-pure IIFE async generators stripped out in production.

[phyllisstein]

Choose one: is this a bug report or feature request?

Bug report.

Input Code

;(async () => {
  await import('app')
})()

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

module.exports = {
  presets: [
    ['@babel/env', {
      loose: true,
      modules: process.env.npm_lifecycle_event === 'build:es' ? false : 'commonjs',
      useBuiltIns: 'usage',
    }],
    '@babel/typescript',
    '@babel/react',
    '@babel/stage-0',
  ],
}

Expected Behavior

Babel should transpile the async IIFE down such that the app module is imported, in both development and production environments.

Current Behavior

Following #6803, Babel adds a /*#__PURE__*/ annotation to the output code:

/*#__PURE__*/
_asyncToGenerator(
/*#__PURE__*/
regeneratorRuntime.mark(function _callee() {
  return regeneratorRuntime.wrap(function _callee$(_context) {
    while (1) {
      switch (_context.prev = _context.next) {
        case 0:
          _context.next = 2;
          return import("./src/app");

        case 2:
        case "end":
          return _context.stop();
      }
    }
  }, _callee, this);
}))();

In production builds, UglifyJS drops the function by default, as described in its documentation:

• side_effects (default: true) -- Pass false to disable potentially dropping
  functions marked as "pure". A function call is marked as "pure" if a comment
  annotation /*@__PURE__*/ or /*#__PURE__*/ immediately precedes the call. For
  example: /*@__PURE__*/foo();

Pasting the output code into https://skalman.github.io/UglifyJS-online/ results in its being uglified entirely away. I'm not sure whether this was the behavior intended by the original pull request, but the heuristic that determines a function's purity seems to paint with a pretty broad brush:

https://github.com/satya164/babel/blob/42c740ca0a009ac4019a339c7f9be9293c93ab8c/packages/babel-helper-remap-async-to-generator/src/index.js#L76-L85

Possible Solution

Roll back #6803.

Context

We use async IIFEs to coordinate module loading and webfont availability. It was surprising when we found that the application wouldn't boot at all in production, and took an afternoon's digging to understand that the "pure" annotation was the culprit.

Your Environment

software	version(s)
Babel	v7.0.0-beta.34
node	v9.2.0
npm	v5.6.0
Operating System	macOS 10.13.2 (17C83a)

[babel-bot]

Hey @phyllisstein! 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]

Babel should output something like

(
  /*#__PURE__*/
  _asyncToGenerator(
    /*#__PURE__*/
    regeneratorRuntime.mark(function _callee() {
      return regeneratorRuntime.wrap(function _callee$(_context) {
        while (1) {
          switch (_context.prev = _context.next) {
            case 0:
              _context.next = 2;
              return import("./src/app");
    
            case 2:
            case "end":
              return _context.stop();
          }
        }
      }, _callee, this);
    }))
)();

So the __PURE__ annotation is attatched to the correct function call.

[Andarist]

The full rollback of #6803 is not needed. The mentioned heuristics need to be fine-tuned and exclude cases when async function is immediately invoked. This is somewhat caused by the fact that it takes only a single annotation to mark all nested calls as pure. Examples:

/*#__PURE__*/fn1(fn2())
/*#__PURE__*/fn()()()()

I've once raised concern about this behaviour here but we couldn't think of any real life scenarios when that would be dangerous and we might have found the first one here, cc @kzc.

Anyway - we need to be more careful about inserting those annotations, but the same kinda applies to any transformation which babel does. Only the real world and real applications can verify assumptions made by the plugins and if the logic behind them is bullet proof.

[nicolo-ribaudo]

I just realized that what I said in the comment above doesn't make sense, since even if the call is pure it can't be dropped (otherwise it becomes undefined())

[kzc]

@Andarist Uglify's behavior regarding this pure annotation is set in stone at this point - it is used in the wild and is not likely going to change. I'd advise to accommodate it with parentheses or other means for this use case. I don't think that it's a [good first issue] since #6803 is breaking code. Either a revert or a timely PR is in order by someone familiar with the issue and the Babel code base.

[Jessidhia]

I'm not sure it's that big of a deal to change Uglify's behavior. It would be a non-breaking change -- a code size regression, maybe, but it would, in no way, cause working code to not work anymore. But you may have different guarantees in your project.

[kzc]

Uglify is behaving as intended in the top post. The call expression is greedy.

[kzc]

@Andarist Is this the fix you had in mind?

--- a/packages/babel-helper-remap-async-to-generator/src/index.js
+++ b/packages/babel-helper-remap-async-to-generator/src/index.js
@@ -65,23 +65,25 @@ const awaitVisitor = {
 export default function(path: NodePath, file: Object, helpers: Object) {
   path.traverse(awaitVisitor, {
     file,
     wrapAwait: helpers.wrapAwait,
   });
 
+  const isCall = path.parentPath.isCallExpression();
+
   path.node.async = false;
   path.node.generator = true;
 
   wrapFunction(path, helpers.wrapAsync);
 
   const isProperty =
     path.isObjectMethod() ||
     path.isClassMethod() ||
     path.parentPath.isObjectProperty() ||
     path.parentPath.isClassProperty();
 
-  if (!isProperty) {
+  if (!isProperty && !isCall) {
     annotateAsPure(
       path.isDeclaration() ? path.get("declarations.0.init") : path,
     );
   }
 }

Note: untested.

[Andarist]

@kzc Yeah pretty much, that's why I've marked it as "good first issue". Babel@7 is at the moment at the beta stage and I believe that premature reverts would only stall things, it is expected that some bugs might still appear during that stage - we obviously hope to fix all of them as soon as possible.

As beta versions should be pinned in your package.json, I believe more appropriate course of action is to downgrade your babel until a fix is released.

[Jessidhia]

Ended up claiming this because it kinda affected me at work 😅

Couldn't update to newer betas to include other bug fixes because I'd be bringing in this bug instead.

[nicolo-ribaudo]

Fixed by #6999

[phyllisstein]

Thanks for picking this up @Kovensky , and thanks for the heads-up @nicolo-ribaudo ! Super grateful for your time and your help.