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

feat: set sideEffects to false in package.json #262

Merged
merged 1 commit into from
Aug 28, 2023
Merged

Conversation

zOadT
Copy link
Collaborator

@zOadT zOadT commented Aug 27, 2023

This PR sets sideEffects to false to improve the builds for the users.

What does it do?

Some informations can be found here. In our case it will only prevent Planck from being included in the bundle if its not used (and maybe some other minor optimisations from the bundler)

// without the flag, Planck will be included in the build
// bundle even if its not used
import { Vec2 } from 'planck'

// this block is only needed to demonstrate it for TS files
// because TS removes imports that are not used.
if (false /* could also be some build flag */) {
  console.log(Vec2)
}

So yes, pretty minor thing, but also only a pretty minor change. I am currently indeed actively overriding this setting to false in a webpack config.

Is it Webpack specific?

No, even though most of the documentation you see online is for Webpack, also tools like Vite are using it.

How to test it?

You can test the change by making a simple build setup:

npm install planck webpack vite

Create a file src/index.js with the code given above (you can also use .ts for vite. For webpack you would have to add a webpack config to configure ts-loader). To test vite you also have to create a index.html at the root that includes this script tag: <script src="src/index.js" type="module"></script>. Then run

npx webpack --mode=production

or

npx vite build

and check the generated js file in the dist folder.

You can edit node_modules/planck/package.json directly to see how it changes the generated code.

@shakiba
Copy link
Collaborator

shakiba commented Aug 27, 2023

Thanks! Currently testbed has side effect (it reimplement Testbed.mount from main package), would that be a problem now, or if we want to split testbed implementation into its own build?

@zOadT
Copy link
Collaborator Author

zOadT commented Aug 27, 2023

Thanks! Currently testbed has side effect (it reimplement Testbed.mount from main package), would that be a problem now, or if we want to split testbed implementation into its own build?

Ah ok thanks, didn't know about that in testbed. Then we have to provide a glob of modules with side-effects (see last code example of this section)

I guess probably something like

"sideEffects": ["./dist/*-with-testbed.*"],

should work, but I have to test it if it really works with the exports field in package.json.

@zOadT
Copy link
Collaborator Author

zOadT commented Aug 27, 2023

The glob worked! In the above example Planck still won't be bundled, in this example it still got included in the bundle: (tested with webpack and vite)

import { Vec2 } from "planck/with-testbed";

if (false) {
  console.log(Vec2)
}

@shakiba
Copy link
Collaborator

shakiba commented Aug 27, 2023

Cool, thanks!
It would great if you add some instructions, or a repo with sample project, to test it (in case someone wants to change it they know how to test it).

@zOadT
Copy link
Collaborator Author

zOadT commented Aug 28, 2023

Cool, thanks!
It would great if you add some instructions, or a repo with sample project, to test it (in case someone wants to change it they know how to test it).

Good point! I don't really have any fancy setup, but I added a short section about how to test it.

@zOadT zOadT merged commit 3efc923 into master Aug 28, 2023
1 check passed
@shakiba shakiba deleted the side-effects-false branch October 3, 2023 05:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants