sveltejs / svelte

web development for the rest of us
https://svelte.dev
MIT License
79.76k stars 4.23k forks source link

Add a check before defining the component in the registry #7595

Closed mohamedMok closed 2 years ago

mohamedMok commented 2 years ago

Describe the problem

At ADEO group, we are working in a Web-component and we are using Svelte to build our web component.

Using the config customElement: true we are able to create web component from our Svelte component.

When using the svelte:options tag, we have a web component with the name defined in the tag. <svelte:options tag="my-component" />

Everything works fine so far,

Our library is used with a micro front architecture. Many apps can call the same web component.

And we have this error:

Uncaught DOMException: Failed to execute 'define' on 'CustomElementRegistry': the name "my-component" has already been used with this registry

Describe the proposed solution

A solution that we have on our end is to check if the custom element has been registered. If not, then we register it.

In this file src/compile/render-dom/index.ts: https://github.com/sveltejs/svelte/blob/master/src/compiler/compile/render_dom/index.ts#L572

You could add:

if (!@_customElements.get("${component.tag}")) {
  @_customElements.define("${component.tag}", ${name});
}

Let me know if you want me to contribute.

Alternatives considered

The alternative we found, is to declare the tag as null. And create a file with the check for each web component library.

Importance

would make my life easier

baseballyama commented 2 years ago

I don't think this is Svelte job. Because the way you suggested would hide the error. Then users can not recognize unintentional duplicated component names.

The alternative we found, is to declare the tag as null. And create a file with the check for each web component library.

Yes. I think this is proper way.

mohamedMok commented 2 years ago

Thanks for your reply @baseballyama I'm closing the issue

patricknelson commented 1 year ago

I think this should be reopened (or maybe a new issue created). I'm tinkering with this in version 4 and it looks like this simple API might to come up as a problem again in version 4. So, this doesn't look like it'll work anymore:

customElements.define("custom-element", CustomElement);

Due to the changes merged in #8457, the <svelte:options> for the custom element tag got a bit more complex. It looks like this was handled by creating a create_custom_element() wrapper that creates the custom HTML element which then gets handed off to customElements.define(). This new wrapper function then takes in as its arguments those the options from <svelte:options customElement={...} /> (which now replaces v3's simpler tag= option), thus making the above no longer possible.

So, if you want to use HMR with Vite and custom elements (e.g. noted in #6164 and probably others), you would need to duplicate the options in your <svelte:options customElement=... /> (or forego it entirely) and manually call create_custom_element yourself like below with those options, because otherwise the compiled component (which already contains the below) will repeatedly call customElements.define() (resulting in this error).

import { create_custom_element } from "svelte/internal";

if (!customElements.get("custom-element")) {
    customElements.define("custom-element",
        create_custom_element(
            CustomElement, // the component constructor
            {"camelCase1":{"reflect":true,"type":"String","attribute":"camel-case1"}}, // props_definition
            ["default"], // slots
            [], // accesors
            false // Shadow dom
        )
    );
}

This naturally doesn't feel like the right ultimate solution for v4 (with Vite, the default dev server) since obviously you probably shouldn't be importing from svelte/internals (given the name). 🤔


Edit: Created #8681 to address the above.

baseballyama commented 1 year ago

I think it's good to open a new issue!

patricknelson commented 1 year ago

Alright @baseballyama, created #8681 to address that separate issue instead (i.e. in v4 with the constructor no longer being descendant of HTMLElement and thus incompatible with customElements.define()).

For what it's worth, I ended up here since I had the same error as @mohamedMok (since I wasn't using null as my tag name) and just accidentally discovered #8681 in my journey to address it by manually defining the custom element. 😅 Sorry about that!