Rollup's `external` option only sufficient to suppress `import`, `skip` additionally required to suppress `require`

[wearhere]

UPDATE 07.13.2017: This issue is written as if we still had the skip option from version 2.x. As this comment notes, 3.x does not fix the problem with this module and external but rather removes the workaround of specifying skip. :(

---

As per @Rich-Harris's comment here, this plugin will automatically skip modules marked as external (without the user having to specify the skip option). But, my testing reveals that the plugin will only do this if the module is imported using an import statement, not via a require statement—in the latter case, you must also specify skip.

Example:

// index.js
import {getFirstName} from './StringUtils.js';
console.log(getFirstName('Jeff Wear'));

// StringUtils.js
var _ = require('underscore');

module.exports.getFirstName = function(name) {
  // Slightly contrived I know
  return _.first(name.split(' '));
};

// package.json
{
  "devDependencies": {
    "rollup": "^0.45.2",
    "rollup-plugin-commonjs": "^8.0.2",
    "rollup-plugin-node-resolve": "^3.0.0"
  },
  "dependencies": {
    "underscore": "^1.8.3"
  }
}

The following Rollup configuration will not prevent Underscore from being bundled:

export default {
  entry: 'index.js',
  external: ['underscore'],
  plugins: [
    nodeResolve(),
    commonJS(),
  ],
  targets: [{
    format: 'iife',
    dest: 'bundle.js'
  }]
};

—I must also specify nodeResolve({skip: ['underscore']}). (skip alone is not sufficient—if I don't define external, Rollup prints a "Treating 'underscore' as external dependency" warning.)

However, if I make StringUtils an ES6 module:

// StringUtils.js
import _ from 'underscore';

export function getFirstName(name) {
  return _.first(name.split(' '));
}

Then external: ['underscore'] is sufficient.

Is this expected? It would be nice if external was sufficient for both ES6 and CommonJS modules.

It's also kind of weird that I need both skip and external in the CommonJS case. EDIT: Upon re-reading this comment I take this back since even if we get rollup-plugin-node-resolve to skip resolution we might not want to treat the dependency as totally external but rather want Rollup to resolve it some other way.

[unional]

Just external does not do it for named import either:

import { set } from 'lodash'
export { set }

bundles lodash if without skip

[fregante]

I assume this was solved by #90. If not, leave a comment

[jochenberger]

I think that #90 did not solve the issue but rather removed the workaround, because it is no longer possible to specify skip in addition to external.

[wearhere]

Whoever fixes this, please note the following quirk in the previous behavior of skip (a bug that the new implementation should avoid!)

Consider this setup using rollup-plugin-node-resolve@2.x:

// index.js
import {doSomethingWithHandlebars} from './StringUtils.js';
console.log(doSomethingWithHandlebars());

// StringUtils.js
var Handlebars = require('handlebars/runtime');
module.exports.doSomethingWithHandlebars = function() {
  Handlebars.madeUpFunction();
};

// package.json
{
  "dependencies": {
    "rollup": "^0.45.2",
    "rollup-plugin-commonjs": "^8.0.2",
    "rollup-plugin-node-resolve": "^2.0.0"
  },
  "dependencies": {
    "handlebars": "^4.0.10",
  }
}

// rollup.config.js
import nodeResolve from 'rollup-plugin-node-resolve';
import commonJS from 'rollup-plugin-commonjs';

export default {
  entry: 'index.js',
  external: ['handlebars/runtime'],
  plugins: [
    nodeResolve({
      skip: ['handlebars/runtime']
    }),
    commonJS(),
  ],
  targets: [{
    format: 'iife',
    dest: 'bundle.js'
  }]
};

The above skip list will not exclude Handlebars from the bundle!

Instead you have to do skip: ['handlebars']. That is, it appears that skip's values must be the canonical package names even if you're require'ing a subpath.

But hopefully the fix to this issue will allow external: ['handlebars/runtime'] to be sufficient.

[DanielJoyce]

External doesn't work at all for me. It always pulls in a relative path if it is found in node_modules.

[lili21]

any updates ?

[lili21]

3.x does not fix the problem with this module and external but rather removes the workaround of specifying skip. :(

what happened ?

[dima-takoy-zz]

any updates?

[dima-takoy-zz]

I checked it with latest versions, all works fine, with commonjs module and with es6 module. Repro here: https://github.com/mecurc/72-rollup-resolve

Can be closed now.