@vue/compiler-sfc output is not tree-shakeable

mgdodge commented 3 years ago

Version

3.0.4

Reproduction link

https://github.com/mgdodge/rollup-plugin-vue-treeshake-bug-vue3

Steps to reproduce

Originally logged with rollup-plugin-vue, but the tree-shaking issues seem to be coming @vue/compiler-sfc. The following steps are still valid, and still occur with the latest version.

Vue 3 tree-shaking bug

This repo has two branches - one written in Vue 2, one in Vue 3. The components and logic involved are identical (a simple diff of the branches will illustrate this). The only differences are in the dependencies introduced by Vue itself, using defineComponent in Vue 3 instead of Vue.extend for Vue 2, and the inclusion of postCSS for processing the css (required for vue3).

Reproducing the bug

Setup

Clone repo
Checkout Vue 2 branch
Run npm install followed by npm run build and npm pack to create .tgz that can be used in external projects. Rename that file to demolib-vue2.tgz
Checkout Vue 3 branch and repeat last step, renaming file to demolib-vue3.tgz
Create two sample Vue cli projects, one each for Vue 2 and Vue 3
Install cli project dependencies, including the correct .tgz files from the prior steps

Usage

Load 2 of the 3 components provided by the library. Components are Cat, Dog, and Fish.

import { Cat, Dog } from 'demolib'; // No Fish

// For Vue 2
Vue.component('cat', Cat);
Vue.component('dog', Dog);
Vue.mount('#app');

// For Vue 3
const app = createApp(App);
app.component('cat', Cat);
app.component('dog', Dog);
app.mount('#app');

<template>
    <p>Pet options:</p>
    <cat />
    <dog />
    <!-- Fish is not used -->
</template>

Verify tree-shaking

Execute npm run build in each cli project. The resulting "optimized" javascript files will include a chunk-vendors file which should include a tree-shaken version of demolib. Search this file for the following strings:

"The cat is named"
"The dog is named"
"The fish is named"

If tree-shaking worked, you should not see The fish is named anywhere in the output. In the Vue 2 version, tree-shaking works properly. In the Vue 3 version, the unused code is still included.

What is expected?

Code output should be tree-shakeable

What is actually happening?

Code is not tree-shakeable

As noted in this comment on the rollup-plugin-vue bug report, adding /*__PURE__*/ comments to some of the output gets things really close, but there are still some code refactors that are required to get things working.

HcySunYang commented 3 years ago

I think the root cause is here(According to this comment):

script.render = render;
script.__scopeId = "data-v-8c4b8c4a";

I think we only need to make render and __scopeId as part of the defineComponent function parameters, and no need to add the /*#__PURE__*/ annotation on pushScopeId/popScopeId call, because it has side effects.

uimijs commented 3 years ago

你只需要在构建中删除defineComponent code.replace(/(vue.)*defineComponent\(/gi, "/*#__defineComponent__*/(")

mgdodge commented 3 years ago

Not surprisingly, vite.js libarary mode output, though minified, still suffers from this. Thinking this should get a bump in priority, since more prominent tooling/documentation is now promoting libarary builds which will suffer from this.

cowills commented 3 years ago

Should this issue be labelled as a bug instead of a feature? Tree shaking seems like a critical feature for vue 3 given the changes to the global API and the switch from options to composition.

bjkippax commented 3 years ago

Should this issue be labelled as a bug instead of a feature? Tree shaking seems like a critical feature for vue 3 given the changes to the global API and the switch from options to composition.

I'd be inclined to agree here. Tree-shaking is fantastic and to have it not working is inconvenient.

mseele commented 3 years ago

Any progress in this topic? Sadly this is a showstopper to move our private libraries to vue 3, too.

bjkippax commented 3 years ago

Any progress in this topic? Sadly this is a showstopper to move our private libraries to vue 3, too.

Agreed. It’s causing our organisation problems too.

ckvv commented 3 years ago

@mseele @bjkippax We can support tree-shaking by compiling it into multiple files. like this https://github.com/ckpack/v-ui-template, And here is a pack I wrote with it https://github.com/ckpack/vue-color

mgdodge commented 3 years ago

@chenkai0520 I've seen this suggested before, and responded here. While it might work, it is a lot more code to maintain and is a regression when moving from vue 2 to vue 3. The intent of this issue is to discover why it's regressed, and hopefully resolve it. In addition, more comments in that thread indicate that the tree-shaking is only working at a "per file" level, not at a "per function" level - so if your file included some functions that go unused, the unused functions are still included in the bundle.

Also, as indicated in this comment above, this bug is also present in vite library output mode. If the "official" tooling for generating libraries suffers from this issue, I would think it need to be fixed properly, or at the very least, the documentation updated to let people know they need to structure their projects differently, as you are suggesting here.

No disrecpect, I'm glad you've got a workaround, and it's valuable to have a way to do things while waiting for this bug to be addressed.

yyx990803 commented 3 years ago

This is now fixed by https://github.com/vitejs/vite/commit/316d7afc0c84e51359938a12ebe1b09ca34ea8bd + cb2d7c0e

Will need @vitejs/plugin-vue >= 1.8.1.

vue-loader@next will get similar fix soon.

bjkippax commented 3 years ago

This is now fixed by vitejs/vite@316d7af + cb2d7c0

Will need @vitejs/plugin-vue >= 1.8.1.

vue-loader@next will get similar fix soon.

Thanks @yyx990803 - greatly appreciated!!

yyx990803 commented 3 years ago

Now also fixed in vue-loader 16.6+ (https://github.com/vuejs/vue-loader/commit/11e3cb8a8a4a4e0aedc2978ce6d7e549a61de3d7)

mgdodge commented 3 years ago

@yyx990803 , I don't believe this is truly fixed...at least, I can't get it to work. I created another minimal repo to demonstrate - https://github.com/mgdodge/vue-compiler-treeshaking-bug.

The repo uses vite directly instead of any custom rollup configuration. In the demo, I have updated vite to the latest beta, which contains these fixes as far as I can tell.

The repo has a folder for a vite library which should be tree shakeable. It also has two more folders with sample apps (one vite app, one cli app) that attempt to use the library. The README should explain in more detail. Also, the cli app looks like it's using the right version of vue-loader as well.

yyx990803 commented 3 years ago

Ok, so this is because the lib-vue build minifies the dist files, which removes the /*#__PURE__*/ annotations in the generated code. You also forgot to add /*#__PURE__*/ before your own defineComponent calls (I'm not sure why you are doing it in the first place, since you don't need it if you are not using TS, or if you are using <script setup>).

We should probably default to not minifying in lib mode in Vite.

The code generated by @vitejs/plugin-vue is correct and can be treeshaken if you disable minify in lib-vite.

yyx990803 commented 3 years ago

Should be fixed by https://github.com/vitejs/vite/commit/06d86e4a2e90ca916a43d450bca1e6c28bc4bfe2

mgdodge commented 3 years ago

@yyx990803 thanks for all the work on this - you are amazing! Updated to the latest vite release, and when writing the library in vite, then using in a vite app, the code does in fact seem tree-shakeable. Testing tree-shakeability with npx agadoo also looks good. But when using the library in a vue cli app, tree shaking is not happening.

Digging deeper, right at the bottom of the vite library output, it looks like the following code is the only remaining snag:

var components = /* @__PURE__ */ Object.freeze({
  __proto__: null,

  // Removing the [Symbol.toStringTag] line fixes tree shaking
  [Symbol.toStringTag]: "Module",

  Cat: cat,
  Dog: dog,
  Fish: fish
});

I honestly don't know what that line is for, or if it's strictly necessary. If it is, I wonder what can be done to make things work in webpack - most of the world is still going to be using that for a bit, so it's not exactly something that can be easily ignored...

That code is coming from this line in vite. I think I'll file a bug in the vite repo, since technically it's not a problem with @vue/compiler-sfc any more. I can use a similar setup with rollup-plugin-vue (using @vue/compiler-sfc behind the scenes) and tree-shaking works fine with that output. I just hate to lose all this context.

vuejs / core