Plugins: Add named exports for all plugins

Created on 1 May 2020  ·  19Comments  ·  Source: rollup/plugins

  • Rollup Plugin Name: all plugins except for node-resolve and babel

Feature Use Case

Using the default export creates issues with CommonJS if we want to export more than one item. When using @rollup/plugin-babel, the different import styles look like:

import babel from '@rollup/plugin-babel';
const babel = require('@rollup/plugin-babel').default;

In contrast, the named exports look more similar:

import { babel } from '@rollup/plugin-babel';
const { babel } = require('@rollup/plugin-babel');

Note: both formats work today because we export the plugin under babel and default.

Feature Proposal

As soon as we add any additional exports to a module, as was the case for @rollup/plugin-node-resolve, we need to use named exports. I think we should switch to named exports for all plugins for consistency. We can continue to export default as well for the ES module import format, but CJS outputs would have breaking changes.

c¹ ⋅ discussion x⁷ ⋅ stale

Most helpful comment

If that's the case you might consider revising the issue title to reflect that. "Switch to" implies dropping the existing export mechanism and replacing it with a new one. "Add" would be a more apt description.

All 19 comments

I am in favor named exports, they create less friction and allow future additions.

I'd recommend doing this one PR at a time. While tedious, easier to manage the releases and scheduling.

Hooray!

I thought this would make at least one person happy. And I see the need for babel. But I am not happy doing this unforced for the other plugins

  1. It will be a breaking change for each plugin
  2. Command line plugin usage will no longer work for many plugins

To explain (2):
At the moment, you can

rollup -i input.js -f es -p @rollup/plugin-node-resolve

and

rollup -i input.js -f es -p node-resolve

Now the problem is for Rollup to find the relevant export. So it requires the plugin and then

  1. if it is an object, use the plugin name as a property and call it (this needed to be added to make terser work…)
  2. otherwise call the plugin itself (which only works for default exports)

For 1., at the moment it would try plugin["@rollup/plugin-node-resolve"] in the first case and plugin["node-resolve"] in the second case. None of this would work.

So if you do this, we need to adjust the logic in Rollup first. PR very much welcome, the logic is very localized in https://github.com/rollup/rollup/blob/master/cli/run/commandPlugins.ts#L66-L73

And then once this is done, Rollup's documentation should be updated quickly, otherwise I see a ton of issues rolling in.

Babel will not work with cli anyway. We can introduce some other solution like special export

export const plugin = () => ({ name: 'plugin' })
export const specialExportWithPluginForCli = plugin

Why would Babel not work?

And technically, there should be no reason to have a separate plugin for CLI. CLI even supports plugin parameters. One easy approach could be to keep the default exports as well as the named export and have Rollup CLI check the default property as well as the entire export.

I'd like to keep default exports anyways to reduce the level of breakage. I'll put up a PR for Rollup so it checks for default exports.

We can phase this change in slowly and even wait on some PRs until we have other breaking changes to include.

So we've got two really valid use cases here.

  1. We need the ability to export, or provide to the user, more than just the default export.
  2. We need the default export to support things like loading via CLI

There is a pattern that solves both, even though it's not a _purist_ pattern. In the case of node-resolve where we wanted to export the default options, we could have gone a different route:

const defaultOptions = {};
const plugin = () => {};

plugin.defaults = Object.freeze(Object.assign({}, defaultOptions));

export default plugin;

While it's not as clean as named exports, it works. I've seen it a lot recently. Namely working with PouchDB and Electron. We can also attach types to the default export via namespace so we end up with something like:

export interface MainDatabase {
  config: PouchDB.Database;
  users: PouchDB.Database;

  sync: (uri: string) => void;
}

I think the case for default export for use on the CLI is strong. Users had clamored for that for a long while and just recently got that in. I think we can meet the needs of both use cases without causing too much disruption.

I'm against placing all the exports on the default function for the ES module build. We could do it for the CommonJS build but it would complicate the typings, and I think named exports is more straightforward.

I suppose I'm still hunting for the cost/benefit here. We're talking about a lot of changes and adding additional complexity to the ecosystem by making these changes. If we're going to move forward with named exports on every plugin then there needs to be clear benefit that outweighs the complexity.

What is the advantage of this relatively large change? Export purity seems like a terrible argument for all of this work/churn.

I think @shellscape's should actually work quite well. What we could do is

  • use a default export for the actual plugin
  • use named exports for everything else (we can also export the plugin twice, once as the default and once named if that is what we want)
  • do some magic for the CJS build, e.g. like this:
    js // rollup.config.js export default { input: 'src/index.js', onwarn(warning, warn) { if (warning.code !== 'MIXED_EXPORTS') { warn(warning); } }, output: { format: 'cjs', dir: 'dist', footer: 'module.exports = Object.assign(exports.default, exports);' } }

This would automatically attach all named exports to the default export and use that as the only export. Thus we could have a single build and a somewhat consistent interface when switching from CJS to ESM.

Hopefully this won't land for plugins with single export. After years of using default exports this is a terrible proposal with no real advantages.

Why not switching instead everything to ES6 (and abolish require())? Then CommonJS cannot make problems.

It is in first place about replacing in the documentation require() with import. This shall happen soon or later.

-1 on dropping default export for plugins. If plugin maintainers wish to add an additional redundant named export - that's fine. But breaking existing plugins with default export just for questionable API elegance doesn't make sense.

I'm glad to see that export default was reinstated in node-resolve: https://github.com/rollup/plugins/commit/fe037dbd59b0ee04c2de7b6b48c8cfcc98ccbcb6.

As mentioned above, the rollup CLI --plugin option will have a problem identifying the entry point for plugins with names containing a dash (-) if they lack default export. In commonjs it is legal to have named exports with dashes using the subscript notation, but dashes are not valid in an export symbol in ES6. I see that the camelCase nodeResolve was chosen as the export name for node-resolve, but it just as easily could have been the snake_case node_resolve or even the no-dash noderesolve. This plugin naming confusion is unnecessary with default export.

This issue is not proposing that we drop default exports, but instead export both named and default exports. ES module files will continue to work exactly the same, this only affects CommonJS. With Lukas' proposed solution we can make this a non-breaking change.

If that's the case you might consider revising the issue title to reflect that. "Switch to" implies dropping the existing export mechanism and replacing it with a new one. "Add" would be a more apt description.

Hey folks. This issue hasn't received any traction for 60 days, so we're going to close this for housekeeping. If this is still an ongoing issue, please do consider contributing a Pull Request to resolve it. Further discussion is always welcome even with the issue closed. If anything actionable is posted in the comments, we'll consider reopening it.# Comment to post when closing a stale issue.

Was this page helpful?
0 / 5 - 0 ratings