Importing components forces registration without check

[cuberoot]

Expected Behaviour

I should be able to use SWC in two different javascript bundles from a single page.

Actual Behaviour

We get errors when customElements.define is called and I cannot see any way around it.

Reproduce Scenario (including but not limited to)

Build two different code bundles that include SWC components. Try to load them into the same page.

Steps to Reproduce

Unpack the sample code below and then do:

yarn install
yarn build
open index.html

Platform and Version

Sample Code that illustrates the problem

swc-collision.zip

Logs taken while reproducing problem

Console logs:

Uncaught DOMException: Failed to execute 'define' on 'CustomElementRegistry': the name "sp-icon" has already been used with this registry
    at eval (webpack:///./node_modules/@spectrum-web-components/icon/sp-icon.js?:15:16)
    at Module../node_modules/@spectrum-web-components/icon/sp-icon.js (file:///Users/mitaylor/Scratch/swc-collision/dist/library_b.js:889:1)
    at __webpack_require__ (file:///Users/mitaylor/Scratch/swc-collision/dist/library_b.js:20:30)
    at eval (webpack:///./node_modules/@spectrum-web-components/dropdown/src/Dropdown.js?:13:98)
    at Module../node_modules/@spectrum-web-components/dropdown/src/Dropdown.js (file:///Users/mitaylor/Scratch/swc-collision/dist/library_b.js:865:1)
    at __webpack_require__ (file:///Users/mitaylor/Scratch/swc-collision/dist/library_b.js:20:30)
    at eval (webpack:///./node_modules/@spectrum-web-components/dropdown/sp-dropdown.js?:2:74)
    at Module../node_modules/@spectrum-web-components/dropdown/sp-dropdown.js (file:///Users/mitaylor/Scratch/swc-collision/dist/library_b.js:853:1)
    at __webpack_require__ (file:///Users/mitaylor/Scratch/swc-collision/dist/library_b.js:20:30)
    at eval (webpack:///./library_b.js?:3:103)

[cuberoot]

@Westbrook I am looking at the spec for the custom registry and I don't think that would fix this problem with SWC imports. In my example, I imported @spectrum-web-components/dropdown/sp-dropdown. But, the same problem would happen if I had imported @spectrum-web-components/dropdown/Dropdown, because it imports a whole bunch of other auto-registering components.

So, I could not import Dropdown.ts and add it to a custom registry without registering a whole bunch of SWC components in the global registry as side effect.

[Westbrook]

In the nearest term, I'm unsure whether there's a more realistic answer to this situation than; don't attempt to ship two individually bundled applications on the same page. The most zen/happy place for web components delivery at large is in a completely unbundled context where the module graph is being managed and deduplicated by the browser and no matter how many "applications" are on the same page their dependencies are resolved and shared without stepping on each other. This, I understand, is not always possible, and even with technologies like HTTP2+ and CDN and smart edge delivery it is not always going to support the highest quality delivery of an application to an end-user, though it's getting closer and closer. (I'll leave the benefits of unbundled delivery of applications in conjunction with preload/modulepreload and service workers for another day, but don't sleep on it!)

Managing this context, as is...
At current, shipping two applications on the same page that are bundled in some way requires the two applications to have some contextual understanding of each other. Listing the two "applications" as individual entry points in a larger application is likely the best way to enable this with the tooling available to use today. I've heard that there are some new "federated apps" techniques available in the most recent version of Webpack, and while they seem quite interesting, they're by no means required to make this possible. Listing two different entry points in a more standard Webpack/rollup config would make this possible as well. This does involve building both applications from the same source, but a number of strategies more than adequately manage this context, e.g. monorepos, workspaces, etc.

Why not if(!customElements.get('my-el') {?
While we had used this technique in the past, there are a number of realities that come clear after relying on it for a long period. First, it will 100% prevent multiple registries of the same component in a page that it receiving multiple bundled applications, each with that component, and 100% prevent errors in the second registration, so it can be quite enticing. In the case that those elements are the same version, great! You've now shipped two 100% working applications, with only the small cost of sending that JS over the wire twice. In the case that they are not, you've now created a really quiet bug that is in the very last place you will look that prevents your application from working exactly the way that you expect it to. Hopefully, the two versions are compatible, which saves you a lot of headaches, but even if they are which element is actually registered relies on code order and load/parse timing which means that if you expect new features in one app, but the other app is smaller/closer on the network, it will be very opaque as to why those new features are not available in your application. If they aren't compatible, then you may well still run into a page blocking error that just shows up elsewhere in your code, again making it difficult to trace back to its actual origin. Beyond that, if you allow for shipping code with this sort of gating, it also means that when/if NPM resolution decides to install two versions of the same element into the SAME application, you will once again not receive a site of error failure. Debugging is going to be easier for future you if the errors in your code can be as large and early as possible in the development process.

It could be said that instead, we could rely on a warning gate instead of an error gate, which will certainly prevent deploy time failures, but it won't bring the situation of silent failures to light any earlier as a warning can be easily missed by even the most cautious of eyes or worse ignored as not actually being an error by tired/late/distracted eyes.

What else could we do now?
While Scoped Registries are not available, nor is an API as proposed polyfill, we could work with a tool called Scoped Elements as developed by OpenWC and widely leveraged at ING. This is actually part of the conversation we went into in #679 where I initially proposed the refactor of our element registrations. It is a decent and stable tool, and while I've not had time to do any performance testing of our own with it, I've heard people having good results with it. The problem with this approach (beyond not lining up with the actual specified API) is that the mechanism in lit-html that this tool leverages to make this sort of scoping possible will not be persisted into the next major version of the renderer. That being so, choosing to go this path would lock us into lit-html@1.x until the more compliant polyfill currently in development was complete. Even if the timing did not actually block much, going this path today would require retrofitting our components for two different polyfills over the next quarter or two.

What if we are not in a hurry?
The more compliant polyfill would in theory allow for both our library and consuming application to position themselves in a place where once the API was finalized and implemented (in web components time, maybe 5 years 😞, with Adobe yelling about it, maybe less!) we'd be able to just flip the switch and consume the native version 🤞. There are a number of what I would call serious outstanding questions about the API (I'd need to dive back through the conversations we had at last months TPAC meetings to fully review that here, but I'm particularly worried about the likelihood that inheritance is skipped in order to get the spec approved earlier), so the timeline and stability of the currently proposed API could be in similarly serious question. However, I'd still say that waiting for this is the best possible solution in an "app have to be bundled" world.

What if we went big?
I could see a possible future where we took a completely different path and pivoted away from a "pass as you go component library" and went more towards a "component framework" and did a lot more work around component resolutions and injections in our own code on top of the platform APIs to reduce, and hopefully eliminate, these sorts of collisions. I've not ideas right now on what that might even look like, but in particular, I think that going this direction would start by looking at the "rely directly on the ES Module graph" approach that we take right now and require the formulation of our own graph resolution strategy. No telling if it would be successful, or worth the work, but I want to make sure that I'm not leaving options off the table here if anyone is interested in pursuing or requiring certain research to be done.

How do we find the right path forward?
My team will likely be looking into the idea of embedding in the new year, which will need to spend some time directly looking at the realities that arise in the context of this functionality. How do we embed two features on the same page? My early goal is to lean on a more HTTP2/CDN approach, and it might be the sort of solution that more consumers could leverage. Already a number of consumers have asked for NPM-free usage patterns of our components. Maybe there's reason to expand on that request beyond the UNPKG and JSPM links I've responded to this query with to date? Maybe doubling down on one of these scoped solutions would be correct, or going all the way to a component framework approach? Maybe there are other options that I don't know about/can't think about that should be investigated?

Please chime in below so we can find a path forward to this situation that is productive for everyone...

[cuberoot]

A simple path that occurs to me is to bake the version of a component into itself. Then, instead of just trying to register a component blindly. Check to see if the component is already registered and if the version of the registered version matches the version that wishes to now register.

[cuberoot]

Note that I did find the Scoped Elements tool in my research. I thought of using that in one of the two apps. However, as noted, I cannot import the raw components (e.g. Dropdown) without registering other components as a side effect.

[Westbrook]

To clarify my suggestions above, only the "Managing this context, as is..." section really speaks to options that would solve this from the outside. Scoped Elements would indeed not be something you could do at the app level today to avoid this situation. We could, today, prioritize a refactor in this way, if we needed to make immediate changes to support consumers. I outline my understanding of that decision in the space not specifically to dissuade that course of action but to clarify the costs of such a decision.

Another option from the outside, a consuming app could leverage all dynamic imports and only import dependencies that aren't already registered. This would not be a "production app" pattern, but in a "test harness" use case, a test harness could be refactored to support this relatively directly.

I kind of felt managing a more nuanced registration check leant towards the "Going big" section in that we're talking about custom checks that we'd not only need to fully document, but would need to build the management of into our tooling. What would you like to see here to feel like we've clarified a path that was both approachable from a scale/usage point of view as well as maintainable long term?

[cuberoot]

At the high-level, I sort of grieve the state of things. As a developer that works on SDK components, this makes it difficult for me to produce a module that contains SWC without it being extremely fragile. What I want to have is to provide multiple higher-order photography components, which use SWC, that you could include on your page via <script> tags. Ideally, you should be able to use more than one. And you should be able to use SWC in your own page as well. I don't currently see a way to do that without complicated dynamic imports that, as you say, is not a great production pattern.

That being said, here's the check I think we should be doing. When a component is imported, instead of blinding registering, check to see if this exact version is already registered. We should be able to bake that in from the package.json, no? If a different version is already registered, throw a more meaningful message.

[Westbrook]

throw a more meaningful message.

Can you clarify whether throw means an actual error? Further, can you share what you think you want to see if version information is available? If an error is involved, I have a hard time seeing how extra information would be specifically more useful than the already registered error that would currently be thrown.

We should be able to bake that in from the package.json, no?

Gathering this information is slightly less complicated than gathering it at the right time. Currently we release versions via an alias to lerna publish which manages the alteration of the system of package.json files as needed and the publication process in a single wrapped command. The research there would be to find whether there are extension hooks for that process to do something like consume the updated version numbers into the component files before publishing the new version to NPM. I have some vague long term thoughts to revisit lerna as a versioning/monorepo strategy, and not supporting something like this would be the right instigator to accelerating that research.

[Westbrook]

@cuberoot realized that a consuming applications can do something about this today, without any intervention from the library by monkey patching customElements.define, which actually centralizes anything that we might do at the individual package level, short of moving 100% to Scoped Elements or Scoped Registries (which is likely the better answer long term when next one of those packages can be counted on as "stable"). How completely dirty and wrong would it feel to leverage the following at the very top of your SDK?

const oldDefine = customElements.define;

customElements.define = function(tagName, Constructor) {
    if (customElements.get(tagName)) {
        console.log(`The custom element '<${tagName}>' has already been defined.`);
        return;
    }
    oldDefine.call(this, tagName, Constructor);
}

Then you have full control over the shape, quality, and even presence of the messaging (which could be particularly useful to shape one way for your test set up and another way for you SDK deployments, etc.) to make it meaningful in your application and could theoretically inject is late, meaning at the beginning of your "second" application, rather than the beginning of everything, in that case that you didn't have full access to making changes at any one time.

This, of course, doesn't touch on version inclusions, so there'd be no way, yet, to report on that, but it could be the sort of thing that allowed you to work out smaller local problems while we worked towards a large scale intervention a little later on.

[dsadhanala]

@Westbrook For spark post UI components we created a simple decorator & mixin, which is used for defining customElements. This decorator actually checks if element is already defined or not, if it already defined and exist on the page, it simply skips registration. example snippet looks something like below. Happy to go over if you want to discuss in detail.

    if (customElements.get(elementName)) return;
    // define as custom element
    window.customElements.define(elementName, targetClass, options);

usage as decorator

@defineElement({ elementName: COMP_NAME })
export class TheoComp extends BaseElement {}

in Non-TS environments, we also added static method on our base element instance, if someone import component class as ES6 module and want to add own CE name, then consumers can do below.

import { TheoComp } from '@package';
TheoComp.defineElement({ elementName: COMP_NAME });

[cuberoot]

Thanks for the above suggestion @Westbrook. I believe that one of the stated reasons for not checking if a component is registered before registering is to avoid a vulnerability where a false component with the same name is registered earlier. I believe that this work-around would also reopen that vulnerability 🙂. Basically, anyone that can register a component can do pretty much anything to your JavaScript context.

I know that the other reason for not checking is because you want potential version mismatches to trigger an error.

I don't mind doing the monkey patching for our own code in the short term. I am leery about monkey patching a 3rd party client's environment where our code is used.

[sijakret]

Great discussion - chiming in..

We had pretty much the same issue in the design system in our organization.
We were also hesitant to fully bank on the scoped registries just yet.

For now we solved the issue via a different path: peerDependencies

As long as you make sure that..

1. components are imported in a way so they only register themselves (i.e. don't lead to registration side effects)
2. your peerDependencies (+devDepenencies) track your deps
3. when using an individual component all of it's deps are also installed and registered

..you and up with a flat dependency 'tree' in your consuming application where you can swap individual components.

Practically we have two ways to import a component:

• @lib/comp-name: pulls in all dependancy comps transitively, so you actually even get compile time errors in case you are missing deps)
• @lib/comp-bame/solo: pulls in only that component. Is required to be side-effect-free otherwise.

This solution does not shoot for locally deviating versions of components but leads to one flat list of comps with the ability to switch some of them out.
So it solves the problem: how can we swap out components in a particular lineup without running into name collisions

If you think about it, peerDependencies are actually intended for exactly this purpose: library patterns where multiple dependencies want to share singular instances of one another.

Sidenote: since npm 7 peer deps are actually installed automatically again which leads to a really transparent behavior of this approach regarding the install behavior. https://github.blog/2021-02-02-npm-7-is-now-generally-available/#peer-dependencies

What do you think about this approach?

[najikahalsema]

Tracking this in #2666