Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why isn't rollup-plugin-node-resolve a builtin feature? #1555

Closed
jorgebucaran opened this issue Aug 16, 2017 · 5 comments
Closed

Why isn't rollup-plugin-node-resolve a builtin feature? #1555

jorgebucaran opened this issue Aug 16, 2017 · 5 comments

Comments

@jorgebucaran
Copy link
Contributor

@jorgebucaran jorgebucaran commented Aug 16, 2017

Hi. I am just curious, what was the reason behind not making rollup-plugin-node-resolve functionality built-in?

I've always found it confusing that I must use node-resolve to bundle my app with a module also bundled with rollup.

@Rich-Harris
Copy link
Contributor

@Rich-Harris Rich-Harris commented Aug 16, 2017

Two reasons — one philosophical, one practical, neither set in stone:

Philosophically, it's because Rollup is essentially a polyfill of sorts for native module loaders in both Node and browsers. In a browser, import foo from 'foo' won't work, because browsers don't use Node's resolution algorithm. So if Rollup were to automatically bundle stuff it found lying around in node_modules, in a sense it would be slightly misleading. The point at which you add Rollup plugins is the point at which you're diverging from universally agreed-upon behaviour, and it seems appropriate that that's an explicit choice.

(This isn't just about 'bare imports' like the foo example, but also the way Node will resolve ./bar to ./bar/index.js and ./bar/index.json — frankly, a terrible decision that I don't want tainting Rollup's codebase! — and things like that.)

On a practical level, it's just much easier to develop software if these concerns are neatly separated out with a good API. Rollup's core is quite large, and everything that stops it getting larger is a good thing. Meanwhile, it's easier to fix bugs and add features (such as .mjs resolution) in node-resolve, which in theory widens the pool of potential contributors.

There's also a 'where does it lead?' argument — if Rollup supports node resolution by default, should it also convert CommonJS by default (even though some CommonJS modules can't be translated to ESM), or shim the process object, or supply built-in modules like url etc etc? There is an argument for saying 'yes' to those questions, but in my experience it's much, much better if the developer is in control over those decisions. Not to pick on Browserify, but I've seen so many bundles that include a couple of hundred lines of process shim just because the source code innocently included a line like this...

if (typeof process === 'undefined') {
  // some code happens
}

...that I consider that kind of implicit behaviour to be a huge footgun. Rollup's goal is to help you make the leanest, fastest-starting bundle possible, and we can't do that while second-guessing your intentions.

Hope this clarifies!

@jorgebucaran
Copy link
Contributor Author

@jorgebucaran jorgebucaran commented Aug 17, 2017

It does and thanks! Maybe some of these words will make their way to the FAQ? 🤔😉

Cheers!

@xtuc
Copy link

@xtuc xtuc commented Aug 17, 2017

I propose to add a note to the README: #1559

@JerryWiltz
Copy link

@JerryWiltz JerryWiltz commented May 8, 2018

For these 2 rollup.config.js files, I can't tell which one ( resolve or node ) to use when:

import resolve from 'rollup-plugin-node-resolve';
export default { ....
plugins: [ resolve() ]
};

OR

import node solve from 'rollup-plugin-node-resolve';
export default { ....
plugins: [ node() ]
};

@TrySound
Copy link
Member

@TrySound TrySound commented May 8, 2018

@JerryWiltz This one

import nodeResolve from 'rollup-plugin-node-resolve';
export default { ....
  plugins: [ nodeResolve() ]
};
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.