output contains asyncGenerator(), but I never use it.

[musicode]

my rollup version is 0.36.3, my config is as below:

import babel from 'rollup-plugin-babel'
import resolve from 'rollup-plugin-node-resolve'
import commonjs from 'rollup-plugin-commonjs'
import uglify from 'rollup-plugin-uglify';

var minify = process.env.NODE_ENV === 'release'

export default {
  entry: 'src/index.js',
  format: 'umd',
  moduleName: 'index',
  plugins: [
    babel({
      presets: [ 'es2015-rollup' ],
      babelrc: false,
      comments: false,
      runtimeHelpers: true
    }),
    resolve({
      jsnext: true,
      main: true,
      browser: true,
    }),
    commonjs(),
    (minify && uglify()),
  ],
  dest: minify ? 'dist/index.min.js' : 'dist/index.js'
}

[musicode]

There are 3 features never used.

var _typeof = typeof Symbol === "function" && typeof Symbol.iterator === "symbol" ? function (obj) {
  return typeof obj;
} : function (obj) {
  return obj && typeof Symbol === "function" && obj.constructor === Symbol && obj !== Symbol.prototype ? "symbol" : typeof obj;
};

var asyncGenerator = function () {
  function AwaitValue(value) {
    this.value = value;
  }

  function AsyncGenerator(gen) {
    var front, back;

    function send(key, arg) {
      return new Promise(function (resolve, reject) {
        var request = {
          key: key,
          arg: arg,
          resolve: resolve,
          reject: reject,
          next: null
        };

        if (back) {
          back = back.next = request;
        } else {
          front = back = request;
          resume(key, arg);
        }
      });
    }

    function resume(key, arg) {
      try {
        var result = gen[key](arg);
        var value = result.value;

        if (value instanceof AwaitValue) {
          Promise.resolve(value.value).then(function (arg) {
            resume("next", arg);
          }, function (arg) {
            resume("throw", arg);
          });
        } else {
          settle(result.done ? "return" : "normal", result.value);
        }
      } catch (err) {
        settle("throw", err);
      }
    }

    function settle(type, value) {
      switch (type) {
        case "return":
          front.resolve({
            value: value,
            done: true
          });
          break;

        case "throw":
          front.reject(value);
          break;

        default:
          front.resolve({
            value: value,
            done: false
          });
          break;
      }

      front = front.next;

      if (front) {
        resume(front.key, front.arg);
      } else {
        back = null;
      }
    }

    this._invoke = send;

    if (typeof gen.return !== "function") {
      this.return = undefined;
    }
  }

  if (typeof Symbol === "function" && Symbol.asyncIterator) {
    AsyncGenerator.prototype[Symbol.asyncIterator] = function () {
      return this;
    };
  }

  AsyncGenerator.prototype.next = function (arg) {
    return this._invoke("next", arg);
  };

  AsyncGenerator.prototype.throw = function (arg) {
    return this._invoke("throw", arg);
  };

  AsyncGenerator.prototype.return = function (arg) {
    return this._invoke("return", arg);
  };

  return {
    wrap: function (fn) {
      return function () {
        return new AsyncGenerator(fn.apply(this, arguments));
      };
    },
    await: function (value) {
      return new AwaitValue(value);
    }
  };
}();

var get$1 = function get$1(object, property, receiver) {
  if (object === null) object = Function.prototype;
  var desc = Object.getOwnPropertyDescriptor(object, property);

  if (desc === undefined) {
    var parent = Object.getPrototypeOf(object);

    if (parent === null) {
      return undefined;
    } else {
      return get$1(parent, property, receiver);
    }
  } else if ("value" in desc) {
    return desc.value;
  } else {
    var getter = desc.get;

    if (getter === undefined) {
      return undefined;
    }

    return getter.call(receiver);
  }
};

[musicode]

var set$1 = function set$1(object, property, value, receiver) {
  var desc = Object.getOwnPropertyDescriptor(object, property);

  if (desc === undefined) {
    var parent = Object.getPrototypeOf(object);

    if (parent !== null) {
      set$1(parent, property, value, receiver);
    }
  } else if ("value" in desc && desc.writable) {
    desc.value = value;
  } else {
    var setter = desc.set;

    if (setter !== undefined) {
      setter.call(receiver, value);
    }
  }

  return value;
};

[panzerdp]

I have exactly the same problem.
The build adds the presented unused code.

[kennetpostigo]

Set externalHelpers to true in the configuration to remove this.

[hollowdoor]

@kennetpostigo If I use the externalHelpers option I get this error ReferenceError: babelHelpers is not defined if I run a test build in the browser.

Edit: If I do the opposite (externalHelpers=false) then it works.

[kennetpostigo]

@hollowdoor did you turn this on in the rollup.config.js? It's meant to go in there. Also are you using es2015-rollup?

[hollowdoor]

Oh no wait I was wrong. If I change it to false it inserts the asyncGenerator again. What's weird about this is that it works for a test build, but not for a sub-module. rollup keeps throwing The keyword 'await' is reserved .

@kennetpostigo I don't have a rollup.config.js file. I use rollup as a module. And I turned it on/off in there. If I turn externalHelpers to true I get ReferenceError: babelHelpers is not defined. If I turn externalHelpers to false I get the asyncGenerator in the build, and also a The keyword 'await' is reserved error when I try to build in a consuming script.

So the story goes I have a little module I've published that contains the offending asyncGenerator that works when I run isolated tests. It doesn't work if I try to include that module into an iife build for inclusion in an App.

[kennetpostigo]

@hollowdoor gotcha. Flipping it to true was the only quick fix I found for this. I haven't tried using it as a module but can't imagine it being much different. Are you using es2015-rollup? It should include the helpers

[hollowdoor]

Yes I'm using es2015 rollup. The helpers aren't being included for whatever reason.

Also I don't need them for the module. The only babel-helper code put in there is for typeof.

[hollowdoor]

Ok. It kind of sucks, but what I had to do was let the asyncGenerator remain. In my app build I used the allowReserved option on the rollup's acorn option like this acorn: {allowReserved: true}. So no {externalHelpers:true} in my module's config.

Now my build config for the app looks like this:

export default {
  entry: 'src.js',
  format: 'iife',
  acorn: {allowReserved: true},
  plugins: [
      babel(),
      nodeResolve({ jsnext: true, main: true }),
      commonjs()
  ],
  dest: 'main.js',
  moduleName: 'none'
};

Rather strange bug I must say.

Related rollup/rollup#564 but not the same. I hope things keep working because now my setup could be clobbered by reserved words. Also there are now duplicates of asyncGenerator. :(

I am also not using asyncGenerator, nor the other extraneous stuff listed in one of the earlier posts by @musicode.

[Rich-Harris]

Am unable to reproduce this with the current version (2.7.1) – does anyone have a sample repo illustrating the problem? Thanks

[hollowdoor]

I don't know if this will help.

I have a repo with a library https://github.com/hollowdoor/dom_compose

That library is bundled as es2015, and commonjs modules.

I have a consuming app that has this configuration:

import babel from 'rollup-plugin-babel';
import commonjs from 'rollup-plugin-commonjs';
import nodeResolve from 'rollup-plugin-node-resolve';

export default {
  entry: 'src.js',
  format: 'iife',
  acorn: {allowReserved: true},
  plugins: [
      babel(),
      nodeResolve({ jsnext: true, main: true }),
      commonjs()
  ],
  dest: 'main.js',
  moduleName: 'none'
};

The bablrc for the app looks like this:

{
  "presets": ["es2015-rollup"]
}

The output file main.js contains all kinds of extra/duplicate babel helper definitions.

For the app if I set externalHelpers to true it puts the dependencies in the arguments of the iife. If I set externalHelpers to false I get all kinds of helpers I don't use.

For the dom-compose module if externalHelpers is false when I build the app script I get an async is a reserved word error.

Finally if I don't set externalHelpers (default false) in both the module, and app I can set acorn to allow reserved true. But then I get duplicate babel helper definitions.

I've considered just not using es2015 (except the modules) in my module, but I think that would defeat the purpose of compiling a module that's meant to be consumed by browsers. The other thing I'm considering is switching to buble for the module, and continue to use babel for the app. That wouldn't get rid of the unused babel helpers in the app though.

It could be that I just don't know what I'm doing. :)

Or there's some deep bug, or conflict way down in some other module. Whatever it is it's a mystery to me.

I'm using rollup 0.37.0, and rollup-plugin-babel ^2.6.1.

[Rich-Harris]

@hollowdoor thanks. Are you getting the same results with the latest versions of rollup-plugin-babel and babel-preset-es2015-rollup? It seems to be behaving for me.

The other thing I'm considering is switching to buble for the module, and continue to use babel for the app

I'd recommend that, and not just because I'm biased – the modules will get smaller as a result, and you won't have the weird typeof polyfill appearing multiple times in your app. You would need to use the dangerousForOf and dangerousTaggedTemplateString transforms (as far as I can tell from reading the source code, this is a case where it's legit to use those transforms).

[hollowdoor]

It might be a day, or so before I check the latest babel-preset-es2015-rollup. I've got version 1.2 for babel-preset-es2015-rollup for some reason. Which is weird because I just installed it recently.

I do think I'll switch to buble anyway. For modules it would be better fit. I will need the dangerous transforms for testing, or maybe not. I can test with a browser that supports template literals. As far as compiling the module itself I don't think it needs to know that it's a template tag. But I will need the dangerousForOf.

Before I do switch I'll try testing the newer babel-presets to see if it's working. I'll try to remember to report back here.

[eight04]

rollup/rollup#1595

[TehShrike]

I have a reproduction in the modernization branch of abstract-state-router.

Using the latest versions of babel-core, babel-plugin-external-helpers, babel-preset-es2015, rollup, and rollup-plugin-babel, my output (bundle.js) contains a big chunk of asyncGenerator code that should not be necessary or included at all.

[Andarist]

Just for anyone interested, this can be 'fixed' for now in the userland with:

import babel from 'rollup-plugin-babel'
import { list as babelHelpersList } from 'babel-helpers'

// ...

  babel({
    plugins: [
      'external-helpers',
    ],
    // fixing temporary rollup's regression, remove when rollup/rollup#1595 gets solved
    externalHelpersWhitelist: babelHelpersList.filter(helperName => helperName !== 'asyncGenerator'),
  }),

[lukastaegert]

Btw. a proper “fix” is nearly complete, should arrive some time next week. Just doing some final touches now.

[Andarist]

@lukastaegert is there any open PR on this? would love to see the code and heuristics to determine that this can be tree-shaken. It is quite a non-trivial case

[lukastaegert]

Not a PR yet but it’s all at https://github.com/lukastaegert/rollup/tree/simple-object-shape-tracking
It can already tree-shake babel-helpers and quite a few more complex cases. However, I want to included basic handling for getters first before I make this an official PR. I would love if someone took a look in case I overlooked something important.

[Andarist]

Would have to spend more time analyzing whats going there, its not the shortest PR made ;) I've looked mainly on the tests and I see some dope job done 👏 Once you create a PR, feel free to ping me I'll try to do more thorough review - but some more insight on the nodes visitors would be good to have, I haven't dived into those parts of the rollup's codebase before.

I must say that I'm really impressed by the changes you have done to rollup's algorithm and the scope of your PRs - it's always amazing to see a PR when somebody just pops up (I believe you have started contributing to rollup recently, and you have started with a huge rewrite, correct me if im wrong) and do such big changes to the codebases. It reminds me of @jimbolla's PR to react-redux, also a great rewrite 😃

What is missing at the moment for the community is a guide/article on how to write a tree-shakeable code. If you are willing to have a chat some day about this, I might write up something - always wanted to, just had to find time to dive deeper into tree-shaking algorithms and I believe you are one of the most informed people at the moment :)

Ofc while a guide might be helpful for the start, it should be later incorporated (after splitting it appropriately) as docs for your eslint plugin. Also about the plugin - I conclude from the Readme that it is reimplementing tree-shaking algorithm, would it be possible to extract tree-shaking algo into a separate library which could be used both by rollup and the plugin (and possibly by other projects)?

Also I've created a babel plugin for automatic #__PURE__ annotation insertion - would love to hear your feedback if that thing makes any sense to you or am I just a little bit delusional 😉

[lukastaegert]

Thanks for the kind words 😃. About the plugin, the short version of the story goes like this (which is basically the story how I came to be a rollup contributor):

• Promised to hold a talk about rollup at a local user group meetup
• When playing around with rollup, was shocked at how easily seemingly random code changes could break tree-shaking
• Thought that it would really help to have a plugin that highlights code that breaks the algorithm
• Dove into rollup's code. Found it to be somewhat convoluted in places and not at all generic. Decided not to copy rollup's code but write my own generic implementation + automated tests against rollup
• Had to add a lot of exceptions to get the highlighting right
• Thought: Maybe I should just change rollup to work more like my plugin…

So, after all, the plugin had the new algorithm first. But due to how ESLint works as compared to rollup, I could not just copy the code back. But yes, the ultimate goal must be to extract the algorithm and use it in the plugin. With the recent advances of rollup's algorithm, it is getting harder and harder for the plugin to keep up.

The main obstacle here is that rollup's algorithm relies on adding properties to AST nodes while with ESLint, this is strictly forbidden. But then again, the plugin does not need the whole algorithm but just the "hasEffects*" functions attached to the AST nodes. Time will find a solution here. My feeling is that we should first find a way for rollup plugins to add to/modify the AST visitors which would enable things like proper JSX plugins or plugins that make tree-shaking selectively more or less strict.

I am very aware that what is missing most is good guides for the community. Also, my plugin may seem very technical to the common user. In a future version, all errors could have much more user-oriented messages and wiki links attached educating the user about good and bad patterns. But it's a lot of work and I do not want to spread my resources too thinly. For now, one thing I keep stumbling about is that when doing an ES6 library with named exports you should NEVER add them again as properties of a default export. You can try it out with loadesh-es. In its current form, basically not tree-shaking will take place. However, if you go into your node_modules and remove just the default export, the imported size will instantly drop to half. Much more will be possible in the future once function return value handling is implemented. Maybe at some point I will open an issue about this.

I think your plugin can be very useful but its universal nature is dangerous. If there is a way to limit the plugin to individual files that you have control over e.g. by putting a comment in the first line (or, does babel have access to file names?), this would make it potentially even more useful because you could just let it run in your main index file. But that's just an idea. Wrongly placed "pure" comments in a library that others depend on can cause funny things. Unless you actually write in a side-effect free way, maybe that's the main takeaway.

[Andarist]

For now, one thing I keep stumbling about is that when doing an ES6 library with named exports you should NEVER add them again as properties of a default export. You can try it out with loadesh-es. In its current form, basically not tree-shaking will take place.

I havent realize that lodash does this 🙈 I guess using babel-plugin-lodash is still the way to go. From what I've heard though lodash@5 is going to have cherry-picked modules exclusively - no default, probably no chaining. If one wants to use it in "the old-style" it will provide a babel plugin so the old-style can be rewritten to cherry-pick modules.

The same as attaching to the default export goes with static properties on classes imho - this too should be avoided when possible. Statics can be easily emulated with modules and that wont prevent tree-shaking (unless ofc one want to pass the class to some function).

I think your plugin can be very useful but its universal nature is dangerous.

I'm very well aware of that, the plugin must be used with caution.

If there is a way to limit the plugin to individual files that you have control over e.g. by putting a comment in the first line (or, does babel have access to file names?), this would make it potentially even more useful because you could just let it run in your main index file.

Babel plugin can have access to the filename, found a way to access it in other plugin. I will add possibility to scope with filenames as a feature request :)

But anyway - this is going a little bit offtopic here, gonna reach out to you on the twitter or something ;)

[mattpilott]

@lukastaegert Any more progress on your potential future PR?

[Andarist]

The PR is here - rollup/rollup#1667

[lukastaegert]

Well, the PR has been submitted: rollup/rollup#1667
Unfortunately even though I have the power to merge it myself I do not have the power to release a new version of rollup and it appears @Rich-Harris is on an extended vacation with limited internet access.

Unfortunately npm installing rollup from a PR will not work because the build step will not be triggered. But if you depend on it or want to try it locally you can

git clone https://github.com/lukastaegert/rollup.git
cd rollup
git checkout simple-object-shape-tracking
npm install
npm run build
npm link

and then run npm link rollup in your other project. That should replace rollup with your local version.

[mattpilott]

Awesome, thanks

[lukastaegert]

To make it even easier I have added a branch that contains all the build artefacts so you don't have to do the local setup.

npm install rollup@github:lukastaegert/rollup#simple-object-shape-tracking-bundled

should work now and can also just be added to your package.json.

[Andarist]

@lukastaegert I've prepared (what a pun!) a PR, if you merge it and rebase ur branch against updated master, it should get even easier ;)

[mattpilott]

Joy of joys!

[lukastaegert]

I‘m currently on the road but I will certainly take a look later that evening! Though my solution of adding the artifacts to a branch certainly works, it is really inflexible and needs to be updated manually. If this works, this will make things much easier for everyone in the future. Talking about babel, isn’t it possible to use a PR as the version in their REPL? Now that would be quite a feat for rollup...

[Andarist]

Talking about babel, isn’t it possible to use a PR as the version in their REPL? Now that would be quite a feat for rollup...

It certainly would be. I think the most important PRs that got this implemented there are:
babel/website#1284
babel/website#1356

[FND]

FWIW, it appears that Rollup v0.51 has fixed this issue - at least according to my test case (cf. faucet-pipeline/faucet-pipeline-js@f4a1667).

[lukastaegert]

Thanks for the info, I forgot about this issue number. There is now a proper test in place that the actual babel helpers are removed completely if unused so I will close this now.