schmod / babel-plugin-angularjs-annotate

Add Angular 1.x dependency injection annotations to ES6 code
http://schmod.github.io/babel-plugin-angularjs-annotate
MIT License
241 stars 26 forks source link

Conflict with `transform-es2015-function-name` #15

Open stephank opened 7 years ago

stephank commented 7 years ago

Run the following:

echo 'x => ({ x: () => x });' | babel --plugins transform-es2015-function-name

Expected output is the unchanged input:

x => ({ x: () => x });

But instead we get the following:

_x => ({ x: () => _x });

In a real-world project, we use webpack with babel-loader in a config like:

loaders: [
    {
        test: /\.js$/,
        exclude: /node_modules/,
        loader: 'babel',
        query: {
            presets: ['es2015'],
            plugins: ['angularjs-annotate'],
            cacheDirectory: ''
        }
    },
    /* ... */
]

Where the es2015 preset contains transform-es2015-function-name.

But this results in some of our controllers not being able to find certain dependencies.

Not sure if this is an upstream bug, or if this should be dealt with here. The generated code is otherwise valid, and I guess upstream could argue that nothing is wrong here.

This is on babel 6.18.0 and babel-plugin-transform-es2015-function-name 6.9.0. Our app, where we get the missing dependency errors, uses babel-plugin-angularjs-annotate 0.6.0.

Some more scenario's:

// BAD: replace arrow function with regular function
function foo(x) { return { x: () => x }; };
// function foo(_x) {
//  return { x: () => _x };
// }

// GOOD: replace `x` return with something else
x => ({ x: () => 7 });
// x => ({ x: () => 7 });

// GOOD: replace property name with something else
x => ({ z: () => x });
// x => ({ z: () => x });

// GOOD: use method syntax
x => ({ x() { return x; } });
// x => ({ x() {
//     return x;
//   } });
schmod commented 7 years ago

@stephank Go ahead and open a ticket upstream with Babel. This indeed looks like a bug with transform-es2015-function-name.

Our unit tests already have another (different) conflict with this transform, and I disable it in most cases by creating a modified es2015 preset that does not include that transformation.

Obviously, I do not expect users to need to jump through this hoop in real-world scenarios, so I'm considering this a bug.

stephank commented 7 years ago

They didn't outright close it, but did sort of bounce the issue back here: https://github.com/babel/babel/issues/4782

For what it's worth, for us specifically, this is rather low prio. It's fairly easy to work around. :)

schmod commented 7 years ago

Thanks for taking the time to open the ticket. I'll follow up with the Babel folks.

trisys3 commented 7 years ago

I think I am having a similar problem. For me, it looks like a combination of transform-es2015-shorthand-properties and transform-es2015-function-name is causing a situation where if I have the same name for the shorthand function and one of the function parameters, babel adds an underscore to the parameter. This comes up when I need the dependency and the service/controller to have the same name, mostly in UI Router resolves where nested states need to override the parent's resolve or general resolves can use a global service with the same name

The shortest code I could come up with to demonstrate was:

echo 'y = {x(x) {}}' | babel --plugins transform-es2015-shorthand-properties,transform-es2015-parameters

Output:

y = {
    x: function x(_x) {}
};

If I remove either shorthand-properties or function-name, the problem goes away. With just shorthand-properties, I get

y = {
    x: function (x) {}
};

whereas with just function-name I get

y = { x(x) {} };

Investigating the issue drove me to this plugin, which I am thinking about using. The stability is slightly worrying (before 1.0) but I have used less stable modules.

I will also try @shmod's suggestion and blacklist function-name, as I have never needed it and do not foresee a case where I will.