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

Minify throws TypeError when minifying if with return statement in it. #967

Closed jackmaw closed 5 years ago

jackmaw commented 5 years ago

Describe the bug

Hi everyone, i'm using @babel/core 7.6.0 and babel-minify 0.5.1 and i got TypeError, trying minifying code provided below:

To Reproduce

Minimal code to reproduce the bug

function foo(bar) {
    if (bar) {
        return 'baz';
    }
}

Actual Output

TypeError: unknown: Cannot set property inList of [object Object] which has only a getter
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! ring-react-components@1.12.2 test-minify: `minify ./src/components/ActionsBox.js --outFile mActionBox.js`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the ring-react-components@1.12.2 test-minify script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

How are you using babel-minify

minify ./src/components/ActionsBox.js --outFile mActionBox.js

Also, can be reproduced in babljs.io: https://babeljs.io/repl/#?babili=true&browsers=&build=&builtIns=false&spec=false&loose=false&code_lz=GYVwdgxgLglg9mABMOcAUAjAhgJwJSIDeAUAJAzCKa4EmL2Kk4CmUIOS2AXgNzEOIAvsUFA&debug=false&forceAllTransforms=false&shippedProposals=false&circleciRepo=&evaluate=false&fileSize=false&timeTravel=false&sourceType=module&lineWrap=false&presets=babili&prettier=false&targets=&version=7.7.1&externalPlugins=

{
  plugins: [],
  presets: []
}

Possible solution

Error is somewhere in babel-plugin-minify-simplify, when i disable this plugin minifying works, but i cant do that for every dependency and i think that minifying should just work for that case.

fabienrohrer commented 5 years ago

I just run into the same issue after updating my build system, caused by this line:

https://github.com/babel/minify/blob/master/packages/babel-plugin-minify-simplify/src/index.js#L557

fabienrohrer commented 5 years ago

If I replace this problematic line, without understanding what I do, but it seems more logical regarding to the code, then it's working:

// path.get("body")[0].inList = false;
node.body[0].inList = false;
GrimaceOfDespair commented 5 years ago

This started happening to us since yesterday noon. Still digging into any library dependency update that might cause this.

GrimaceOfDespair commented 5 years ago

Current lead: @babel/parser was released yesterday:

https://www.npmjs.com/package/@babel/parser

image

fabienrohrer commented 5 years ago

This issue definitively started from this release 😁 I tried to downgrade the packages unsuccessfully.

GrimaceOfDespair commented 5 years ago

This issue definitively started from this release I tried to downgrade the packages unsuccessfully.

Same here. Looking into subdependencies now.

Can't find a way to list node packages by release date :/

jackmaw commented 5 years ago

I got this problem after migration from 6.x to 7.x, it works when i use babel-minify, or babel-preset-minify with simplify=false option, also if I rewrite given code:

function foo(bar) {
    if (bar) {
        return "baz";
    }
}

into:

function foo(bar) {
    if (bar) return "baz";
}

or:

function foo(bar) {
    let baz = undefined;
    if (bar) {
        baz = "baz";
   };

  return baz
}

it minifies code without any error.

Also if i added some test into packages/babel-plugin-minify-simplify/__tests__/fixtures/if-conditional-return-5/

actual.js

function foo(bar) {
    if(bar) {
        return 'baz';
    }
}

expected.js

function foo() {
  if (bar) return 'baz';
}

then yarn test it also works.

It's is look's like the problem is not with the plugin itself, but something doesn't work if you transpile it with babel, then you try to minify it.

I'm running out of ideas right now:(

fabienrohrer commented 5 years ago

I'm running out of ideas right now:(

What about my patch mentioned above: https://github.com/babel/minify/issues/967#issuecomment-550329264

fabienrohrer commented 5 years ago

I just applied it in my repo: https://github.com/cyberbotics/webots/pull/1069

feross commented 5 years ago

The fix appears to be in this commit: https://github.com/babel/babel/commit/0f949990c33cc3c2b3dc24632f7639be166756b2 which hasn't made it into a @babel/core release yet.

@hzoo - Can you release a new @babel/core to unbreak babel-minify?

(Also, unrelated to this bug, but the latest version of @babel/core, 7.7.1, doesn't appear to have made it onto npm)

GrimaceOfDespair commented 5 years ago

Looks like we got a fix!

image

fabienrohrer commented 5 years ago

wow that's great! I'm checking it right now.

fabienrohrer commented 5 years ago

I confirm this is working :-)

jackmaw commented 5 years ago

All works, after update to babel-core 7.7.2. Thank you very much for fast fix, you're the best! Since that i'm closing this issue.