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

Allows the Cedar library to be tree shaken in consuming applications #131

Merged
merged 7 commits into from
May 3, 2023

Conversation

HeavyMedl
Copy link
Collaborator

@HeavyMedl HeavyMedl commented Apr 27, 2023

Problem

The cedar.mjs distributable asset is the total collection of all Cedar library components. Even though cedar.mjs is an ES module, Vite/Rollup cannot tree shake the components it needs from cedar.mjs. In consuming application builds, this results in Vite/Rollup building the entirety of the Cedar into a module, where it is eventually downloaded by end users.

As a test, I configured Vite to build just the Climber's landing page bundle, stripped all Cedar components from all nested components within the page component. I imported a single Cedar component, CdrContainer, to see the resultant bundle.

Screenshot 2023-04-26 at 6 52 17 PM

Within the compiled bundle, I expect to see just the CdrContainer code, however, I see the entirety of the Cedar library.

// .../target/classes/dist/climber/climber-60b97b4d.js
c({
  name: "CdrAccordion",
  components: { IconCaretDown: Xs },
  props: {
    id: { type: String, required: !0 },
    opened: { type: Boolean, default: !1 },
    compact: { type: Boolean, default: !1 },
    borderAligned: { type: Boolean, default: !1 },
    level: { type: [String, Number], required: !0 },
    contentSpacing: { type: Boolean, default: !0 },
    label: { type: String },
  },
  emits: ["accordion-toggle"],
...
c({
  name: "CdrPopover",
  components: { IconXSm: qr, CdrButton: zr, CdrPopup: rl },
  props: {
    position: {
      type: String,
      required: !1,
      default: "top",
      validator: (e) => re(e, ["top", "bottom", "left", "right"]),
    },
    autoPosition: { type: Boolean, required: !1, default: !0 },
    label: { type: String, required: !1 },
    id: { type: String, required: !0 },
    contentClass: { type: String, required: !1 },
    open: { type: Boolean, default: !1, required: !1 },
  },
  emits: ["opened", "closed"],
...

Solution

This PR sets preserveModules to true which breaks up the large cedar.mjs distributable into multiple ES modules. This allows the Cedar library to be tree shaken in consuming applications.

I installed the npm packed tarball from this PR into climbers-site. Using the same test setup, we see a major difference at build time:

Screenshot 2023-04-26 at 7 20 05 PM

Now we only see CdrContainer code in climber-3e7f1771.js.

Thats nearly a 80% reduction in the uncompressed bundle size (60% reduction on the gzipped version). Obviously, this would be a less dramatic difference the more unique Cedar components I used, but its still a major improvement.

Notes/caveats

I've taken some liberties with this PR that should be examined and reviewed by the team.

Removed UMD format from Vite config

In order to use preserveModules, we need to isolate a ES build in the Vite config.

  build: {
    lib: {
      entry: './src/lib.js',
      formats: ['es'],
      // fileName: (format, entryName) => {
      //   const { base } = path.parse(entryName);
      //   return `${base}.mjs`;
      // },
      fileName: '[name]',
    },
    rollupOptions: options,
  },

Using fileName: '[name]' preserves the directory structure of the output ES modules within dist. This seems like the intuitive thing to do. However, if you prefer a flat output, you can opt to use the commented out function I left.

I'm not sure if anybody uses the UMD module that Cedar outputs, but it case you'd still like this, I've created a separate vite.umd.config.mjs. You'd just need to run vite against this in your NPM build script.

Updated external in rollupOptions.mjs

I've changed this to use a function that ensures any submodules of these dependencies are externalized. Without this, while using preserveModules, submodules from core-js and lodash-es get built into the distributables, which we don't want.

History

When we first migrated to Vue 3 and Vite, Vite was on version 2. We validated that tree-shaking was working against apps built with Webpack 4 and Vite 2.

Webpack needed PURE annotations in the compiled code in order to tree shake stuff out, while Vite 2 seemed to work out of box.. or so we thought.

It's unclear whether or not we mistook tree shaking for common chunks generated by Vite/Rollup. Its possible tree shaking hasn't been working since the migration but because we're serving smaller chunks, it appeared tree shaking was happening.

Reference

…es for distribution. Enables tree-shaking downstream
@HeavyMedl HeavyMedl added the bug Something isn't working label Apr 27, 2023
@@ -2,7 +2,7 @@
import {
defineComponent, useCssModule, computed, ref, reactive, onMounted, provide,
} from 'vue';
import debounce from 'lodash-es/debounce';
import { debounce } from 'lodash-es';
Copy link
Collaborator Author

@HeavyMedl HeavyMedl Apr 27, 2023

Choose a reason for hiding this comment

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

We need to destructure the named exports from the lodash-es module, which points to the module entry defined in lodash-es/package.json. Otherwise, in the context of testing in consuming applications, Vitest whines about not being able to import lodash-es/debounce without a .js extension.

output: {
globals: {
vue: 'Vue',
},
preserveModules: true,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The magic

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Remove this file if UMD is not needed.

@benjag benjag self-assigned this Apr 27, 2023
Copy link
Member

@peripateticus peripateticus left a comment

Choose a reason for hiding this comment

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

🎉🎉🎉

@benjag benjag merged commit 05069a5 into main May 3, 2023
@benjag benjag deleted the pr/tree-shaking branch May 3, 2023 19:59
@peripateticus peripateticus mentioned this pull request May 10, 2023
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants