RFC: Deprecating dist-custom-elements-bundle

[splitinfinities]

Premise

In helping the Ionic Framework team the launch of v6, we've made updates to the Framework Bindings (React, Angular, and Vue) which have better bundling capabilities by using the dist-custom-elements output target. Through this process, we've discovered that we really didn't understand why we were maintaining two output targets - dist-custom-elements and dist-custom-elements-bundle.

As of Stencil v2.12.0, if you are using dist-custom-elements-bundle, you will begin to see a deprecation message for the dist-custom-elements-bundle output target which asks you to use dist-custom-elements instead. In an upcoming major version of Stencil, the dist-custom-elements-bundle will be removed. The deprecation message will have a link to this issue, so that we can keep feedback organized.

If you use the dist-custom-elements-bundle output target, and have a unique reason that the dist-custom-elements output target will not solve, we'd love it if you shared your use case here.

Thank you so much for using Stencil!

2022.01.06 - Update

Hey folks, @rwaskiewicz here. I'm working on reviewing the needs/concerns y'all have brought up, great feedback thus far! I wanted to post a high level update in the PR summary (although I'm sure I'll have conversations with a few of you in threads).

Typings

A few of you have mentioned that typings aren't being generated for the dist-custom-elements output target. Although it's strange, I can confirm that that is indeed the current behavior of Stencil. I've got some additional research work scheduled around this topic, but assuming that doesn't throw up any red flags, we'll be adding typings to dist-custom-elements.

ESM Support

I'll be reaching out in GH here to get a better idea of what libraries/frameworks are blocking adoption in your projects. Jest is certainly a big one that I'm aware of, but I want to get a better sense of the situation from y'all.

Externalizing Dependencies

I need to do a little more research/thinking about this before making any kind of statement on where we'll go with it

Re-exporting components via a common index.js file

Similar to externalizing dependencies, I want to do a little testing around this. I don't think it would be hard per se, but I wanted to be cautious about overpromising specific solutions to (very valid) use cases.

[MarkChrisLevy]

I'm using dist-custom-elements-bundle but had to make few improvements in order to make it work (see https://github.com/ionic-team/stencil/issues/2531#issuecomment-900903021, few of those improvements will be useful in dist-custom-elements, e.g. external option - I will make PR for this and few others).

Bundle build packs all components inside one collection into one module, which can be then imported and loaded with defineCustomElements function, how it is different to dist-custom-elements?

[johnjenkins]

@MarkChrisLevy there is no defineCustomElements in the dist-custom-elements bundle as all components are in their own separate module. I guess it was a bit of an anti-pattern / only designed for prototyping anyway.

Instead, every module comes with their own defineCustomElement. This will import and bootstrap not only the component in question, but also any explicit dependency e.g.

  // my-specific-button.tsx
  ...
  render() {
    <div>
      <my-generic-button />
    </div>
  }

  ...
  
  // my-other-project.ts
  @import '@my-components/components/my-specific-button';

In that instance my-specific-button will import and define my-generic-button (if it hasn't already been defined) - by default, defineCustomElement will run automatically upon importing a component module as a sideEffect. You can opt-out of this by setting autoDefineCustomElements: false in the outputTarget config for dist-custom-elements.

[MarkChrisLevy]

@johnjenkins Thanks for explanation, will give it a try and let you know if I faced any obstacles .

[roman-telia]

In our case, SSR support is much worse with the new dist-custom-elements as This change can impact frameworks that are not supporting es6 modules, mainly SSR. Are there going to be some fixes for SSR support provided by the Stencil team?

[johnjenkins]

@roman-telia the team are working to improve ssr with the define-custom-elements output rn. I think they'd really like any feedback about how they can improve this further :)

[roman-telia]

@roman-telia the team are working to improve ssr with the define-custom-elements output rn. I think they'd really like any feedback about how they can improve this further :

We noticed that in 2.11.0-0 there are fixes with SSR checks. We had some issues with elements using HTMLElement classes from the global scope and that caused our SSR apps to crash on the build step. We will check if the latest changes solve these issues.

Regarding the Hydrate app, When we are using the hydrate app does the different bundle types have any impact on prerendered/hydrated HTML?

[johnjenkins]

Regarding the Hydrate app, When we are using the hydrate app does the different bundle types have any impact on prerendered/hydrated HTML?

I don't believe so as pre-rendered hydration is it's own output I think. Someone from the core team can correct me on this though

[splitinfinities]

Worth explicitly denoting here that plain Node SSR environments may need modules converted to a CommonJS format to work.

[splitinfinities]

Regarding the Hydrate app, When we are using the hydrate app does the different bundle types have any impact on prerendered/hydrated HTML?

I don't believe they do, however like I mentioned above, unless the environment you're using the functions produced from the dist-hydrate-script output target does not support es6 modules - like vanilla Node for example. I still need to do some research around what it would take to get the es6 modules to be read in a CommonJS format with just a plain Node server. The "dist" output target (colloquially the "Lazy load" format for your components) do have the commonjs or systemjs formats as options produced from the compiler. So we may need to consider producing those outputs as well for ease of component library consumption within Node environments for SSR.

Now for the direct answer here, they generally don't, but since they remove the lazy loading features, they have more hard edges around when the component gets upgraded on the frontend, like how customElements.whenDefined/HTMLElement. connectedCallback can happen out of sync in some circumstances. The lazy load code and its task queue works to solve that problem, but it's not added in the custom elements output in order to produce the most plain web component code as we possibly can.

[MarkChrisLevy]

@splitinfinities @johnjenkins I did some testing with my component libs and found out that dist-custom-elements will do the work, however two things I'm missing:

• The types (d.ts) are not generated at all -> adding additional output target dist-types doesn't work, as stencil throws "invalid outputTarget". Imho types should always be generated and typesDir: string should be added to dist-custom-elements output target.
• Making some deps as external -> this can be done by rollup plugin, but it would be easier to have "external: (string | RegExp)[]" option in dist-custom-elements output target, which will be passed to rollup

In both cases I can make apropriate PR.

[MarkChrisLevy]

@splitinfinities @johnjenkins Any comment about types and externals? (I mentioned about it in my previous comment). Those two things blocks me in making a switch from dist-custom-elements-bundle to dist-custom-elements.

[wbern]

In my org we use custom-elements-bundle to then recompile the output into a systemjs bundle, which ensures we load the custom elements before the apps load (single-spa / systemjs). We basically want to eliminate lazy-loading in all instances except our own.

I'm not sure if we can load it as seamlessly with the dist-custom-elements output type, I guess we'd have to bundle it all together somehow?

[wbern]

I tried to use dist-custom-elements now, and it's a little finicky because there's no index.js that exports all the files, so we can't easily just bundle all the files into a single bundle without pointing at every file.

If you added an index.js that wasn't empty but instead re-exported every file, we could use that to create our own bundle.

[vmcbaptista]

I face exactly the same issue @MarkChrisLevy is reporting, types are not being generated

[kalongthewires]

To build on what others have said and add some (hopefully helpful?) context: We have a component library built using Stencil and we have two main groups of users -- those that want an easy plug-and-play but still performant option, e.g. they can import like so:

import { WhateverComponent } from 'our-web-component-library';

And a second group of micro frontend users that care a lot about optimizing performance as much as possible. They like having the ability to import individual component files and seeing zero references to unused components in their production bundle:

import { WhateverComponent } from 'our-web-component-library/dist/components/whatever-component';

We were effectively serving all of our users by including all three outputTargets: dist (for the TypeScript types especially), dist-custom-elements, and dist-custom-elements-bundle in our stencil.config. In our package.json then we set the following:

"module": "dist/custom-elements/index.js",
"types": "dist/custom-elements/index.d.ts"

This was working great and we really appreciated the flexibility the different outputTargets provided.

The main pain points for us when we remove dist-custom-elements-bundle:

• No index.js file with components that we can wire up to module in package.json
• If we follow the instructions to set "module": "dist/components/index.js", we see a warning on every build that the module property should be set to dist/index.js.

[rwaskiewicz]

Hey all, I wrote up a quick summary of where we're at with this issue. As we start to gear up for Q1 (which starts in February for Ionic), I'll be working to help refine the scope of work needed to make the transition off of dist-custom-elements-bundle as seamless as possible. Hang tight and thanks for your patience!

[TRMW]

Both the default dist output target and dist-custom-elements-bundle export a defineCustomElements function that you can use to register all components included in a Stencil library. From what I can tell, the dist-custom-elements target currently does not. This would be nice to have as a convenience method for consumers of our design system, and would make migrating off of custom-elements-bundle more seamless for anyone currently using this function.

I see that dist-custom-elements does have a autoDefineCustomElements option, but I believe this only includes all child components of a single imported component.

[rwaskiewicz]

Hey folks,

We just released Stencil v2.17.0, which has the option to export your components via a single index.js file. The documentation is available as a part of the dist-custom-elements documenation, please let us know if you have any issues/questions/comments!

[miqmago]

We are using stencil to build this library forms-reactive which is a port from angular reactive forms to any javascript.

I'm trying to switch from dist-custom-elements-bundle to dist-custom-elements in order to publish the library. With dist-custom-elements-bundle we could make reference to types and module in package.json like this:

    "types": "dist/custom-elements/index.d.ts",
    "module": "dist/custom-elements/index.js",

I've been reading the documentation @rwaskiewicz pointed in previous comment and when switching to dist-custom-elements it seems that I should use the following values, but I'm not really sure if it would introduce any breaking changes:

    "types": "dist/components/index.d.ts",
    "module": "dist/components/index.js",

The point is that when running the build, a warning appears which is confusing me:

[ WARN  ]  Package Json: package.json:6:5
           package.json "module" property is set to "dist/components/index.js". It's recommended to set the "module"
           property to: ./dist/index.js

Please any help will be really appreciated.

[rwaskiewicz]

@miqmago I believe this is a bug in Stencil. We're going to track that with https://github.com/ionic-team/stencil/issues/3438

[rwaskiewicz]

@MarkChrisLevy

In this comment you mentioned missing the following functionality:

Making some deps as external -> this can be done by rollup plugin, but it would be easier to have "external: (string | RegExp)[]" option in dist-custom-elements output target, which will be passed to rollup

Can you explain your use case for this functionality? Is this specific to dist-custom-elements?

You mentioned that it could be achieved by a rollup plugin, is there a particular benefit that you see Stencil supporting this functionality natively?

[MarkChrisLevy]

@rwaskiewicz Today I wanted to add a new FR related to this topic, so good timing of your comment 😉 There are two cases related to "external" option:

1. Marking, that a library (piece of external code), that is used by a component should not be packed in the component's output bundle
2. Marking, that an external lazy loaded component that is used by a component should not be packed in the component's output bundle

In the first case, lets say that we have a dependency to moment.js - right now, if you build a component and within the code of the component there are imports from moment.js, then all required code from moment.js (and its dependencies) will be put in the output bundle. That is fine and proper default behavior, but in my case moment.js is used across multiple components and within main project (which imports those components), moment.js is also directly imported so I would end up with moment.js being bundled three times. Having an "external" option in dist-custom-elements will allow to avoid that. As I mentioned, this case can be covered by a simple rollup plugin defined in stencil.config.ts for the component:

export function externalsPlugin(externals: (string | RegExp)[]) {
    return {
        id: "externalsPlugin",
        resolveId(id: string) {
            for (const e of externals) {
                if ((typeof e === "string" && id === e) || (e instanceof RegExp && e.exec(id))) {
                    return {id, external: true}
                }
            }
        }
    }
}

Second case however cannot be fully resolved by a rollup plugin. Lets say, that we created a component, that uses @ionic/core lazy loaded component, e.g. ion-button. If you build the component, you will have in the output bundle the ion-button code but also other ionic components (whole collection is consumed). Having an "external" option would tell rollup to treat @ionic/core as external but also to treat stencil collections as external components.

In my Stencil fork both of those cases are covered in dist-custom-elements-bundle output target, below you may see what changes I had to make in order to make it work:

• https://github.com/CODE-AND-MORE/stencil/blob/7c4d13be238fe76abc097c07ddc7b305bfe0383e/src/compiler/bundle/bundle-interface.ts#L13
• https://github.com/CODE-AND-MORE/stencil/blob/7c4d13be238fe76abc097c07ddc7b305bfe0383e/src/compiler/bundle/bundle-output.ts#L98
• https://github.com/CODE-AND-MORE/stencil/blob/7c4d13be238fe76abc097c07ddc7b305bfe0383e/src/compiler/output-targets/dist-custom-elements-bundle/index.ts#L58
• https://github.com/CODE-AND-MORE/stencil/blob/7c4d13be238fe76abc097c07ddc7b305bfe0383e/src/compiler/output-targets/dist-custom-elements-bundle/index.ts#L140-L145

Answering for your last question, is this necessary to have "external" option in Stencil - if you want to solve second case, that is a must, as it cannot be solved by a rollup plugin. But even for the first case, adding external will make my life (and likely other developers) easier and cleaner.

[rwaskiewicz]

Thanks @MarkChrisLevy! Would you do be a favor and create the FR(s) in that case so we can properly split them off from this issue?

[MarkChrisLevy]

@rwaskiewicz Done - #3576

[TRMW]

Both the default dist output target and dist-custom-elements-bundle export a defineCustomElements function that you can use to register all components included in a Stencil library. From what I can tell, the dist-custom-elements target currently does not. This would be nice to have as a convenience method for consumers of our design system, and would make migrating off of custom-elements-bundle more seamless for anyone currently using this function.

I see that dist-custom-elements does have a autoDefineCustomElements option, but I believe this only includes all child components of a single imported component.

I'm wondering if anyone could respond to this. It would definitely be easy to migrate consumers of our component library from the deprecated custom elements to the new one if the new one also provided a defineCustomElements method.

I've also noticed that the docs for the non-deprecated dist-custom-elements target does not show examples of how end consumers would go about defining individual custom elements from this target. We can of course provide our own examples in our migration guide, but it would be great if we could just link our end consumers to a section on this page.

[rwaskiewicz]

Both the default dist output target and dist-custom-elements-bundle export a defineCustomElements function that you can use to register all components included in a Stencil library. From what I can tell, the dist-custom-elements target currently does not. This would be nice to have as a convenience method for consumers of our design system, and would make migrating off of custom-elements-bundle more seamless for anyone currently using this function.

I see that dist-custom-elements does have a autoDefineCustomElements option, but I believe this only includes all child components of a single imported component.

I'm wondering if anyone could respond to https://github.com/ionic-team/stencil/issues/3136#issuecomment-1130718870. It would definitely be easy to migrate consumers of our component library from the deprecated custom elements to the new one if the new one also provided a defineCustomElements method.

@TRMW someone on the team is actively looking into this. We can't promise it would be an exact 1:1 match with the behavior of dist-custom-elements-bundle, but we do see the value in giving folks an easier migration path here.

I've also noticed that the docs for the non-deprecated dist-custom-elements target does not show examples of how end consumers would go about defining individual custom elements from this target. We can of course provide our own examples in our migration guide, but it would be great if we could just link our end consumers to a section on this page.

We also have a docs revamp planned for this output target in the coming weeks 🙂

[TRMW]

@rwaskiewicz This is excellent news! Looking forward to both of these updates. :)

[rwaskiewicz]

Hey folks,

Stencil 3.0.0 was released yesterday, officially removing dist-custom-elements bundle.

We're going to be doing the design work for https://github.com/ionic-team/stencil/issues/3576 in an upcoming sprint. In the mean time, I'm going to close this RFC out. Thank you so much for your participation/discussion! It really helped shape how v3.0.0 came out. I appreciate it 💙

[tfrijsewijk]

@rwaskiewicz:

Stencil 3.0.0 was released yesterday, officially removing dist-custom-elements-bundle.