babel / minify

:scissors: An ES6+ aware minifier based on the Babel toolchain (beta)
https://babeljs.io/repl
MIT License
4.39k stars 225 forks source link

babel-preset-minify fatally breaking for-of loops #976

Open Macil opened 5 years ago

Macil commented 5 years ago

Bug Report

Current Behavior babel-preset-minify breaks for-of loops in some cases. It seems like there's two necessary conditions:

  1. babel-preset-env detects that the value being looped over is an array and activates its array-iteration fast-path.
  2. A conditional with a return statement exists before the for-of loop.

Input Code

function Main() {
  if (Math.random() > 0.5) {
    return;
  }
  const templates = [];
  for (const template of templates) {
    template.foo();
  }
}

Current Output

Here's the buggy output prettified:

"use strict";
function Main() {
  if (!(0.5 < Math.random()))
    for (var template, templates = [], _i = 0; _i < (void 0).length; _i++)
      (template = (void 0)[_i]), template.foo();
}

Note the presence of (void 0).length and (void 0)[_i], which will always cause exceptions. Each case of (void 0) should be templates.

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

{
  "presets": [
    "@babel/preset-env",
    [
      "babel-preset-minify",
      {
        "builtIns": false,
        "mangle": false
      }
    ]
  ]
}

Environment

{
  "dependencies": {
    "@babel/cli": "7.6.4",
    "@babel/core": "7.6.4",
    "@babel/preset-env": "7.6.3",
    "babel-preset-minify": "0.5.1"
  }
}

Additional context I ran into this issue while using Storybook, which includes babel-preset-minify by default in production builds.

babel-bot commented 5 years ago

Hey @Macil! 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."

wessberg commented 4 years ago

My team experienced this issue as well in a large code base. Giving the evaluate: false option to turn off the babel-plugin-minify-constant-folding solves the issue, so it would seem that it is related to that one.

clarle commented 4 years ago

I can reproduce it with your example. For my notes, here's what it looks like with evaluate: false as @wessberg mentioned:

"use strict";
function Main() {
    if (!(0.5 < Math.random()))
        for (var template, templates = [], _i = 0, _templates = templates; _i < _templates.length; _i++)
            (template = _templates[_i]), template.foo();
}

The preset-env preset is running first and transforming it into this:

"use strict";

function Main() {
  if (Math.random() > 0.5) {
    return;
  }

  var templates = [];

  for (var _i = 0, _templates = templates; _i < _templates.length; _i++) {
    var template = _templates[_i];
    template.foo();
  }
}