sveltejs / rfcs

RFCs for changes to Svelte
274 stars 81 forks source link

Better composition #12

Closed Rich-Harris closed 4 years ago

Rich-Harris commented 5 years ago

Rendered

arxpoetica commented 5 years ago

Totally love this.

Suggestions in place of expose have been:

map:thing, pass:thing, with:thing, for:thing, each:thing, export:thing

evs-chris commented 5 years ago

Just wanted to drop this here before I forget, but I was thinking about how to achieve a container component like tabs. I was wondering if it would make sense to be able to collect bits of template from a component into sort of mini/child/implicit components that could be passed around. That could allow a unified declarative and programmatic API. It could also maybe open up an avenue for reuse at the template level if the parent template could export these mini components, kinda like partials from ractive.

jamesbirtles commented 5 years ago

Angular has let-thing="thing" for this kind of thing, has let:thing been considered?

PaulMaly commented 5 years ago

I believe we should adopt Ractive's {{yield}} experience. Using Ractive I never had this issue.

Because often I work with Smart TVs I need to have lists controlled via keyboard and use virtualization (eg VirtualList example) to render large data sets quickly. In Ractive I always used {{yield}} or {{>content}} to do that:

<listview>
   <a href="/channels/{{id}}">
         <img src={{img}}> 
         {{title}}
   </a>
</listview>

The main point in this, that item's markup usually no need to be a component at all. It's too simple and has no additional logic.

Ractive's basic example of {{yield}} with exposing parts of context:

<ul>
  {{#each items}}
    <!-- Expose item and index to yield context. -->
    <li>{{yield with . as item, @index as index}}</li>
  {{/each}}
</ul>

So, in Svelte it could be:

<ul>
  {#each items as item, i}
    <!-- Expose item and index to yield context. -->
    <li>
      <slot name="item" with:item with:index={i}></slot>
    </li>
  {/each}
</ul>

One more thing which concerns me is that we lose consistancy with Web Components. Maybe we should use some svelte-specific tag eg <svelte:yield /> or something?

Also, I'm the guy who use the slots API programmatically, actually) One of the strengths of Svelte is that components could be used with any other frameworks. Very often I "sell" Svelte thus. I just tell my clients, "If we write our components on Svelte, they will work everywhere everytime, because it just a vanilla js". But without any way to use slots programmatically via constructor, this strength will be over.

Please, check my example with Svelte in React app: https://github.com/PaulMaly/react-svelte and and same example with additional slot support: https://habr.com/ru/post/420113/#comment_19011011

pngwn commented 5 years ago

But without any way to use slots programmatically via constructor, this strength will be over.

I just don't think this is true. The inability to use slots programmatically doesn't mean you can't use Svelte components with other frameworks, you would just need to take a different approach. I'd like to see some examples that can only be achieved using the programmatic slot API.

PaulMaly commented 5 years ago

@pngwn Seems you didn't look at my examples. In general any example, where React component or markup should be rendered inside of Svelte component. These are hundreds of variations depending on business challenges. And it's the first question, people ask me when I talk about this ability. I believe you've just not enough experience in Svelte's promotion if you never heard these questions.

pngwn commented 5 years ago

I did look at your examples. I see nothing that couldn't be achieved by passing data into the component.

PaulMaly commented 5 years ago

@pngwn Seems you look, but not see. Ok, I give you a quick example here.

Let's imagine that we've awesome Svele modal component with nice animations, which supports overlays, closing by esc and some other cool stuff. So, we want to use this brilliant component in our React app and show login form inside that modal. So, in v2 we can do that simply:

// Modal.svelte
<svelte:window on:keydown="close(e)"/>

<div class:open class="modal" on:click="close(e)">
   <div class="modal-content">
       <slot>
            <p>Modal content</p>
        </slot>
   </div>
</div>
// SvelteModal.js

import React, { PureComponent } from 'react';

import Modal from './Modal.svelte';

export default class extends PureComponent {

  componentDidMount() {
    ...    
   const { open } = this.props;

    this.modal = new Modal({
      target: this.el,
      data: { open },
      slots: {
        default: this.slot
      }
    });
    this.modal.on('state', ({ current: { open }, changed }) => {
        if (changed.open) {
          this.props.onToggle({ open });
        } 
    });
    ...
  }
  ...
  render() {
    return (
      <div ref={el => this.el = el}>
        <div ref={el => this.slot = el}>
          {this.props.children}
        </div>
      </div>
    );
  }
}
// using

import SvelteModal from './SvelteModal.js';

<SvelteModal onToggle={this.modalToggle}>
  <!-- React-driven things -->
  <input name="login">
  <input name="pass">
  <button onClick={this.loginSubmit}>Login</button>
</SvelteModal>

So, how you suppose to solve this case without ability to use slots programmatically? Btw, it's really simple case, in most of the casese everything will be more difficult.

Rich-Harris commented 5 years ago

@PaulMaly like this:

// Modal.svelte
<svelte:window on:keydown="close(e)"/>

<div class:open class="modal" on:click="close(e)">
-   <div class="modal-content">
+   <div bind:this={div}>
-       <slot>
-            <p>Modal content</p>
-        </slot>
   </div>
</div>
// SvelteModal.js

import React, { PureComponent } from 'react';

import Modal from './Modal.svelte';

export default class extends PureComponent {

  componentDidMount() {
    ...    
   const { open } = this.props;

    this.modal = new Modal({
      target: this.el,
      data: { open },
-      slots: {
-        default: this.slot
-      }
    });
+    this.modal.div.appendChild(this.slot);
    this.modal.on('state', ({ current: { open }, changed }) => {
        if (changed.open) {
          this.props.onToggle({ open });
        } 
    });
    ...
  }
  ...
  render() {
    return (
      <div ref={el => this.el = el}>
        <div ref={el => this.slot = el}>
          {this.props.children}
        </div>
      </div>
    );
  }
}
PaulMaly commented 5 years ago

@Rich-Harris I was thinking about this variant when searching solution in v3, but for me, it looks like a hack. Besides, it could work fine only for this simple example. On serious cases, there will be too much dom manipulations and it won't be pleased by developers who want to use Svelte components in their projects.

Rich-Harris commented 5 years ago

I've released a new alpha version with the features described in the RFC:

https://v3.svelte.technology/repl?version=3.0.0-alpha19&gist=8e68120858e5322272dc9136c4bb79cc

For now I went with @UnwrittenFun's suggestion for the directive name — let:foo. It's easy to type, correctly implies that it creates a new variable in scope, and has been done elsewhere. I can persuaded to change it if people feel strongly.

There was some confusion above — to be clear, the let: directive is used in the parent template, where values are received, not on the <slot> element in the child component. There, you just use regular props.

So you'd have something like this...

<!-- App.html -->
<MyList items={theseItems} let:item let:index>
  <a href={item.url}>{index + 1}: {item.name}</a>
</MyList>
<!-- MyList.html -->
<ul>
  {#each items as item, i}
    <li>
      <slot {item} index={i}></slot>
    </li>
  {/each}
</ul>

@evs-chris

I was wondering if it would make sense to be able to collect bits of template from a component into sort of mini/child/implicit components that could be passed around

Can you elaborate? I've resisted the idea thus far because it makes things quite a bit more complicated — generally I think it's probably preferable to create a separate JS module if you need to be able to import components from a single location:

// my-component-library/index.mjs
export { default as Foo } from './src/Foo.html';
export { default as Bar } from './src/Bar.html';
arxpoetica commented 5 years ago

+1 for let:*

PaulMaly commented 5 years ago

@Rich-Harris Sorry, I'll be a bad guy again, but seems slot scope not working for me:

https://v3.svelte.technology/repl?version=3.0.0-alpha19&gist=cf988d2f5306ed3b7ebb035cc866c9ad

Have I missed something important about let: or it's not implemented yet?

Despite this, thanks for your excellent work!

UPDATE: I found in Discord that seems let: just broken or not ready. Sorry, probably my reaction just was too quick and we need to wait so far this RFC will be stable.

evs-chris commented 5 years ago

Disclaimer: this is not very well thought out, is probably overly complex with terrible ergonomics, and likely just not a good idea™

Regarding mini components, they're probably largely orthogonal to this RFC and may just be poorly thought out poor mans slots. It could also just be a weird bastardization of plain old components. But it was sort of what I came up with when trying to think through how I'd want to implement a tab component that would match how I'd expect to use it. They may also be an artifact of me being stuck in a non-lightweight-component mindset.

Ideally a tab component would behave the same way in either a declarative or imperative context, and svelte is particularly suited to handling the imperative side with inline components. From what I can tell, if you drop the declarative aspect, you're already there - pass in an array of tab objects with titles and components for content and done! But to handle the declarative side, it would be nice to have a way to automatically collect fragments into something accessible from hosting component e.g.

<Tabs>
  <svelte:part into:array="tabs" title="First Tab"><h2>This is the first tab.</h2></svelte:part>
  <svelte:part into:array="tabs" title="Second Tab"><h2>This is the second tab.</h2></svelte:part>
</Tabs>

During component creation, the parts could be gathered into their respective into variable, in this case an array (warning, not well thought out) as opposed to a simple ref (into:ref or into:let). Sort of singular or plural parts, so maybe into:one and into:many? I imagine the host component would then be able to inject the content with another special tag e.g. {#each tabs as tab}<div on:click="() => selected = tab">{tab.title}</div>{#if selected}<svelte:inject {selected.content} />{/if}. The construction of the part would be scoped to its owner template via closure, and any props that are declared can be provided at the use site e.g. <svelte:part into:var="somePart" let:foo>{foo}</svelte:part> ...later... <svelte:inject {somePart} bind:foo={bar} />. Given that, it should be possible to take any old part and pass it to any component that happens to be attached to the same document (or equivalent in ssr terms) to be rendered there. There's a bit of hand-waving here regarding how a part is created e.g. are they constructors or instances when collected, but I imagine constructors with a way to specify that a part should only be created once would be preferable.

Since tabs is a bit of state that belongs to the container component, it can reasonably have an addTab method that takes a part and maybe some options like index to mutate the tabs array to have a new tab appear. Likewise, a removeTab could remove a given tab or the tab at a given index.

Up to here, parts have been passed as component content, but you could probably also split up a larger template into parts to make it a little easier to deal with app-view types of components (as opposed to widgets). I have found that it's easier to track

<Tabs>
  <Tab title="Customer">{{>customer}}</Tab>
  <Tab title="Reference">{{>reference}}</Tabs>
</Tabs>

<template id="customer">
  <!-- giant form thing
    ... three screens later -->
</template>

<template id="reference">
  <!-- moar giant form thing
     ... two screens later -->
</template>

than

<Tabs>
  <Tab title="Customer">
    <!-- giant form thing
       ... three screens later -->
  </Tab>
  <Tab title="Reference">
    <!-- moar giant form thing
      ... two screens later -->
  </Tab>
</Tabs>

Again, not really well thought out, and maybe not a common issue, but I do lots of time in line-of-business apps that have ridonculous amounts of stuff in their views.

I also imagine that a part/mini component could compile to its own web component when targeting web components, which would maybe reduce some friction between the two targets?

timhall commented 5 years ago

I’m not sure if this was covered, but since context sort of becomes part of a component’s API (it expects certain context values have been set), can/should it be passed into the constructor?

const Tabs = Symbol('tabs')

new App({
  context: {
    [Tabs]: ...
  }
});

The main issue with this being only string and symbols are allowed, which goes counter to {} and other acceptable keys (Map vs object). I suppose you could pass in a Map as the context value to avoid that issue.

const Tabs = {}

new App({
  context: new Map([
    [Tabs, ...]
  ])
});

I can imagine fairly straightforward ways around not having context available on the constructor (just define it in the top level component instead of passing it in), just wanted to put this out there.

Rich-Harris commented 5 years ago

Is there a situation in which App knows what to do with that context but couldn't accept it as a prop (and then do setContext(whatever, prop))?

chris-morgan commented 5 years ago

let:foo={bar} feels back to front to me. When reading through the RFC text, and let:item was explained to be shorthand for let:item={item}, I started wondering which side was the value being bound, and which the name to bind it to. I stopped and considered this for twenty seconds or so before continuing, to see whether the conclusion I came to at that point would be correct or not; the conclusion I came to was that let:foo={bar} would be like let foo = bar;, that the left hand side would be the local name, and the right hand side the value it was being assigned. This lines up with variable declaration in general, and with passing properties to child components, and with how the contents of {} is consistently an expression and not a pattern (is that the right term in JS? I mean the left hand side of a let statement: an identifier or a destructuring pattern).

However, I was wrong. On further consideration I can grudgingly see reasons why it would be done the other way, and allowing destructuring would also kinda require that the local bindings be the right hand side rather than the left.

Still, it’s a data point. let: is misleading, and pairing it with {} on the right-hand side for a pattern instead of an expression is anomalous in Svelte, to the best of my knowledge.

Rich-Harris commented 4 years ago

Going to merge this in, since we long ago settled on let: and the setContext/getContext API.

Re the suggestion to allow context as an initialisation prop — it certainly could be done, though I haven't encountered a use for it yet, so it's probably best filed under YAGNI.