feat(dist-custom-elements): automatically import and define depencies

[johnjenkins]

The little known dist-custom-element bundle has potential to be great for bundlers and tree-shaking - all components and dependencies are put into separate bundles. The disadvantage at the moment is that each and every component (and any dependency they may have) must be imported and defined individually.

This PR adds a defineCustomElement() to each component bundle created within dist-custom-element. Importing and calling it will not only define the component in question but any dependencies that component might have. e.g.

import { defineCustomElement } from '@ionic/core/components/ion-button';
defineCustomElement();

Will define <ion-button /> and also import and define <ion-ripple-effect />.

[rwaskiewicz]

LGTM! Let's see what CI says

[arvindanta]

Hi @rwaskiewicz , Am using the React wrapper and this auto import and defining dependencies is not working with the React Wrapper from Stencil.

Any workaround to handle it ?

[arvindanta]

I used the v2.9.0-0 version to test this out. If I use the react output target , I see that all the components are getting bundled in the prod build instead of just the ones that are being used.

If I use the dist/components instead of React, its working fine.

Can you please let me know if this will be implemented with React wrapper as well ?

[arvindanta]

@johnjenkins @rwaskiewicz Tree shaking is broken in Stencil React output target.

ionic-team/stencil-ds-output-targets#198

Please find below my observation:

I notice that, the tree shaking is not happening due to the below: The Spinner variable is getting reassigned and then assigned to the export of this module. This is causing the treeshaking to break.

If I create a new variable and assign it instead of using the same Spinner variable and assign it to the export , tree shaking is working fine.
For example if I use an another variable Spinner1 and use

**let Spinner1 = /*@__PURE__*/ proxyCustomElement(Spinner, [1, "fw-spinner"**, {
    "size": [1],
    "color": [1]
  }]);
  export {  Spinner1 as S}
  **

`import { attachShadow, h, proxyCustomElement } from '@stencil/core/internal/client';

const spinnerCss = "";

**let Spinner** = class extends HTMLElement {
constructor() {
  super();
  this.__registerHost();
  attachShadow(this);
  this.size = 'default';
  this.color = '';
  this.sizeMap = {
    small: 12,
    default: 16,
    medium: 24,
    large: 32,
  };
}
getSize() {
  return this.sizeMap[this.size] || 16;
}
render() {
  const diameter = this.getSize();
  return (h("svg", { class: `spinner ${this.size}`, style: {
      'width': `${diameter}px`,
      'height': `${diameter}px`,
      '--spinner-color': `${this.color}`,
    }, viewBox: `0 0 50 50` }, h("circle", { class: 'path', cx: '25', cy: '25', r: '20', fill: 'none', "stroke-width": '5' })));
}
static get style() { return spinnerCss; }
};
**Spinner = /*@__PURE__*/ proxyCustomElement(Spinner, [1, "fw-spinner"**, {
  "size": [1],
  "color": [1]
}]);
function defineCustomElement() {
const components = ["fw-spinner"];
components.forEach(tagName => { switch (tagName) {
  case "fw-spinner":
    if (!customElements.get(tagName)) {
      customElements.define(tagName, Spinner);
    }
    break;
} });
}

export { Spinner as S, defineCustomElement as d };`

[arvindanta]

@splitinfinities The above PR is bloating the React js wrapper build(dist-custom-elements) and bundling all components . I have updated my observations above. Kindly review it. Thanks.

[rwaskiewicz]

Hey @arvindanta 👋

Thank you for your bug report. The team will investigate when they have a chance, but I can't promise a specific timeline. In the future, please know that the team is properly notified of comments on existing issues/PRs, there is no need to ping individual members of the team directly

[arvindanta]

Hi Team,

In the below dist custom elements component created, Tree shaking is not working in React output target.

Reassigning MyComponent with the proxyCustomElement is breaking the tree shaking ability.

Have attached the screenshot which uses the eslint-plugin-tree-shaking to check for issues with tree shaking.

If I use different variable instead of reassigning to MyComponent, tree shaking is working fine with React Wrapper.
``

`import { attachShadow, h, proxyCustomElement } from '@stencil/core/internal/client';

function format(first, middle, last) {
  return (first || '') + (middle ? ` ${middle}` : '') + (last ? ` ${last}` : '');
}

const myComponentCss = ":host{display:block}";

let MyComponent = class extends HTMLElement {
  constructor() {
    super();
    this.__registerHost();
    attachShadow(this);
    /**
     * The first name
     */
    this.first = "first";
    /**
     * The middle name
     */
    this.middle = "second";
    /**
     * The last name
     */
    this.last = "last";
  }
  getText() {
    return format(this.first, this.middle, this.last);
  }
  render() {
    return h("div", null, "Hello, World! I'm ", this.getText());
  }
  static get style() { return myComponentCss; }
};
**MyComponent = /*@__PURE__*/ proxyCustomElement(MyComponent, [1, "my-component", {
    "first": [1],
    "middle": [1],
    "last": [1]
  }]);**
function defineCustomElement() {
  const components = ["my-component"];
  components.forEach(tagName => { switch (tagName) {
    case "my-component":
      if (!customElements.get(tagName)) {
        customElements.define(tagName, MyComponent);
      }
      break;
  } });
}

export { MyComponent as M, defineCustomElement as d };
`

React wrapper code below:

/* eslint-disable */
/* tslint:disable */
/* auto-generated react proxies */
import { createReactComponent } from './react-component-lib';

import type { JSX } from 'import-error/dist/components';

import { MyComponent as MyComponentCmp } from 'import-error/dist/components/my-component.js';
import { SecondComponent as SecondComponentCmp } from 'import-error/dist/components/second-component.js';
import { ThirdCmp as ThirdCmpCmp } from 'import-error/dist/components/third-cmp.js';

export const MyComponent = /*@__PURE__*/createReactComponent<JSX.MyComponent, HTMLMyComponentElement>('my-component', undefined, undefined, MyComponentCmp);
export const SecondComponent = /*@__PURE__*/createReactComponent<JSX.SecondComponent, HTMLSecondComponentElement>('second-component', undefined, undefined, SecondComponentCmp);
export const ThirdCmp = /*@__PURE__*/createReactComponent<JSX.ThirdCmp, HTMLThirdCmpElement>('third-cmp', undefined, undefined, ThirdCmpCmp);

[arvindanta]

Hi Team, This is one of the key features I was looking for regarding dist-custom-elements target.

The only issue that this is having is now the React wrapper is not tree shakable.

**MyComponent = /*@__PURE__*/ proxyCustomElement(MyComponent, [1, "my-component", {
    "first": [1],
    "middle": [1],
    "last": [1]
  }]);**

The above function uses re assign MyComponent variable and this gets exported and is breaking the tree shakable ability (which is very imp with React wrapper(custom elements option)).

Instead of reassigning to MyComponent, If I create a new variable and assign it as below

let MyComponent1 = /*@__PURE__*/ proxyCustomElement(MyComponent, [1, "my-component", {
    "first": [1],
    "middle": [1],
    "last": [1]
  }]);**
export { **MyComponent1** as M, defineCustomElement as d };

Tree shaking is working fine.

Can someone point me to where to handle this change ?

proxyCustomElement is a side effect function and webpack/rollup is tree shaking the modules that uses proxyelement and reassign to the variable.

Please see the attached screenshot. I ran the coverage from chrome dev tools, the statement where proxyCustomElement call is getting reassigned to the class component (MyComponent/SecondComponent/ThirdCmp) is getting used while the rest of the code in those modules are unused and this is breaking the tree shaking ability and these modules are bundled together always

[rwaskiewicz]

Hey @arvindanta - can you please add this information to your existing issue in the stencil-ds-output-targets repo? That keeps information all in one place for the team to investigate this issue when they have the opportunity.

[arvindanta]

Hi @rwaskiewicz Sure , I will add this info there. I feel the issue exists due to the mentioned change with the stencil core package and not very specific to the react output target.