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

Dynamic elements implementation <svelte:element> #5481

Conversation

alfredringstad
Copy link

@alfredringstad alfredringstad commented Oct 1, 2020

This PR is based of #2324 and implements a new tag in Svelte called <svelte:element>. This is used to dynamically render a HTML tag where you don't know the type upfront.

Use cases

When rendering data from a CMS you may end up with a structure looking like this:

[
  {
    "type": "div",
    "children": [
      {
        "type": "h1",
        "children": ["Lorem ipsum"]
      },
      {
        "type": "p",
        "children": ["Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nam rhoncus accumsan sollicitudin. Proin accumsan id eros nec placerat. Maecenas ullamcorper ullamcorper nisi."]
      }
    ]
  }
]

The way to render this today is by creating a Svelte component that renders the corresponding HTML tag by defining all of them. However, this adds a lot of boilerplate and unnecessary weight to the compiled bundle. The implemented solution in this PR solves this problem.

Usage

The <svelte:element> takes a special property called tag, which is the type of DOM element you want to render (e.g. div, h1 etc). All other props/attributes are passed through to the DOM element itself, meaning you can use it just like you would use any HTML element.

Example: Passing tag as a string

<svelte:element tag="div">
  This is rendered as a regular div.
</svelte:element>

Example: Passing tag as a variable

<script>
  export let tag = 'h1';
</script>

<svelte:element {tag} {...$$props}>
  This renders as whatever is passed as a prop to this component and forwards any props supplied.
</svelte:element>

Implementation details

The implementation is based on the works of erzr (see PR #3928) as well as the new implementation of the {#key} block. It works with ssr and hydration.

This PR introduces a new Node type called DynamicElement that wraps the regular Element implementation. The Element node has also been extended with a prop called dynamic_tag, which is used instead of the name to render the element, if this is present.

When the tag is changed the element is destroyed and a new one is created. If other props change except the tag, the element is re-used.

The server side implementation of DynamicElement is a copy of the Element implementation, with one important difference. Since we don't know upfront what kind of HTML element this is, we cannot do many of the optimizations/special cases (e.g. handling events in different ways depending on the element type or having textarea content be set both via value and as children). I think these tradeoffs are okay, since most of the time this should be used with more or less "static" content (if you want to have full control of it, you should probably create a Svelte component for this anyway).

Remaining

  • Add to documentation

Copy link
Member

@tanhauhau tanhauhau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've quickly look through the implementation, overall it's great.

I have 2 questions that I'm curious about:

  1. how does bindings, and event listeners works on the <svelte:element>?
    Currently, some of the events or bindings works only for a specific elements, for example: bind:checked and bind:group only works for input[type="checkbox"] and input[type="radio"], bind:value only works for input & etc.

  2. I notice that if the tag variable changed in <svelte:element {tag}>, you'll remove the entire elements + all the child elements and recreate them. I wonder is it possible to reuse the child elements, by swapping the elements only?

also i feel that having the following would be great:

  • test cases that involved dynamic elements + bindings / dynamic elements + event listener?
  • test case that involved using store value as tag? <svelte:element tag={$tag} > though I imagine it should work fine
  • dev warnings + test case for passing non string value or undefined / null to the tag variable.

@AlbertMarashi
Copy link

AlbertMarashi commented Jan 28, 2021

Been testing this and think it's an awesome feature!
One question, Is it possible to apply styles to the dynamic elements? I tried adding a class to the element but it didn't seem to get picked up in the <style> tags

this isn't a major problem however, as I expect most people using this feature to be using CSS-in-JS / stylesheets for the page-builders, arbitrary content renderers that this feature might be used for

@ymatuhin

This comment has been minimized.

@aradalvand

This comment has been minimized.

@firefoxic

This comment has been minimized.

@benmccann

This comment has been minimized.

@DanielPower
Copy link
Contributor

DanielPower commented Mar 16, 2021

Instead, what if Svelte just came with a set of importable values that could be passed into svelte:component? With the solution presented in this PR, we can render an arbitrary HTML element based on the tag passed into <svelte:element> OR we can render an arbitrary component based on the tag passed into <svelte:component>.

What if instead the solution was:

<script>
  import { elements } from "svelte";
  import CustomComponent from "./CustomComponent.svelte";
  let element = elements.h1;
</script>

<main>
  <svelte:component this={elements.h1}>Hello World!</svelte:component>
  <button on:click={() => element = element === elements.h1 ? CustomComponent : elements.h1}>Toggle Element</button>
</main>

This solution is currently possible by creating your own wrappers for each element you want to use this way. But I think implementing it in Svelte would be more convenient and flexible than the svelte:element solution.

@aradalvand
Copy link

@DanielPower What's wrong with svelte:element? It's more semantically correct and it's more intuitive than the solution you're proposing, in my opinion.

@jonatansberg
Copy link

Instead, what if Svelte just came with a set of importable values that could be passed into svelte:component? With the solution presented in this PR, we can render an arbitrary HTML element based on the tag passed into <svelte:element> OR we can render an arbitrary component based on the tag passed into <svelte:component>.

What if instead the solution was:

<script>
  import { elements } from "svelte";
  import CustomComponent from "./CustomComponent.svelte";
  let element = elements.h1;
</script>

<main>
  <svelte:component this={elements.h1}>Hello World!</svelte:component>
  <button on:click={() => element = element === elements.h1 ? CustomComponent : elements.h1}>Toggle Element</button>
</main>

This solution is currently possible by creating your own wrappers for each element you want to use this way. But I think implementing it in Svelte would be more convenient and flexible than the svelte:element solution.

As you already noted this is possible to do in userland. The problem is that it’s not flexible enough.

A solution that requires you to know ahead of time what elements you will be using does not work for arbitrary elements like rendering content from a third party (like a CMS) or use in a library where the library author does not have control of the usage.

@DanielPower
Copy link
Contributor

@AradAral The only issue I have with svelte:element is that it results two different methods depending on whether you're trying to use an element vs a component, when one generic method could handle all cases.

<svelte:element tag={'h1'}>

Versus

<svelte:component this={elements['h1']}>

The latter would require importing an object of elements. But otherwise behaves exactly the same, and also allows using the same feature for both elements and components.

@jonatansberg Objects are also indexable by string. So <svelte:component this={elements[tag]}> would handle arbitrary elements just fine. Is there something I'm missing that makes the svelte:component solution less capable than the svelte:element solution?

@jonatansberg
Copy link

@DanielPower Yes, bundle size. (And support for custom elements.)

@DanielPower
Copy link
Contributor

Here is an example of my proposal: https://svelte.dev/repl/926ffe1260ef444f8dfa1f63f985a1a8?version=3.35.0

I hadn't considered custom elements though. My proposal was based on the assumption that Svelte could just introduce a helper object with each HTML element. But custom elements throws a wrench in that.

@AlbertMarashi
Copy link

AlbertMarashi commented Mar 17, 2021

Instead, what if Svelte just came with a set of importable values that could be passed into svelte:component? With the solution presented in this PR, we can render an arbitrary HTML element based on the tag passed into <svelte:element> OR we can render an arbitrary component based on the tag passed into <svelte:component>.

What if instead the solution was:

<script>
  import { elements } from "svelte";
  import CustomComponent from "./CustomComponent.svelte";
  let element = elements.h1;
</script>

<main>
  <svelte:component this={elements.h1}>Hello World!</svelte:component>
  <button on:click={() => element = element === elements.h1 ? CustomComponent : elements.h1}>Toggle Element</button>
</main>

This solution is currently possible by creating your own wrappers for each element you want to use this way. But I think implementing it in Svelte would be more convenient and flexible than the svelte:element solution.

This unnecessarily complicates the dynamic elements. Let's just stick with svelte:element

What you are suggesting already exists in userland, and it still has the exact same issues which prompted the addition of the svelte:element feature

I'm not going to get into it, but what you're suggesting won't solve anything

@AlbertMarashi
Copy link

There are unaddressed comments and merge conflicts on this PR. Those would need to be addressed before this could be merged

Ignore my question about styles/classes not being picked up by the dynamic elements, it's not critical.

I'm currently using this fork on a page-builder project i'm working on. The feature works flawlessly, and i'm sure the class issue could be resolved at a later stage. I'd rather this get pulled sooner rather than later

@alfredringstad alfredringstad force-pushed the dynamic-elements-implementation branch from 2f6a886 to 54e1c23 Compare March 31, 2021 17:25
@alfredringstad
Copy link
Author

Sorry for keeping you waiting. I have now updated the PR with some added functionality (retaining children when tag type has changed), better warnings, and have added documentation.

@aradalvand
Copy link

aradalvand commented Mar 31, 2021

@alfredringstad Thanks a lot for this!

Personally, I've also encountered certain cases where I want to render a parent element conditionally, that is, when a condition is true I want to have the parent element, and otherwise I don't.

I'm not sure if this would also be possible in this new feature. But I'd propose that when the value of the tag attribute of <svelte:element> is falsy (i.e. null, undefined, empty string, etc.) the parent element is not rendered (so that the children are directly rendered without that parent element).

What do you think about that?

@jonatansberg
Copy link

jonatansberg commented Apr 1, 2021

@alfredringstad Thanks a lot for this!

There are also some cases where you want to render a parent element conditionally, so that when a condition is true we want to have the parent element, and otherwise no parent element. I'm not sure if such a feature is already implemented here but it could also be that when the value of the tag attribute is falsy (i.e. null, undefined, empty string, etc.) the parent element is not rendered (so the children are directly rendered without that parent element).

What do you think about that?

@AradAral You could solve this using a conditional and slots, like this:

<script>
  export let condition = false;
  export let tag = 'a';
</script>

{#if condition && tag}
  <svelte:element {tag} {...$$restProps}><slot /></svelte:element>
{:else}
  <slot />
{/if}

REPL demo

@aradalvand
Copy link

aradalvand commented Apr 1, 2021

@jonatansberg In the scenario I was referring to you don't have slots:

Update: Sorry I was confused for a second, I hadn't looked at the REPL you linked to.
I now just realized that what you meant was we could have a component like that and call it ConditionalWrapper for example, and have that component accept a slot and then render the slot with a wrapper or not based on the condition.

Good idea. Thanks!

@alfredringstad
Copy link
Author

@alfredringstad any ETA as to when you might be able to do that final change?

I'm not sure when I'll have time to get back to this. Renaming tag to this shouldn't take too much time, but extracting a common ancestor class is a bit more tricky. Feel free to continue building on this if you have time.

@aradalvand
Copy link

aradalvand commented Jul 18, 2021

This is a great PR, still waiting for the final changes to be made.

@benbender
Copy link
Contributor

@alfredringstad I've got surely no right to be pushy, but as this is a crucial enhancement to be able to build quality component-libraries on top of svelte, I wanted to repeat the question about the eta? Or if this could be handed over to someone else with the needed knowledge of the svelte-compiler and a bit more time on his or her hands? Would be very much appreciated!

@AlbertMarashi
Copy link

Hi guys, it's been some time since the last update on this. Is anyone able to step in and get this PR merged? As far as I know there's no issues with it.

I'm hating having to use a svelte fork to have this feature in my app.

I'm sure many others are urgently waiting for this release.

@aradalvand
Copy link

@AlbertMarashi
Same. Been waiting for this feature for a long time, honestly...

@frederikhors
Copy link

@AlbertMarashi
Same. Been waiting for this feature for a long time, honestly...

There is no long time concept in OSS.

@benmccann
Copy link
Member

This PR would need to be rebased before it can be merged

@AlbertMarashi
Copy link

Personally, I liked tag better, because bind:this can be used to bind the reference to the element, but that option isn't available on svelte:component (because it can have multiple root elements)

Nonetheless, I'd like to continue with this approach provided that I can still use bind:this in conjunction with this={tag} as it's already been a while since this was initially proposed

@AlbertMarashi
Copy link

AlbertMarashi commented Oct 21, 2021

Anything I can do to help? @alfredringstad @benmccann

Anything that needs to be fixed?

@dummdidumm
Copy link
Member

dummdidumm commented Oct 21, 2021

I think this needs some more cleanup work. I discovered that applying css classes was not working yet for dynamic elements (which I did a quick-fix for), which put me down the path of discovering how the rest is built out, and I feel like there are some quick-fixes in that code which hurt maintainability in the long run. Essentially, both DynamicElement and Element share some common things but that is not made explicit through for example a shared base class, which leads to duplication and some type casts.

@AlbertMarashi
Copy link

I think this needs some more cleanup work. I discovered that applying css classes was not working yet for dynamic elements (which I did a quick-fix for), which put me down the path of discovering how the rest is built out, and I feel like there are some quick-fixes in that code which hurt maintainability in the long run. Essentially, both DynamicElement and Element share some common things but that is not made explicit through for example a shared base class, which leads to duplication and some type casts.

Fine with leaving the any castings, I'd just love to start using this, and I would be happy to create a new PR after this is merged which unifies the DynamicElement / Element with some kind of interface or base element.

@baseballyama
Copy link
Member

Hi.
First of all, thank you for implementing this feature alfredringstad !!
This is a really really convenient feature!

And I started to clean up this PR but I need 1-2-3 weeks due to working resources.
I know many people wait for this feature but I think we should clean it up before merging as the maintainer said.
Therefore I'm doing.

And I found 1 bug that event handlers and attributes are lost when updating <svelte:element>.
So I need to solve this issue before merging it.

<script>
	let tag = 'h1';
	let num = 1;
	let style = "color: red";

	function click() {
		num += 1;
		tag = `h${num}`;
		style = num % 2 === 0 ? "color: red" : "color: blue";
	}
  </script>

<svelte:element this={tag} on:click={click} style={style}>{tag}</svelte:element>

And I would like to confirm that is there a remaining discussion point about this feature?

@AlbertMarashi
Copy link

AlbertMarashi commented Oct 28, 2021

is there a remaining discussion point about this feature

Don't think so, you can ignore all of my comments, happy with whatever the svelte team wants to call the "this" attribute.

I think the community is keen to start using it ASAP

@baseballyama
Copy link
Member

I mean I just would like to confirm that is there any remaining discussion point regarding <svelte:element> specification.

And regarding this attribute, I have no concern because maintainers already discussed it.
#5481 (comment)

@AlbertMarashi
Copy link

@baseballyama don't think so, I think everyone's happy with moving forward.

@baseballyama
Copy link
Member

I created a new PR. (Because I don't have permission to push to this branch)
#6898

I'd appreciate it if you check it!
Thank you🎉

@tanhauhau
Copy link
Member

Close in favor of #6898

@tanhauhau tanhauhau closed this Nov 27, 2021
@lukaszpolowczyk
Copy link

Someone wanted tag={null} to make the element disappear, but the content of the element stay. But this is non-standard and unexpected behavior and the idea was rejected, as I understand it.

I HAVE AN IDEA
Let's allow has this syntax:
<svelte:element tag="svelte:fragment">content</svelte:element>.

We can make it so that tag="svelte:fragment" only works where svelte:fragment is allowed.

What do you guys think?

@AlbertMarashi
Copy link

that makes no sense lukaz. Fragments are not for this

@lukaszpolowczyk
Copy link

@AlbertMarashi No problem, the name can be whatever you want.
This function from slots was associated closest to me, that's why I threw that name.

You guys are a million times better at coming up with names.

@AlbertMarashi
Copy link

AlbertMarashi commented Apr 7, 2022

Just use a <slot> or wrapper/internal component as it's not much repetition.

@lukaszpolowczyk
Copy link

Ok, I was just proposing the name. Someone else at some place asked for such a function, only proposed the syntax tag={null} but that was rejected for some reason.

@baseballyama
Copy link
Member

Someone wanted tag={null} to make the element disappear, but the content of the element stay

The latest PR removes (and prevents to create) an element if value of this attribute is nullish or empty string (null, undefined, "").
(Attribute name tag is now changed to this)

5850155

@lukaszpolowczyk
Copy link

@baseballyama Ok. I have no problem with it.
As the developers of svelte want, they can introduce it in the future under any name, this="svelte:transparentshell" - whatever. :D

I know they're in no hurry for this feature, and they'll come up with a name. :D

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

Successfully merging this pull request may close these issues.

None yet