Skip to content

chore(@vue/shared): add "sideEffects": false #5480

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

Merged
merged 1 commit into from
Apr 13, 2022

Conversation

haoqunjiang
Copy link
Member

@haoqunjiang haoqunjiang commented Feb 24, 2022

As far as I can tell, all the code in this package is side-effects-free.

Without the sideEffects flag, bundlers rely on code analysis and
the /*#__PURE__*/ comments for tree-shaking.
As bundlers' implementations vary, the analysic may or may not be
exhaustive.

For example, a file that contains only the following line of code:

import { capitalize } from '@vue/shared'

If bundled by esbuild (esbuild --bundle test.js --tree-shaking=true),
the output would be tens of lines.

(Technically, it's not esbuild's bug, because Object.freeze can be overwritten, Array.isArray could be overwritten to a getter, + can have side effects… Even though we know they are side-effects-free, it would require much more work from us to make bundlers happy)

After this PR, the output will be an empty IIFE.

Credit to @hardfist for discovering this issue.

As far as I can tell, all the code in this package is side-effects-free.

Without the `sideEffects` flag, bundlers rely on code analysis and
the `/*#__PURE__*/` comments for tree-shaking.
As bundlers' implementations vary, the analysic may or may not be
exhaustive.

For example, a file that contains only the following line of code:
```js
import { capitalize } from '@vue/shared'
```
If bundled by esbuild (`esbuild --bundle test.js --tree-shaking=true`),
the output would be tens of lines.

After this PR, the output will be an empty IIFE.

Credit to @hardfist for discovering this issue.
@netlify
Copy link

netlify bot commented Feb 24, 2022

✔️ Deploy Preview for vue-next-template-explorer ready!

🔨 Explore the source changes: 7b8b4c2

🔍 Inspect the deploy log: https://app.netlify.com/sites/vue-next-template-explorer/deploys/6217927e48fcda0009538a81

😎 Browse the preview: https://deploy-preview-5480--vue-next-template-explorer.netlify.app

@netlify
Copy link

netlify bot commented Feb 24, 2022

✔️ Deploy Preview for vue-sfc-playground ready!

🔨 Explore the source changes: 7b8b4c2

🔍 Inspect the deploy log: https://app.netlify.com/sites/vue-sfc-playground/deploys/6217927e3aff910007cfc9a4

😎 Browse the preview: https://deploy-preview-5480--vue-sfc-playground.netlify.app/

@netlify
Copy link

netlify bot commented Feb 24, 2022

✔️ Deploy Preview for vuejs-coverage ready!

🔨 Explore the source changes: 7b8b4c2

🔍 Inspect the deploy log: https://app.netlify.com/sites/vuejs-coverage/deploys/6217927e5c702b0008ab705b

😎 Browse the preview: https://deploy-preview-5480--vuejs-coverage.netlify.app

@haoqunjiang
Copy link
Member Author

There's another potential improvement to Vue's tree-shakability:

The sideEffects field was removed from the main package because we don't want bundlers to shake the registerRuntimeCompiler(compileToFunction) call.

But we can use array-style sideEffects to mark all non-runtime-entries with side effects, and let the bundlers take runtime entries as side-effects-free:

  "sideEffects": [
    "dist/vue.cjs.js",
    "dist/vue.cjs.prod.js",
    "dist/vue.esm-browser.js",
    "dist/vue.esm-bundler.js",
    "dist/vue.global.js",
    "dist/vue.global.prod.js"
  ],

(This will shake away the initDev() call in runtime bundles. But I don't think anyone would enable tree-shaking in development, so it should be fine.)

Though, maintaining such an array in the long run seems too much burden, so I didn't add it in this PR.

@LinusBorg LinusBorg added the ✨ feature request New feature or request label Mar 17, 2022
@yyx990803 yyx990803 merged commit 74d239c into vuejs:main Apr 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ feature request New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants