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

commonjs: don't emit modules with side effects - or warn about it #1342

Closed
frehner opened this issue Nov 11, 2022 · 3 comments
Closed

commonjs: don't emit modules with side effects - or warn about it #1342

frehner opened this issue Nov 11, 2022 · 3 comments

Comments

@frehner
Copy link

frehner commented Nov 11, 2022

  • Rollup Plugin Name: commonjs & node-resolve
  • Rollup Plugin Version: commonjs: 23.0.2. node-resolve: 15.0.1

I looked around, and found #1177 that may be somewhat similar, but it doesn't seem to quite be the same thing.

Feature Use Case

When commonjs emits code, in some situations it creates a module that has side effects. This can be problematic for library authors who have the sideEffects field in package.json, as they then are required to figure out which emitted modules have side effects, track them down, and then add them to the sideEffects array.

For more context, this issue was originally raised in the Vite repo here: vitejs/vite#10866 (comment) but was eventually found to be an issue with (probably?) the commonjs plugin.

Here's a StackBlitz example of the emitted code that has a side effect:

https://stackblitz.com/edit/rollup-template-1kqybv?file=dist%2Fnode_modules%2Fuse-sync-external-store%2Fshim%2Fwith-selector.js,rollup.config.js

The file dist/node_modules/@xstate/react/es/fsm.js has two imports:

import '../../../use-sync-external-store/shim/with-selector.js';
...
import { w as withSelector } from '../../../../_virtual/with-selector.js';

The first import must not be treeshaked/removed, as it has the side effect of making the second import work. However, as a library author, it was very difficult for me to find this out as the side effect doesn't exist in my library and was the result of a transformation of a transitive dep.

Feature Proposal

Is it possible to change the emitted code to be self contained and not side-effect-y?

If not, is it somehow possible to notify the dev that modules with side effects were created, so that I could be aware of it and add it to my package.json's sideEffect array?

Thanks!

@Danji-ya
Copy link

Danji-ya commented Jan 9, 2023

I faced this problem too.

import './isBuffer.js';
import { i as isBuffer$1 } from '../../_virtual/isBuffer.js';

@stale stale bot added the x⁷ ⋅ stale label Mar 18, 2023
@stale
Copy link

stale bot commented Mar 25, 2023

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.

@stale stale bot closed this as completed Mar 25, 2023
@thipokch
Copy link

thipokch commented May 9, 2024

I'm having this issue too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants