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

perf: mark defineComponent as side-effects-free #8512

Merged
merged 2 commits into from
Jul 11, 2023
Merged

Conversation

antfu
Copy link
Member

@antfu antfu commented Jun 7, 2023

Rollup now supports marking a function as sied-effect free:

It's a common pitfall in tree-shaking component libraries. For example

export const RouterView = defineComponent(...)
 
export const RouterLink = defineComponent(...)
import { RouterView } from './my-route.ts

It seems RouterLink should be tree shaken, but in reality, because defineComponent is a bit complex that Rollup can't know whether it contains side-effects, causing it still remain in the bundle even if it's not been used.

A solution to that is to add the __PURE__ comment to mark them as pure. But that requires to do so every single time, which can easily be forgotten.

export const RouterView = /*#__PURE__*/ defineComponent(...)
 
export const RouterLink = /*#__PURE__*/ defineComponent(...)

So, instead of asking every callee to add __PURE__, we mark the defineComponent function itself to be side-effect-free using __NO_SIDE_EFFECTS__. So they can always be safely tree-shaken.

In this PR, the following functions are marked side-effect free:

  • defineComponent
  • defineAsyncComponet
  • defineCustomElement
  • defineSSRCustomElement

Side note: because we are using esbuild, which does not preserve comments. We need to use /*! to keep the controlling comment, before esbuild supports __NO_SIDE_EFFECTS__ natively (evanw/esbuild#3149)

Copy link
Member

@sxzz sxzz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sxzz sxzz added the ready to merge The PR is ready to be merged. label Jun 10, 2023
@yyx990803 yyx990803 force-pushed the perf/no-side-effects branch from 293d1b9 to e682bb4 Compare July 11, 2023 09:50
@yyx990803 yyx990803 merged commit 438027c into main Jul 11, 2023
@yyx990803 yyx990803 deleted the perf/no-side-effects branch July 11, 2023 09:52
@Mini-ghost
Copy link
Contributor

Starting from version 0.18.1, esbuild supports the /* @__NO_SIDE_EFFECTS__ */ comment for functions (evanw/esbuild@cf687c9). Would it be possible to consider replacing /*! with /* ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge The PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants