Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
mohamedMok opened this issue Jun 8, 2022 · 5 comments
Closed

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

mohamedMok opened this issue Jun 8, 2022 · 5 comments

Comments

@mohamedMok
Copy link

mohamedMok commented Jun 8, 2022

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
Copy link
Member

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
Copy link
Author

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

@patricknelson
Copy link

patricknelson commented May 31, 2023

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
Copy link
Member

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

@patricknelson
Copy link

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants