Skip to content
This repository has been archived by the owner on Jan 18, 2022. It is now read-only.

ESM output of vue components not tree-shakeable in webpack #344

Closed
mgdodge opened this issue Apr 28, 2020 · 7 comments
Closed

ESM output of vue components not tree-shakeable in webpack #344

mgdodge opened this issue Apr 28, 2020 · 7 comments

Comments

@mgdodge
Copy link

mgdodge commented Apr 28, 2020

Version

5.0.0

Reproduction link

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

Steps to reproduce

  • Clone the repo, which uses rollup-plugin-vue to build a component library via "npm run build".
  • Copy the file dist/testlib.esm.js (the es module build) to a minimal vue-cli app.
  • In the vue-cli app, add import { larry, moe } from '@/testlib.esm.js'; to import only two of the three vue components provided by the library, and display them on the page.
  • Run npm run build in the vue-cli app to produce bundled output

What is expected?

Bundled vue-cli output will include only the vue components used in the app (larry and moe)

What is actually happening?

Bundled vue-cli output includes components which were not used in the app (curly)


Minimal reproduction includes a "helpers" module with one unused helper, to illustrate that the output for raw modules is being properly shaken. ESM output includes the unused helper as an export of the library, but webpack leaves the unused helper out, as expected.

Reported to me via this issue, after which I discovered this solution indicating that the plugin output itself is still an issue.

It seems that each "normalizeComponent" call needs a /*#__PURE__*/ annotation comment for webpack to properly recognize them as tree-shakeable. For example: const __vue_component__$1 = /*#__PURE__*/normalizeComponent({...

@carbotaniuman
Copy link

The quest to make Rollup Vue outputs treeshakable:

So after looking at this issue, I created a sample repository to try and iron out treeshakability issues and confirm my suspicions. I realized quickly that there are 2 main blockers to this, the first of which is the /*#__PURE__*/ annotation mentioned above, which could be fixed by changing this line of vue/vue-component-compiler to const __vue_component__ = /*#__PURE__*/__vue_normalize__(.

The second issue is a little more complicated, but not by much. Each vue render function is transpiled like so:

var __vue_render__ = function() {
    var _vm = this;
    var _h = _vm.$createElement;
    var _c = _vm._self._c || _h;
    return _c("div", { staticClass: "vc-editable-input" }, [
        ...
    ])
};
var __vue_staticRenderFns__$1 = [];
__vue_render__$1._withStripped = true;

The _withStripped property prevents Webpack from treeshaking the function, as it treats it as a global. This could be solved by wrapping __vue_render__ in an IIFE and setting it to /*#__PURE__*/. It also looks like it may be omitted by setting the process.env.NODE_ENV to production, but as a Rollup newbie, I am unsure of how to do that (and whether it would be ideal for libraries to ship code with said environment variable).

@carbotaniuman
Copy link

I managed to get the output to be treeshakable by editing vue-component-compiler and component-compiler-utils, but I'm unsure of whether this is the right thing to do. Some advice would be appreciated before I go to PR.

@mgdodge
Copy link
Author

mgdodge commented May 6, 2020

Regarding process.env.NODE_ENV, if you look at line 12 of package.json in the github repo I created to reproduce the bug, you will notice that it is set via the npm script. It could also be set via any of the standard methods of setting environment variables for your OS.

As for whether that is the right thing to do - it is actually expected to do things that way. The environment variable is not compiled into the output, it is a way the build process identifies whether the build process is producing a final, deliverable version or not.

If you would like to clone a copy of the repo I created for the bug report, you can them make the first change to the vue-component-compiler there. If all is well, then I think that might be enough to submit a PR for this. A single change is more likely to get through than two, especially if the second one is already fixed by passing an environment variable that is expected to be used for that purpose.

@carbotaniuman
Copy link

It works, and I have a copy over here. The only problem is that I can't get the tests to work, but I have manually verified that the output is treeshakable.

@mgdodge
Copy link
Author

mgdodge commented May 7, 2020

I also cloned your repo, and you are right that the tests don't run. In fact, I get typescript errors trying to run a pristine copy without your changes.

I would submit your changes as a PR to the vue-component-compiler repo and mention that testing seems to be broken in ways unrelated to your change.

They do have snapshot testing in there, so even if testing was working properly you would still probably have to resolve those failing snapshot tests, but the change you are putting in simply adds comments to help other tooling, so I don't think that should be a problem.

You have submitted the PR there, link to it here so this issue can be resolved once the PR is accepted over there.

@carbotaniuman
Copy link

Submitted.

@znck
Copy link
Member

znck commented May 11, 2020

Thanks for the PR. Merged and released.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants