Tree-shaking of D3 broken since 0.50.

[mbostock]

Reproduction here:

https://bl.ocks.org/mbostock/40e26f283013422491ca475e2513b08c

The crash occurs on this line in transition.style:

https://github.com/d3/d3-transition/blob/d894452ffd3ff69effe1eb1fdfcb5fd9236ba8f3/src/transition/style.js#L12

It’s trying to reference d3.style imported from d3-selection, but for some reason the import is ignored. Maybe it’s because d3-selection is also imported by the entry index.js, but d3.style is only used internally by d3-transition? I’ll try to reduce this to a smaller test case.

[mbostock]

Looking a little closer at the generated code, it appears to have something to do with the use of the comma operator:

var value0 = styleValue(this, name),
    value1 = value(this);
if (value1 == null) value1 = (this.style.removeProperty(name), style(this, name));

Notice that the first reference to style is properly renamed to styleValue (the name Rollup chose for the d3.style import from d3.selection), but the second style is not.

[lukastaegert]

Thanks for providing a repo and digging into this, this is very helpful!
I'll try to address this during the next days.

Just a question: Is this happening for 0.50.0 as well as 0.50.1 or just for the latter? There have been some changes to sequence (i.e. comma) expression handling in 0.50.1.

[mbostock]

In my testing it appeared broken in 0.50.0, 0.50.1 and 0.51.0. I will investigate further today as I have time. Thanks for looking into this and the prompt response! (Also, very excited that you are working on tree-shaking improvements. Thank you!)

[nathancahill]

I think this is the same issue I found in #1709. Bisecting lead me to 0.50.1, and I believe it has to do with variable names not being tracked correctly in SequenceExpressions. I have a minimal Rollup repl that shows this problem in that issue.

[nathancahill]

Here's another example of the issue when including Redux-Immutable:

var _utilities = require('./utilities');

(0, _utilities.validateNextState)(nextDomainState, reducerName, action);

_utilities gets shaken as an unused import. I suspect this issue affects a lot of code generated by Babel, since Babel uses (0, required.func)() to avoid passing required as the this context to func. Again, fine in 0.50.0, but breaking in 0.50.1.