Tree shaking broken in rollup-plugin-vue@next

mgdodge commented 4 years ago

Version

6.0.0-beta.10

Reproduction link

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

Steps to reproduce

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 an alternative to creating an entirely new cli project to test, I have used the npm package "agadoo" as a quick test in the past. On the vue 2 version of this repo, it fails with only one small warning (which has a pending PR to fix here). On the vue 3 version, the failure description is much larger, indicating a lot more issues. Using the cli is more accurate, of course, but is much more involved.

This bug was discovered while preparing the vue 3 compatible version of vue-sfc-rollup. As the reporter of another tree-shaking bug in rollup-plugin-vue (resolved) it is something I checked on.

shubhadip commented 4 years ago

Hi, i was trying to build a vue-component library using vue 3 and was able to get treeshaking. I have working project at Github. Hope this helps

mgdodge commented 4 years ago

Might give it a try...but the repo I posted is pretty vanilla usage. I would assume that something that "just works" with Vue 2 should be able to upgrade to vue3 without jumping through so many hoops and undocumented configuration...

mgdodge commented 3 years ago

@shubhadip I took a look at the code in that sample repo. I personally couldn't get it to build, but that is beside the point I wish to make - namely, there is a LOT more in there than the bare minimum. The sample repo I provided is based on the minimum required to get a vue component/library on npm, and asking users of my utility to change so much of how they write components is not feasible. For example, instead of a directory of .vue files, you have several directories, one for each .vue file, and each one of those is imported into its own dedicated index.ts file for registration/export. This is a lot more code to maintain. There are some things you have done which are nice/interesting, but your repo introduces a lot more boilerplate than I think is required.

As I indicated, the sample I posted works fine with Vue2, and the differences with Vue3 are so minimal and isolated as to highlight that rollup-plugin-vue is where things start to change in a non-tree-shakeable way.

mgdodge commented 3 years ago

Updated to latest 6.0.0 release, and still not fixed. I tinkered with the output a little, and by manually adding the /*#__PURE__*/ to each call of styleInject, pushScopeId, and popScopeId, I can get really close. Then, if I move the _withId, pushScopeId, popScopeId portions before defineComponent, and define the render function and __scopeId as part of the defineComponent object (which seems to be valid according to the typescript defs), that seems to push things over the finish line!

Here's a before and after:

// === BEFORE ADJUSTMENTS

var script = /*#__PURE__*/defineComponent({
  // component props, setup, etc.
});

const _withId = /*#__PURE__*/withScopeId("data-v-8c4b8c4a");

pushScopeId("data-v-8c4b8c4a");
const _hoisted_1 = { /* createBlock props used below */ };
popScopeId();

const render = /*#__PURE__*/_withId((_ctx, _cache, $props, $setup, $data, $options) => {
  return openBlock(), createBlock("div", _hoisted_1, /* more generated render function */);
});

var css_248z = "CSS to inject";
styleInject(css_248z);

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

// === AFTER ADJUSTMENTS

const _withId = /*#__PURE__*/withScopeId("data-v-8c4b8c4a");

/*#__PURE__*/pushScopeId("data-v-8c4b8c4a");
const _hoisted_1 = { /* createBlock props used below */ };
/*#__PURE__*/popScopeId();

var script = /*#__PURE__*/defineComponent({
  // component props, setup, etc.
  render: /*#__PURE__*/_withId(function render(_ctx, _cache, $props, $setup, $data, $options) {
    return openBlock(), createBlock("div", _hoisted_1, /* more generated render function */);
  }),
  __scopeId: "data-v-8c4b8c4a"
});

var css_248z = "CSS to inject";
/*#__PURE__*/styleInject(css_248z);

Also, worth noting that https://github.com/egoist/rollup-plugin-postcss/issues/293 might be related. As illustrated here, adding the /*#__PURE__*/ to the styleInject calls fixes tree-shakeability, though I'm not sure if that's something that needs to be done on this end or on their end.

colgin commented 3 years ago

Hi, i was trying to build a vue-component library using vue 3 and was able to get treeshaking. I have working project at Github. Hope this helps

Hi, Thanks providing such sample project, but your sample project, every component was bundle to a single file, it's a way to make tree shaking work by file, but tree shaking doesn't work in a specific file, for examle, hello.tsx was bundle to hello.xxx.js in dist/esm, in this file, you export the component, and a function named foo. If a application import foo from lib ONLY, everything in hello.xx.js was bundled, tree shaking broken again.

in mgdodge's sample, every component was bundle in a single output, everything was bundle to your application if tree shaking broken. so the difference between you two is the lines of unused code be bundle to your app. your sample produce unused code in specific file, and mgdodge's sample produce all of the lib.

williamweckl commented 3 years ago

Any updates on this? I'm still experiencing this issue using 6.0.0.

williamweckl commented 3 years ago

For the record, I was able to do tree shaking using the option preserveModules: true at output. Worked as expected to me.

williamweckl commented 3 years ago

Even though setting preserveModules: true made my lib become tree shakable, with this configuration the CSS are not being injected to head...

Also I've tried a lot of different things and was not able to make scoped css to work, with or without preserveModules... The imported component never has the data-v- attribute.

So, not tree shaking neither scoped css are working... Am I doing something wrong?

This is my rollup.config.js:

import path from 'path';

import esbuild from 'rollup-plugin-esbuild'
import vue from "rollup-plugin-vue";
import postcss from 'rollup-plugin-postcss';

export default {
  input: "src/index.ts",
  output: [
    {
      format: "esm",
      dir: "dist",
      sourcemap: true,
    }
  ],
  plugins: [
    vue({
      target: "browser",
      preprocessStyles: true,
      defaultLang: {
        style: 'scss',
      },
      css: false,
    }),
    postcss({}),
    esbuild({
      sourceMap: true,
      minify: false,
      target: 'esnext'
    }),
  ],
  external: ['vue', 'vue-class-component'],
}

My tsconfig.json:

{
  "compilerOptions": {
    "target": "esnext",
    "module": "esnext",
    "strict": true,
    "jsx": "preserve",
    "importHelpers": true,
    "moduleResolution": "node",
    "experimentalDecorators": true,
    "skipLibCheck": true,
    "esModuleInterop": true,
    "allowSyntheticDefaultImports": true,
    "sourceMap": true,
    "baseUrl": ".",
    "types": [
      "webpack-env",
      "jest"
    ],
    "paths": {
      "@/*": [
        "src/*"
      ]
    },
    "lib": [
      "esnext",
      "dom",
      "dom.iterable",
      "scripthost"
    ]
  },
  "files": [
    "./src/shims-vue.d.ts"
  ],
  "include": [
    "src/**/*.ts",
    "src/**/*.tsx",
    "src/**/*.vue",
    "tests/**/*.ts",
    "tests/**/*.tsx"
  ],
  "exclude": [
    "node_modules",
    "dist"
  ]
}

mgdodge commented 3 years ago

Will need to reexamine once https://github.com/vuejs/vue-next/issues/2860 lands

mgdodge commented 3 years ago

Looked at vuejs/vue-next#2860 and things still look broken - see this comment and the new sample repo mentioned there for details.

mgdodge commented 3 years ago

The output produced by vite + @vitejs/plugin-vue is really close to perfect, but still has one bug in it. The output from rollup-plugin-vue is a bit further off, but not by much! Also, it doesn't suffer from the same bug that vite still does.

Please take a look at how the _export_sfc function generated by vite is used, that resolves pretty much all major tree-shaking issues. By manually implementing the _export_sfc logic in the output, and moving css to be external, tree shaking works!

For example, here is a before/after of just this logic:

// Before change
var script$2 = /*#__PURE__*/defineComponent({
  name: 'Cat',
  // vue component name
  props: {
    name: {
      type: String,
      default: 'Spot'
    },
    says: {
      type: String,
      default: 'meow'
    }
  }
});

function render$2(_ctx, _cache, $props, $setup, $data, $options) {
  return openBlock(), createElementBlock("div", {
    class: normalizeClass(_ctx.$style.cat)
  }, [createElementVNode("p", null, "The cat is named " + toDisplayString(_ctx.name) + ". It says " + toDisplayString(_ctx.says) + ".", 1)], 2);
}

var style0 = {
  "cat": "_cat_1c1rs_2"
};

const cssModules = script$2.__cssModules = {};
cssModules["$style"] = style0;
script$2.render = render$2;

// ... later
export { script$2 as Cat };

// After change
var _export_sfc = (sfc, props) => {
  for (const [key, val] of props) {
    sfc[key] = val;
  }
  return sfc;
};
var script$2 = /*#__PURE__*/defineComponent({
  name: 'Cat',
  // vue component name
  props: {
    name: {
      type: String,
      default: 'Spot'
    },
    says: {
      type: String,
      default: 'meow'
    }
  }
});

function render$2(_ctx, _cache, $props, $setup, $data, $options) {
  return openBlock(), createElementBlock("div", {
    class: normalizeClass(_ctx.$style.cat)
  }, [createElementVNode("p", null, "The cat is named " + toDisplayString(_ctx.name) + ". It says " + toDisplayString(_ctx.says) + ".", 1)], 2);
}

var style0 = {
  "cat": "_cat_1c1rs_2"
};

/* Attach css and render function to component in a tree-shaking friendly way */
const cssModules = {};
cssModules["$style"] = style0;
var cat = /* @__PURE__ */ _export_sfc(script$2, [["render", render$2], ["__cssModules", cssModules]]);

// ...later

export { cat as Cat };

Of course, the styleInject issue still exists upstream, but extracting the CSS to a separate file is a good workaround for that, and arguably a more flexible way of doing things. People who want the CSS import that separate file, those who don't just leave it out.

vuejs / rollup-plugin-vue