Incorrect validation for void svelte:element content presence

[anatolipr]

Describe the bug

with the fix for #7449 in 3.48.0 there was a new validation included which doesn't work correctly. The validation prevents content inside a void svelte:element and produces <svelte:element this="input"> is self-closing and cannot have content. if there is any content in the tag, including conditional ones, making it impossible to create universal code which dynamically contain content. For example this code is now considered invalid:

<svelte:element this={tag.type}>
    {#if tag.type !== 'input'}{tag.content}{/if}
</svelte:element>

although the above if statement will correctly not produce content.

Reproduction

See https://svelte.dev/repl/931558d3b0564db79064da1fedf0aa38?version=3.48.0 with a reproduced error.

Expected behavior - validation should either be optional or evaluate conditional content.

Current behavior - the fact that the content is not present is still considered as present.

Logs

No response

System Info

Any system, browser

Severity

annoyance

[baseballyama]

I discussed this with the team when I create the previous PR.
#7453 (comment)

At that time we thought that we don't want to handle this.
I think you can avoid this error if you wrap by {#if} statement.

So now I close this issue.
But if someone describes more use cases and why this is useful, we can re-open this issue and we can think about what should we do.
Thank you!

[anatolipr]

Thanks for looking at this @baseballyama. In my use case the app uses universal code to render an UI defined in a data structure (JSON) where I have various elements defined as data eg select, input, h1 etc. The workaround will have to be to have a duplicate code for the <svelte:element> block which will be an annoyance as all attributes are the same for both eg. select and input and handling this in an {#if} will make my code require more maintenance in the future. I am ok to do the workaround but as I stand my code is way less than what it will be with switching to 3.48 and up.

[baseballyama]

@anatolipr

To create 1 wrapper component is not enough like below?.
(This is sample code but if we need to create just 1 wrapper component, personally this is the acceptable solution)
AFAIK, validation of this issue in Svelte runtime is a bit expensive process.
One of the important features of Svelte is performance, so I would avoid adding additional processes in runtime.
So If userland has a reasonable solution, it's better I think.

But if we need to create a lot of components due to this issue, I think we need to think about solving this issue.
So once I would like to know about it.
Thank you!

<!-- THIS IS JUST SAMPLE -->
<script>
	export let tag = "input";
	export let attrs = { value: 'bar' }
	$: isVoidTag = ["area","base","br","col","command","embed","hr","img","input","keygen","link","meta","param","source","track","wbr"].includes(tag);
</script>

{#if isVoidTag}
	<svelte:element this="{tag}" {...attrs}></svelte:element>
{:else}
	<svelte:element this="{tag}" {...attrs}>something</svelte:element>
{/if}

[anatolipr]

@baseballyama I am going with what you suggested. The only main difference in my case is that my attrs isn't JSON/function returning object, but instead a set of bindings to some values both provided by the current element context and some stores.

Example of one of my use cases:

{#each nodes as node,i}
<svelte:element 
     on:dblclick|stopPropagation={() => dblclk(i)}
     on:click={(e) => $edit ? e.preventDefault() : undefined}
     class:outline={$outline || (pid + i + '/' === $plane && $showPlaneOutline) || (pid === $plane && $highlight === i)}
     class:lock={$edit && node.lock}
     class:inv={!$outline && $visibleTags && $visibleTags.length
         && node.tags !== undefined && node.tags.length
         && (!$visibleTags.some(elem => node.tags.includes(elem)))
         }
     class:notSelectable = {$edit}
     class:_move={!grouped && pid === $plane 
                             && $hasMoved 
                             && ($selection.indexOf(i) > -1 || $selected/1 === i)}
     id={pid === $plane ? i : undefined}
     data-index={pid === $plane ? i : undefined}
     data-isparent={node.nodes ? '' : undefined}
     data-id={pid+i}
     style="{node.apz === 'static' ? '' : (node.apz ? `position:${node.apz};` : 'position:absolute;')}
                  top: {node.y}px;
                  left: {node.x}px;
                  {node.auwd ? '' : `width: ${node.w}px;`}
                  {node.auht ? '' : `height: ${node.h}px;`}
                  {node.ovf && node.ovf !== 'visible' ? `overflow: ${node.ovf};` : ''}
                  {node.css}">...</svelte:element>
{/each}

I agree about the complexity around solving this problem at runtime. I also agree that no features added to Svelte should compromise it's performance.

I was just wondering if there's perhaps a way to introduce a compiler option or a runtime switch (attribute of sorts) to turn the validator on/off. (behavior prior to the bug fix in 3.48). But if this is adding too much additional work to the team, I am ok with refactoring all my code and moving the above attributes template code to a function which I call with a spread operator. Some of the attributes will still remain inline - eg on: attributes, but these aren't so much to duplicate.

Lastly, I thought it's good to mention a bit more of my use case as I think that it is probably very different than most people's usage of svelte:element. The above markup is only used as a browser preview for users in and not intended as the final result. The tool I am building is a WYSWYG editor of sorts with export functionality. The end result is markup created by the export code (separate implementation from Svelte). This export code deals correctly with void tags markup, formatting and more.

Thanks again for spending time on the above. I really appreciate it.

[macmillen]

@baseballyama I am also having this issue and it's particularly bad if you have multiple actions, event listeners and directives on the same element. I needed to refactor everything into one Svelte Action which is suboptimal. This is how it looked before:

  {#if isVoid(htmlBlockData.type)}
    <svelte:element
      this={htmlBlockData.type}
      use:applyAttributes={htmlBlockData}
      data-__id={htmlBlockData.__id}
      on:mousemove|stopPropagation={() => setHoveredBlock(block)}
      on:mouseleave|stopPropagation={() => setHoveredBlock(undefined)}
      use:selectable
      on:click|preventDefault|stopPropagation={() => selectBlock(block)}
      class:__selected-frame={htmlBlockData === $selectedBlock}
    />
  {:else}
    <svelte:element
      this={htmlBlockData.type}
      use:applyAttributes={htmlBlockData}
      data-__id={htmlBlockData.__id}
      on:mousemove|stopPropagation={() => setHoveredBlock(block)}
      on:mouseleave|stopPropagation={() => setHoveredBlock(undefined)}
      use:selectable
      on:click|preventDefault|stopPropagation={() => selectBlock(block)}
      class:__selected-frame={htmlBlockData === $selectedBlock}
    >
      {#each htmlBlockData?.children ?? [] as id}
        <svelte:self htmlBlockData={htmlBlockMap[id]} />
      {/each}
    </svelte:element>
  {/if}

[baseballyama]

I talked with the team, and we will consider how to handle it.

[anatolipr]

Hey @baseballyama do you have an update for this issue?

[Conduitry]

As of 3.51.0, this is now a runtime dev warning rather than an error - https://svelte.dev/repl/931558d3b0564db79064da1fedf0aa38?version=3.51.0

[macmillen]

@baseballyama can we throw this warning only when there is actual content present. Right now I also get this warning when there is an #if or #each inside that doesn't actually render anything

[baseballyama]

I don't have a good idea to hundle it.
When I create #7834, at first I thought same way but I couldn't get idea a reasonable way to handle it.
(This is possible but code should be messy)

I think there is workaround on userland isn't it?