Being more specific with our imports

[kylebuch8]

I have a humble request. Can we be more specific when we import other modules into our components?

The reason I ask is because as we try to use the components and I take a look at the network calls to see what is impacting our page loads, I'm seeing a bunch of files that are included that are never used. For example, in this screenshot, all of the query decorators are imported even though they aren't being used.

In our component files, you'll see this a lot.

import { customElement, property } from 'lit/decorators.js';

And yes, that's nice to be able to do that and it's a convenience. However, what that ends up doing is importing all of the decorators that may not even be used. The lit/decorators.js file ends up importing all of these other files.

export * from '@lit/reactive-element/decorators/custom-element.js';
export * from '@lit/reactive-element/decorators/property.js';
export * from '@lit/reactive-element/decorators/state.js';
export * from '@lit/reactive-element/decorators/event-options.js';
export * from '@lit/reactive-element/decorators/query.js';
export * from '@lit/reactive-element/decorators/query-all.js';
export * from '@lit/reactive-element/decorators/query-async.js';
export * from '@lit/reactive-element/decorators/query-assigned-elements.js';
export * from '@lit/reactive-element/decorators/query-assigned-nodes.js';

When I try to create an optimized bundle, I end up including a bunch of unused code that increases the payload size. I thought that using something like Rollup would do some tree shaking and eliminate the unused code, but it doesn't seem to do that.

So, can we start writing code like this?

import { LitElement } from 'lit-element/lit-element.js';
import { html } from 'lit/html.js';
import { customElement } from 'lit/decorators/custom-element.js';
import { property } from 'lit/decorators/property.js';
import { state } from 'lit/decorators/state.js';

[kylebuch8]

For reference, when I create a bundle for a different component library that we're working on that uses PatternFly Elements, if I exclude PatternFly Elements in my component and create a bundle, I get a bundle size like this.

But when I include rh-cta which imports pf-icon, I get a bundle size like this.

It's over double the size and I know tree shaking isn't happening properly because the chunk file name is query-assigned-elements-8f0538e6-chunk.js. I know rh-cta and pf-icon do not import the queryAssignedElements decorator.

[heyMP]

Interesting. I don't know why query assigned elements would be that big. My first guess is that it's actually the tree-shaken, bundled contents of lit/decorators.js and it's being named query-assigned-elements-chunk.js for some reason.

[heyMP]

I don't think there is a downside to going with your recommendation of specifically importing the files instead of relying on general entrypoints like lit/decorators.js. For our Rollup routines, it's probably going to create individual chunk files for each import meaning that instead of this:

decorators-chunk.js

You're going to get this:

lit-element-chunk.js
html-chunk.js
decorators/custom-element-chunk.js
decorators/property-chunk.js
decorators/state-chunk.js

I would need to test it though to see if that is in fact that case.

[kylebuch8]

I don't think that's the case. @eyevana and I did a rollup for our other component library and got a single chunk in addition to the two components we're distributing.

That chunk includes a bunch of Lit stuff.

[heyMP]

Good to know 👍

[bennypowers]

We could probably whip up a lint rule to fix this automatically

[heyMP]

That would be awesome.

[zhawkins]

Is this something we can apply to all pfe / rhds components? This would help all sites using RHDS/PFE.

[zhawkins]

Say more words about that 😀

[bennypowers]

these two snippets are equivalent

@customElement(name)
class Foo extends HTMLElement {}

class Foo extends HTMLElement {}
function customElement(name) {
  return function(x) {
    customElements.define(name, x); 
  }
}
customElement(name)(Foo)

I suspect that rollup (correctly) identifies that customElements.define call as a side effect, and therefore marks the whole subtree decorators.js as impure, and thus untreeshakable.

If you lie and tell rollup to pretend that lit/decorators/custom-element.js is actually pure, it would probably tree-shake out the rest of the exports from decorators.js

[zhawkins]

If we used direct imports per Kyle, would that make this a moot point?

[bennypowers]

Yes, just that treeshaking could also did this and has a nicer (ugh) "developer experience"

Six of one, half a lint-rule of the other I guess 🤷‍♂️

[zhawkins]

I think I prefer the direct imports approach. That way you don't have to make rollup config recommendations within this project.