feat(node-resolve): add ignoreSideEffectsForRoot option

[tjenkinson]

Rollup Plugin Name: resolve

This PR contains:

• bugfix
• feature
• refactor
• documentation
• other

Are tests included?

• yes
• no

Breaking Changes?

• yes
• no

List any relevant issue numbers: #692

Description

Use the rootDir option to determine if the file being resolved is in the root package, and if it is do not respect the package.json side-effects option, providing ignoreSideEffectsForRoot is true.

Note the default is remaining as false and we are not treating this as a bug, because false matches the default behaviour in webpack.

Refs #692

[LarsDenBakker]

I'm not sure if this is a bug, why should side effects setting not apply to the project itself?

[tjenkinson]

Yeh I was wondering this. I think if the package.json option is false you might only expect it to apply to the entry point for users. I’ll have a look and see what webpack does.

[07akioni]

I'm not sure if this is a bug, why should side effects setting not apply to the project itself?

I'm not sure about the behavior too. However I think it should be as least documented, or maybe made as an option?

For example I'm making a non-side-effects library, and the doc site is placed in the project root too. If I use rollup to create a doc site bundle (which vite does), css (for the site) may be shaken by rollup. When I apply sideEffects: false in package.json, I only want the entry point to be shaken.

Currently I need to remove sideEffects field in package.json before I build the doc site and add it after the site is built. Another workaround is to add docs dir in sideEffects.

What makes things confused is that

with plugin-node-resolve	side-effects: false	side-effects-shaken (code outside node_modules)
✅	✅	✅
✅	❌	❌
❌	✅	❌
❌	❌	❌

If you only use node resolve for extension resolving (not using any module from node_modules), you will find bundle result differs with or without plugin-node-resolve when sideEffects is marked as false.

@LarsDenBakker
@tjenkinson

[tjenkinson]

Finally got around to trying this with webpack.

If you

cd packages/node-resolve/test/fixtures/root-package-side-effect
npm i --save-dev webpack@5.11.0 webpack-cli@4.2.0
npx webpack --entry "./index.js"

the output build is empty, so it looks like webpack works the same way the plugin does currently.

So maybe we should keep the plugin consistent with webpack? I agree it could be a bit confusing though if you're not using node-resolve, and then when you add it to an existing project it initially does not behave like a no-op.

[shellscape]

It might be worth comparing to webpack@4, since v5 is still a bit of a trash can fire.

[tjenkinson]

With webpack 4.44.2 it also does not include 'side-effect.js' with "sideEffects": false

[shellscape]

I think webpack has set good precedent here and following their lead is reasonable. I also think documenting this as suggested by @07akioni is a great idea. @LarsDenBakker and @07akioni could you take another look at this one please?

[tjenkinson]

So if we are following webpack we should close this PR and update the docs. @LarsDenBakker just saw you approved. Do you think we should do different to weback?

[LarsDenBakker]

Sorry I misunderstood, I thought webpack does what youre proposing in this PR. Lets keep it as is then.

[shellscape]

😆 I'll wait from direction from you both on when/if to merge.

[tjenkinson]

I think given webpack introduced this(?) we should match their behaviour by default.

We could update this pr to add an ignoreSideEffectsForRoot option though?

If we think it's worth it I can add that otherwise fine closing this.

[shellscape]

I'm good adding that option.

made changes since

[tjenkinson]

I'm good adding that option.

@shellscape done! @07akioni ignoreSideEffectsForRoot should work for your use case :)

[07akioni]

Hope this can be merged soon. 😄