fix(node-resolve): modulePaths default is not set

[bytestream]

Rollup Plugin Name: node-resolve

This PR contains:

• bugfix
• feature
• refactor
• documentation
• other

Are tests included?

• yes (bugfixes and features will not be merged without tests)
• no

Breaking Changes?

• yes (breaking changes will not be merged unless absolutely necessary)
• no

If yes, then include "BREAKING CHANGES:" in the first commit message body, followed by a description of what is breaking.

List any relevant issue numbers:

Description

https://github.com/rollup/plugins/tree/master/packages/node-resolve#modulepaths states that the default modulePaths value is []. This is not true, the default is null. This results in the resolve package scanning additional directories such as /$HOME/.node_modules - browserify/resolve#273

Setting nodeResolve({modulePaths: []}) prevents /$HOME/.node_modules from being scanned.

I suggest either the default in the README is changed, or modulePaths is correctly set to match the README.

[tada5hi]

you got a point there ^^

[shellscape]

We'll need a test for this change before we can merge it. I know it looks small on the surface but this is a breaking change, and we need a test to prevent regressions in the future.

Would also like to get @lukastaegert take on this. There may be an undocumented reason for the existing behavior.

[lukastaegert]

My understanding is that the resolve default behavior matches what Node does (though I did not check), so it is technically correct. If we change this, then this is a breaking change. Which might be fine if we think this is basically never what people want.

[bytestream]

I think it's simplest if the README is updated, to either specify the default is null, or remove the default line altogether. The context for me noticing is something similar to vitejs/vite#6767 - permission denied on docker

[shellscape]

@lukastaegert ping re: convo

[shellscape]

@bytestream please resolve that conflict and we can merge

[shellscape]

@bytestream thanks for raising the fix and for keeping the branch updated. I updated your branch with the proper tests and updated it to upstream. please understand that (nearly) every PR that gets merged has to have a test, including this one. I had some extra time today to add that, but I don't normally, and we don't normally have a lot of maintainers with extra time on this repo (unfortunately, I wish we did). so in a normal cycle, your PR would have sat unmerged until it was eventually marked as abandoned. we'd love to have you contributing more PRs but please do understand that tests are required and essential to keeping a high quality of shipped code for the repo.