svelte-tag icon indicating copy to clipboard operation
svelte-tag copied to clipboard

Slot "unwrap" strips tags unnecessarily (when not using shadow root)

Open patricknelson opened this issue 2 years ago • 6 comments
trafficstars

Hi @crisward. I'm investigating a solution to a bug with custom element handling in upcoming Svelte 4 https://github.com/sveltejs/svelte/issues/8686 and I'm taking inspiration from svelte-tag which seems to address my issue (sans the unwrap). I was wondering if you can recall the purpose of this unwrap function? https://github.com/crisward/svelte-tag/commit/1e05665a5b2e7900d45570b30874b3e4791db68f#diff-e727e4bdf3657fd1d798edcd6b099d6e092f8573cba266154583a746bba0f346R86-R92

Here's a REPL demonstrating how it's supposed to work (taken from docs): https://svelte.dev/repl/3e6e3ed3885b4efda808fcacd7263f0b?version=3.59.1 In this example the <h1> and <p> tags remain. For reference, the generated HTML is:

<h1 slot="header">Hello</h1>
<p>Some content between header and footer</p>
<p slot="footer">Copyright (c) 2019 Svelte Industries</p>

This functionality appears to be expected in svelte-tag based on the unit tests (e.g. this test). However, while the result should contain <div><div slot="inner">HERE</div></div>), the unit test expects that interior slotted <div slot="inner"> to not exist. 🤔 REPL showing that here, too: https://svelte.dev/repl/a7414057d404440681ad17f31ba7e536?version=3.59.1

patricknelson avatar Jun 02 '23 02:06 patricknelson

🤦‍♂️ D'oh! I think I just figured it out. I think me writing this issue was like talking to a rubber duck.🦆

Turns out it's useful for default slots but not for the named slots. I'll go ahead and submit a PR for this in a bit.

patricknelson avatar Jun 02 '23 03:06 patricknelson

I'll do an npm release of this. Feel free to close when you're ready.

crisward avatar Jun 03 '23 11:06 crisward

Alright, just reopening for now since technically this issue is still present. Setup PR #15 to address this hopefully once and for all 😅

patricknelson avatar Jun 05 '23 21:06 patricknelson

p.s. While I'm certain this is the correct thing to do moving forward, it might be worth discussing the potential that this could be a breaking change for some folks who may have somehow depended on this functionality, i.e.: where the named <div slot=...> element wasn't introduced into the final result and instead only the contents were injected at the slot point.

So, this could warrant a major version change. 🤔 What do you think?

patricknelson avatar Jun 05 '23 22:06 patricknelson

Fixed in forked package: svelte-retag

Keeping issue + PR open since it's technically still an issue here (in case you still want to merge) 😊

patricknelson avatar Jun 08 '23 01:06 patricknelson

I can see how styling could be effected by this. Will check with my own usage to see what happens.

crisward avatar Jun 12 '23 08:06 crisward