Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Anonymous functions sometimes given conflicting names, breaking code. #829

Closed
Cyp opened this issue May 2, 2018 · 6 comments
Closed

Anonymous functions sometimes given conflicting names, breaking code. #829

Cyp opened this issue May 2, 2018 · 6 comments
Labels
babel This is an issue in the upstream project - Babel bug Confirmed bug with es2015 The bug is caused when used with es2015-block-scoping plugin
Milestone

Comments

@Cyp
Copy link

Cyp commented May 2, 2018

Input Code

index.js:

#!/usr/bin/node
'use strict';

const babel = require('babel-core');

console.log(babel.transform(`
	'use strict';
	(() => {
		const con = console;
		const obj = {
			a() {
				con.log('Hi.');
			}
		};
		obj.a.log = str => {
			con.log('OUCH, should never get here! Should be "' + str + '".');
		};
		obj.a();
	})();
`, {presets: ['minify', 'env']}).code);

package.json:

{
  "name": "babelbug",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "author": "",
  "license": "",
  "private": true,
  "dependencies": {
    "babel-core": "^6.26.3",
    "babel-preset-env": "^1.6.1",
    "babel-preset-minify": "^0.4.0"
  }
}

Actual Output

'use strict';(function(){var a=console,b={a:function a(){a.log('Hi.')}};b.a.log=function(b){
a.log('OUCH, should never get here! Should be "'+b+'".')},b.a()})();

Prints 'OUCH, should never get here! Should be "Hi.".'.

Expected Output

'use strict';(function(){var a=console,b={a:function(){a.log('Hi.')}};b.a.log=function(b){
a.log('OUCH, should never get here! Should be "'+b+'".')},b.a()})();

Prints 'Hi.'.

Details

Note the incorrect function a() instead of function(), leading to the con variable being clobbered.

Same thing if using babel-preset-es2015 with babel-preset-minify.

Can't make a direct link to reproduce online – not sure why. But following this link, clicking on 'Plugins', on 'Only official plugins', typing 'mangle' and selecting minify-mangle-names reproduces it:
https://babeljs.io/repl/#?babili=false&browsers=&build=&builtIns=false&code_lz=JAcgrgzgpgBBAuAnAlgY3iA3AKGACjwEoYBeAPhgG9dhUB7AOwRnodKoBs6BzALhgAepCgEMOURPDwDCAXxzBajZnQBGAK3bVFwEUSo1FrAHRdueEAAlkxkIQWLZNeTTXrjI0z3YJEwgzomZhYA8gCqAMKWADRwABZ0YBwAJjAMUABuEjDcUPAwcRJQAIQwAMoJSamqsABEIDAA1HBITTAgtbb2zg5uHkQKsoQD2EA&debug=false&forceAllTransforms=false&shippedProposals=false&circleciRepo=&evaluate=true&fileSize=false&lineWrap=false&presets=es2015&prettier=false&targets=&version=6.26.0&envVersion=

Not sure whether this is a minify bug or a babel bug. Can't reproduce if running 'env' on the output of 'minify' or when running 'minify' on the output of 'env', only if doing both in the same step.

@j-f1
Copy link
Contributor

j-f1 commented May 2, 2018

This looks like a Babel bug, since a similar version of your code works fine with native ES6 support:

screenshot 2018-05-02 at 11 16 08

Note that last line — if we did { a: function () {} }, a.name would be "".

@Cyp
Copy link
Author

Cyp commented May 3, 2018

Somehow, when doing { a: function () {} }, a.name is still "a", at least in chromium and node. Seems doing anything more complicated than just putting brackets around the function makes it drop the .name, though.

functiontest

I don't think the minifier normally tries to preserve function names, since functions get renamed when minifying. Although there does seem to be some sort of weird (failed) attempt at preserving .name here:

http://babeljs.io/repl/#?babili=true&browsers=&build=&builtIns=false&code_lz=MYewdgzgLgBCBGArGBeGBvAUDGBDANgKYBOUAFAJQbY55GlkAGAEgJYA0MAkgOQC2MYAXyswAcxgAiACToCJKADowuPoQC-kmADNiIATPQJEi-aWWqNkxYwoBuGuszqHx0_XL3MQA&debug=false&forceAllTransforms=false&shippedProposals=false&circleciRepo=&evaluate=true&fileSize=false&lineWrap=false&presets=es2015%2Clatest%2Cbabili&prettier=true&targets=&version=6.26.0&envVersion=

const obj = {
  alert() {
    alert(`Hi, I'm calling "${alert.name}" from "${obj.alert.name}".`);
  }
};
obj.alert();  // Hi, I'm calling "alert" from "alert".

becomes

"use strict";
var obj = {
  alert: (function(a) {
    function b() {
      return a.apply(this, arguments);
    }
    return (
      (b.toString = function() {
        return a.toString();
      }),
      b
    );
  })(function() {
    alert(
      "Hi, I'm calling \"" + alert.name + '" from "' + obj.alert.name + '".'
    );
  })
};
obj.alert();  // Hi, I'm calling "alert" from "b".

when minified.

@boopathi
Copy link
Member

boopathi commented May 3, 2018

Names are sometimes automatically assigned - for example -

var foo = function () {} 
foo.name // foo

@j-f1
Copy link
Contributor

j-f1 commented May 3, 2018

@boopathi but they can’t be used inside the function’s scope to call it recursively.

@boopathi boopathi added bug Confirmed bug with es2015 The bug is caused when used with es2015-block-scoping plugin labels May 3, 2018
@boopathi
Copy link
Member

boopathi commented May 3, 2018

This is a bug for sure.

Minimal repro -

function foo() {
  var con = console;
  return {
    a() {
      con.log("foo");
    }
  };
}

This happens when used along with the function name transformer (in preset-env).

@boopathi boopathi added this to the 1.0 milestone May 13, 2018
@boopathi
Copy link
Member

boopathi commented May 14, 2018

The t.NOT_LOCAL_BINDING symbol used in plugin-function-name (https://github.com/babel/babel/blob/bd98041321c60737ddcacd1ad7e056ef6de31879/packages/babel-helper-function-name/src/index.js#L206) is causing this issue.

This was added in babel/babel#3298

The problem is the binding identifier is NOT registered (skipped) during crawl and the mangler finds the first safe identifier which ultimately collides with this name during runtime.

@vigneshshanmugam vigneshshanmugam added the babel This is an issue in the upstream project - Babel label May 14, 2018
boopathi added a commit that referenced this issue May 14, 2018
The t.NOT_LOCAL_BINDING symbol used in plugin-function-name
(https://github.com/babel/babel/blob/bd98041321c60737ddcacd1ad7e056ef6de31879/packages/babel-helper-function-name/src/index.js#L206)
is causing this issue.

This was added in babel/babel#3298

The problem is the binding identifier is NOT registered (skipped) during
crawl and the mangler finds the first safe identifier which ultimately
collides with this name during runtime.

So, we register the binding and don't respect the t.NOT_LOCAL_BINDING.

Fix #829
boopathi added a commit that referenced this issue May 14, 2018
* fix(mangle): handle inferred names for functions

The t.NOT_LOCAL_BINDING symbol used in plugin-function-name
(https://github.com/babel/babel/blob/bd98041321c60737ddcacd1ad7e056ef6de31879/packages/babel-helper-function-name/src/index.js#L206)
is causing this issue.

This was added in babel/babel#3298

The problem is the binding identifier is NOT registered (skipped) during
crawl and the mangler finds the first safe identifier which ultimately
collides with this name during runtime.

So, we register the binding and don't respect the t.NOT_LOCAL_BINDING.

Fix #829

* fix lint errors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
babel This is an issue in the upstream project - Babel bug Confirmed bug with es2015 The bug is caused when used with es2015-block-scoping plugin
Projects
None yet
Development

No branches or pull requests

4 participants