Skip to content

Conversation

@trueadm
Copy link
Contributor

@trueadm trueadm commented Aug 9, 2024

Fixes #12776. We always need to do the check for the style, using the key to avoid doing this won't work for cases where the styles are added in their own shadow DOM – as they always need to be added.

@changeset-bot
Copy link

changeset-bot bot commented Aug 9, 2024

🦋 Changeset detected

Latest commit: a09f81f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
svelte Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@Rich-Harris
Copy link
Member

What if we still used the seen mechanism for non-custom elements? Otherwise there's a lot of unnecessary DOM querying going on

@trueadm
Copy link
Contributor Author

trueadm commented Aug 10, 2024

What if we still used the seen mechanism for non-custom elements? Otherwise there's a lot of unnecessary DOM querying going on

Good point. Adjusted.

@lassebomh
Copy link

lassebomh commented Aug 12, 2024

If customElement is set to false, this would still make append_styles think that the styles already has been added (in prod) when mounting a web component.

I might be off, but inside queue_micro_task the target is set to head/root depending on whether the root is a shadow root. What if the seen check was moved into queue_micro_task, like this?

export function append_styles(anchor, css) {
	queue_micro_task(() => {
		var root = anchor.getRootNode();
		const is_shadow_root = root instanceof ShadowRoot; // Add this line

		// Move the seen check here and use the check:
		if (!DEV && !is_shadow_root) {
			if (seen.has(css)) return;
			seen.add(css);
		}

		// ...and here
		var target = is_shadow_root 
			? (root)
			: (root).head ?? (root.ownerDocument).head;

Then the is_custom_element param wouldn't be necessary, and there wouldn't be a difference between dev and prod, if customElement is set to false.

Thanks a lot for responding quickly to the issue.

@Rich-Harris
Copy link
Member

That would defeat the object, which is to avoid creating unnecessary micro tasks

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

Successfully merging this pull request may close these issues.

Web component styles missing when mounted more than once

4 participants