How to import with side effects?

[mbostock]

This is more of a question than an issue. Does it make sense that I have to import twice in order to achieve an import with side effects? Consider the following D3 build:

import {
  event,
  mouse,
  namespace,
  namespaces,
  requote,
  select,
  selectAll,
  selection,
  touch,
  touches
} from "d3-selection";

import {
  ease,
  transition
} from "d3-transition";

import "d3-transition";

export default {
  get event() { return event; },
  mouse: mouse,
  namespace: namespace,
  namespaces: namespaces,
  requote: requote,
  select: select,
  selectAll: selectAll,
  selection: selection,
  touch: touch,
  touches: touches,
  ease: ease,
  transition: transition
};

The first import of d3-transition exposes ease and transition. But the second is also needed to set selection.prototype.transition.

It makes sense that Rollup would ignore the side-effects by default, so that I can explicitly chose to include that code (or not). But I wanted to double-check that this seems right to you.

[mbostock]

Heh. So this is true of any code that has side-effects, which leads to some interesting constraints on development. For example, consider the following:

var map = (new Map)
    .set("foo", 42)
    .set("bar", 60);

map
    .set("baz", 50);

export default function(key) {
  return map.get(key);
};

If you import the default export, the code that defines the “baz” key never runs—it’s dropped by Rollup.

[Rich-Harris]

As a starting point, to cover the majority of cases, I went with a simple heuristic - if an import declaration is empty (like import 'polyfills'), Rollup assumes the module is being imported for its side-effects and includes all its statements - and left the trickier cases as a bridge to be crossed later.

One option would be to indicate modules with side-effects with a comment, like

/*rollup sideEffects: true */
import { ease, transition } from 'd3-transition';

I'm not wild about that, partly because it's Rollup specific but also because I don't think it's a module author's job to ensure that it does the right thing. I'm not sure what a reliable alternative looks like at the moment though.

For the second example, that's a straightforward bug which I think is easily solved. At the moment, given a situation like this...

var obj = {};
mutate( obj );
otherObj.mutate( obj );

export { obj };

...the second and third lines are assumed to mutate obj, and so are included after obj if it ends up in the bundle (tests here and here). (At some point I'd like it if Rollup could analyse the mutate function and decide whether it's capable of mutating obj, rather than including it just in case, but that's another story). Just need to add some similar logic that assumes that a method call mutates the thing that it's a method of. Marking this as a bug.

[Rich-Harris]

In 0.7.1, the map.set("baz", 50) call will be included, as method calls are assumed to mutate their owners. (This does of course mean that map.get('baz') would also be included, potentially unnecessarily, but that's definitely the lesser evil.)

Still don't have a great way to import for side-effects without duplicate import statements, so will leave this open for now

[mbostock]

👍 for the 0.7.1 fix.

I’m happy with the simple import for importing with side-effects, now that I understand how it works. Seems like the confusion could be solved with documentation.

[Rich-Harris]

Closing this issue as it was resolved a while back – Rollup now takes a more cautious approach, checking all imported modules for side-effects. In some cases that means including more code than is necessary, so #179 suggests ways we can prevent those false positives.