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

Can add sideEffects: false to package.json #2335

Closed
haydenull opened this issue Aug 22, 2023 · 7 comments · Fixed by #2790
Closed

Can add sideEffects: false to package.json #2335

haydenull opened this issue Aug 22, 2023 · 7 comments · Fixed by #2790
Assignees
Labels
c: infra Changes to our infrastructure or project setup p: 1-normal Nothing urgent
Milestone

Comments

@haydenull
Copy link

Is it possible to add sideEffects: false in the package.json file?

This can help rollup with tree shaking.

For example, I want to use fakerjs in development mode but remove it in production mode.

import { faker } from '@faker-js/faker'

if (import.meta.env.DEV) console.log(faker)

When sideEffects is not set to false, even if the condition of if statement is false, faker will still be bundled together.

@ST-DDT
Copy link
Member

ST-DDT commented Aug 22, 2023

What are the requirements for our code to use that safely?

@ST-DDT ST-DDT added p: 1-normal Nothing urgent s: awaiting more info Additional information are requested c: infra Changes to our infrastructure or project setup labels Aug 22, 2023
@haydenull
Copy link
Author

haydenull commented Aug 22, 2023

In web development, intercept API requests and use faker return random data.

However, in production mode, interception is not necessary.

@ST-DDT
Copy link
Member

ST-DDT commented Aug 22, 2023

Sorry, thats not what I meant.
What requirements has sideEffect: false for the Faker code? (Why isnt it true by default if it is that good.)

@haydenull
Copy link
Author

Because removing code during tree shaking is a risky behavior, if a package not specify sideEffects as false, bundlers like rollup will assume that the code in this package has side effects.

In order to ensure the proper functioning of the code, rollup will retain them during tree shaking.

Wether to set sideEffects: false depends on whether the code of faker has any side effects. No modifications to the source code are required.

However, I am not familiar with the code of faker, so I cannot make a judgment.

@ST-DDT
Copy link
Member

ST-DDT commented Aug 22, 2023

Wether to set sideEffects: false depends on whether the code of faker has any side effects.

What is considered a side effect? A Faker internally shared global? Or does side effect require interaction with a third party library like registering a plugin in prettier?

@haydenull
Copy link
Author

I think for faker, the main question is wether it modifies window or global objects. Or wether it introduced other packages with side effects.

sideEffects: false instructs the bundler to remove this package if its exported content is not being used.

I think if my code doesn't use any methods exported by faker, removing faker from build output would not break the program. Therefore, it can be considered as having no side effects.

@matthewmayer
Copy link
Contributor

even if harmless, if this affects the behavior of the bundler it should probably be considered a semver-major change.

@ST-DDT ST-DDT added this to the v9 - Next major milestone Aug 22, 2023
@ST-DDT ST-DDT modified the milestones: v9.x, v9.0 Oct 3, 2023
@ST-DDT ST-DDT linked a pull request Apr 7, 2024 that will close this issue
@xDivisionByZerox xDivisionByZerox removed the s: awaiting more info Additional information are requested label Apr 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: infra Changes to our infrastructure or project setup p: 1-normal Nothing urgent
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants