search

crisward / svelte-tag

MIT License
52 stars 8 forks source link

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

Open patricknelson opened 1 year ago

patricknelson commented 1 year ago

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 commented 1 year ago

🤦‍♂️ 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.

crisward commented 1 year ago

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

patricknelson commented 1 year ago

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

patricknelson commented 1 year ago

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 commented 1 year ago

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) 😊

crisward commented 1 year ago

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