vmware-archive / clarity

Clarity is a scalable, accessible, customizable, open source design system built with web components. Works with any JavaScript framework, built for enterprises, and designed to be inclusive.
http://clarity.design
MIT License
6.43k stars 762 forks source link

core: side effect free imports #4672

Closed coryrylan closed 4 years ago

coryrylan commented 4 years ago

One of the challenges of larger applications is embedding parts or functionality of external dependencies that use the same custom elements. Due to the custom element registry being global this can cause collisions when multiple versions are loaded.

Extending a component also can fail if its internal template relies on a globally registered custom element. To solve this a scoped custom elements registry API is needed. Currently this can be accomplished by the work from https://open-wc.org/scoped-elements/ and an active discussion here https://github.com/w3c/webcomponents/issues/716.

For scoped registries to work the imports of the custom element class files must be side effect free and not automatically register. I will need to make a breaking change now to update the import paths to support this behavior. This is important to fix sooner than later as we want to make sure we keep the library future proof to adopt scoped registries when standardize or need to use a shim.

Related issue and discussions:

https://github.com/w3c/webcomponents/issues/716 https://open-wc.org/scoped-elements/

Previous API

/modal
  modal.element.js
  index.js // marked as side effect
// modal.element.js
...
registerElementSafely('cds-modal', CdsModal);
// package.json
{
  "sideEffects": true,
  "name": "@clr/core/modal"
}
// application developer imports
import { CdsModal } from '@clr/core/modal'; // triggers a side effect auto register
import '@clr/core/modal'; // auto registered to registry

New API

To fix the issue each component will need four changes:

  1. A register.js file that imports the component and other dependencies for side effects.
  2. Update the package.json to mark only the register.js as the side effect
  3. Update imports in the storybook examples to use the appropriate side effect import.
  4. Add a new import map to point to the register.js for the esm unit tests
/modal
  modal.element.js
  index.js
  register.js // marked as side effect
// package.json
{
  "name": "@clr/core/modal",
  "sideEffects":[
     "./register.js"
  ]
}
// register.js
// important that dependencies are only registered within the side effect file
import '@clr/core/internal-components/close-button/register.js';
import { CdsModal } '@clr/core/modal';

registerElementSafely('cds-modal', CdsModal);

declare global {
  interface HTMLElementTagNameMap {
    'cds-modal': CdsModal;
  }
}
// importmap.importmap
{
  "imports": {
    "@clr/core/alert": "/base/dist/core/alert/index.js",
    "@clr/core/alert/register.js": "/base/dist/core/alert/register.js",
    "@clr/core/badge": "/base/dist/core/badge/index.js",
    "@clr/core/badge/register.js": "/base/dist/core/badge/register.js",
  }
}
// application developer imports
import { CdsModal } from '@clr/core/modal'; // does not trigger a side effect auto register
import '@clr/core/modal/register.js'; // auto registered to registry
coryrylan commented 4 years ago

When you find someone working on the same issue at the same time on a similar project πŸ˜‚

https://github.com/adobe/spectrum-web-components/issues/679

C-658VsXoAo3ovC

Westbrook commented 4 years ago

Hey Spiderman! πŸ•ΈοΈ

Have you thought about where/how to import side effect full dependencies (sub-elements or the like)?

It seems that those need to be in the class file modal.element.js so that anyone registering CdsModal themselves would not then have a broken shadow DOM. Really pushes the point that the area between two Shadow DOM boundaries is essentially it's own tiny application (microfrontend if you will πŸ˜‰) and that each one has to manage itself. I wonder if this points to the only 100% side effect free future is one where ALL elements leverage their own scoped registry πŸ€”?

coryrylan commented 4 years ago

Yeah, we are already starting to see evidence of this with our system now. I think the strategy we will take is to make the breaking API change to get our users over to using a side effect import as soon as possible. Once we have that, then under the hood, we can swap out the side effect imports for scoped registries. After all elements use scoped registries and are side effect free then the only side effect would be a single import to use the default tag names.

// register.js
import { Tabs, Tab } '@lib/tabs';
customElements.define('lib-tabs', Tabs); // uses scoped registry for its dependencies
customElements.define('lib-tab', Tab);

However if the component depends on slotted elements then the consumer will also need to alias those dependencies as well.

<my-tabs>
  <my-tab></my-tab>
  <my-tab></my-tab>
  <my-tab></my-tab>
</my-tabs>
import { Tabs, Tab } 'tabs';
customElements.define('custom-tabs, Tabs);
customElements.define('custom-tab, Tab);

Because of this, I think there is some room for improvement for tooling. We could make it easier alias a component with all dependencies, including slotted elements which would not be in the scoped registry.

import { CdsTabs, CdsTab } '@clr/core/tabs';
// tabs use scoped registries but also provide a method that will automatically register a new alias for the slotted dependencies like `tab`
CdsTabs.registerTagPrefix('custom');

Unless components have zero dependencies on any other custom elements, scoped registries it seems are inevitable.

download

github-actions[bot] commented 4 years ago

Hi there πŸ‘‹, this is an automated message. To help Clarity keep track of discussions, we automatically lock closed issues after 14 days. Please look for another open issue or open a new issue with updated details and reference this one as necessary.