sveltejs / rfcs

RFCs for changes to Svelte
274 stars 81 forks source link

Reactive assignments #4

Closed Rich-Harris closed 5 years ago

Rich-Harris commented 5 years ago

A proposal for simpler, smaller components with a new approach to reactivity.

View formatted RFC

See also the original issue https://github.com/sveltejs/svelte/issues/1826


Due to an unfortunate incident at the commit factory, the original PR for this RFC was merged prematurely. Earlier comments can be read there

timhall commented 5 years ago

Looking great! I would say the biggest confusion / stumbling block for me has been the mix of {...} and strings in templates and this looks like a great time to get rid of the special string cases, which I think are all directives:

Event handlers

<p>Count: {count}</p>
<button on:click={() => count += 1}>+1</button>

Component events

Note: I don't really see the need for createEventDispatcher, seems like something the compiler can sort out.

<script>
  import { dispatch as fire } from 'svelte';
</script>

<p>Select a category:</p>

{#each categories as category}
  <button on:click={() => fire('select', { category })}>select {category}</button>
{/each}

Refs

<script>
  let canvas;
</script>

<canvas ref={canvas}></canvas>

Transitions

<script>
  import { fade, fly } from 'svelte-transitions';
</script>

{#if visible}
  <p in:{fly}={{ y: 50 }} out:{fade}>fades in and out</p>
{/if}

Bindings

<Widget bind:childValue={parentValue} />

<Widget bind:{value} />

Actions

<button on:click="toggleLanguage" use:{tooltip}={translations[language].tooltip}>
  {language}
</button>

Classes

<ul class="links">
  <li class:active={url === '/'}>...</li>
  <li class:active={url.startsWith('/blog')}>...</li>
  <li class:active={url.startsWith('/about')}>...</li>
</ul>

It's a small thing in some cases, but I think consistency will be a big win overall and make clear that if you need to access / evaluate js in templates, use { ... } tags

timhall commented 5 years ago

As for an approach to store, I think leaning on reactive assignments would work very well:

<script>
  import { store } from 'svelte';

  let { online = navigator.onLine } = store();
  window.addEventListener('offline', () => online = false);
  window.addEventListener('online', () => online = true);
</script>

<p>You are {online ? 'online' : 'offline'}</p>

Compiles roughly to:

<script>
import { store } from 'svelte';

const Component = defineComponent((__update, __props) => {
  // (like lifecycle function, get store for current instance)
  const __store = store();

  // This part is a little icky, may be a better way to do this
  let { online } = __store.get();
  if (online === void 0) {
    online = navigator.onLine;
    __store.set({ online });
  }

  __store.on('state', values => {
    online = values.online;
    __update({ online: true });
  });
  window.addEventListener('offline', () => {
    online = false;
    __store.set({ online });
  });
  window.addEventListener('online', () => {
    online = true;
    __store.set({ online });
  });
}, create_main_fragment);
</script>

The above should satisfy the current store needs, although there is no built-in $... in templates, which is a bit of a shame (although arguably less magic). This could be take a step further and generalized to support component context and the idea of connecting to observables / subjects = Context (πŸƒ).

interface Context<T> {
  get(): T;
  set(T): void;
  subscribe(callback: (T) => void): () => void
}
<script>
  import CustomStore from './custom-store';
  import { connect, context } from 'svelte';

  let { online = navigator.onLine } = connect(context(CustomStore));

  // connect(context(...)) is most likely redundant, context(...) should be enough
  // but connect(...) is nice for values not from context (e.g. Observables)

  window.addEventListener('offline', () => online = false);
  window.addEventListener('online', () => online = true);
</script>

Compiles roughly to:

<script>
import CustomStore from './custom-store'; 
import { context, ondestroy } from 'svelte';

const Component = defineComponent((__update, __props) => {
  let online = navigator.onLine;

  // (like lifecycle function to find value in current instance's context)
  const __CustomStoreContext = context(CustomStore);

  let __values = __CustomStoreContext.get();
  let { online } = __values;
  if (online === void 0) {
    online = navigator.onLine;
    __CustomStoreContext.set({ ...__values, online });
  }

  const unsubscribe = __CustomStoreContext.subscribe(values => {
    online = values.online;
    __update({ online: true });
  });
  ondestroy(unsubscribe);

  window.addEventListener('offline', () => {
    online = false;
    __CustomStoreContext.set({ online });
  });
  window.addEventListener('online', () => {
    online = true;
    __CustomStoreContext.set({ online });
  });
}, create_main_fragment);
</script>

Maybe this is a bit too much power (mainly the connect bit), but it seems to expand into a fairly minimal compiled output and would make it much easier to pass around observable values (especially if props and store were built on this primitive).

arxpoetica commented 5 years ago

@timhall We've had the debate before about perma-removing single or double quotes from HTML. I favor the current standard of optional. I personally find it easier to scan, plus some editors don't always place nice with certain characters.

timhall commented 5 years ago

I’m fine with the quotes, it’s the string vs tag issue that I was mainly addressing, where there are special cases where you can pass js expressions as a string instead of a tag ({...}). To make it more confusing, there are some cases where tags aren’t allowed (actions, I think).

Unfortunately, this leads to the quote/unquoted debate since β€œ{ ... }” is best supported by editors, but is this a string with an tag inside? and, if so, should it be interpreted as the value’s type or a string? I’m sure this has been debated (and decided) previously, so I picked the unquoted form to try to express the string vs tags issue.

mrkishi commented 5 years ago

The above should satisfy the current store needs, although there is no built-in $... in templates, which is a bit of a shame (although arguably less magic). This could be take a step further and generalized to support component context and the idea of connecting to observables / subjects = Context (πŸƒ).

interface Context<T> {
  get(): T;
  set(T): void;
  subscribe(callback: (T) => void): () => void
}

Nice, that's inline with the repl I created the other day.

ryanatkn commented 5 years ago

For stores, I'll echo the above comments, that I like both reusing reactive assignment and having something generic like React's context that gives descendant components at any depth a way to access stuff (including stores) provided by an ancestor.

As a thought experiment, could a store just be a component? Like all components, they have internal reactive assignment semantics and an exported contract to the world. They could be invisible wrappers with just a <slot> or render whatever they want. They could be imported by any descendant via context, maybe with an API like React's useContext. The idea of a store would become a convention in a system where descendant components can load any contexts provided by ancestors.

This seems weird and I don't know all of the tradeoffs and implications. I like the outcome of removing the standalone store concept while giving components more powers for composition.

A possible API would have descendants directly import the ancestors they want. This seems simple and friendly to type systems, but implies that circular deps need to work.

<script>
  import { getContext } from 'svelte';
  import CornerStore from './CornerStore.html';

  let cornerStore; // type inferred as `Context<CornerStore>` or just `CornerStore`
  getContext(CornerStore, c => cornerStore = c);
</script>

So in React terms, the context object, the value returned from React.createContext that's used to declare which type of context you want, would just be the imported component, and components provide a context of their type to descendants, either implicitly or explicitly. I can't tell if that's an unfortunate restriction or another good unifying simplification.

Rich-Harris commented 5 years ago

The success of these ideas hinges on there being as little compiler magic as possible. Currently, there are two bits of magic:

The reaction we've had so far suggests that people are on board with both of those rules. But if we start adding more rules β€” such as changing the meaning/behaviour of specific function calls β€” we'll quickly find ourselves in the uncanny valley, and people will struggle to understand what is happening to their code.

That's why we need createEventDispatcher instead of just dispatch (or fire if you prefer). In this code...

import { dispatch } from 'svelte';

setTimeout(() => {
  dispatch('hello', { name: 'world' });
}, 100);

...there is no way to know which component fired the event without doing something weird like this:

setTimeout(() => {
-  dispatch('hello', { name: 'world' });
+  dispatch('hello', { name: 'world' }, __self);
}, 100);

The same logic applies to connect and context β€” I think it becomes a lot harder to understand how data is flowing around the system. I think we're in a much better spot if we rely on JavaScript itself, even if it means typing slightly more:

let friends;

const listener = someThirdPartyAPI.subscribe(id, value => {
  friends = value;
});

ondestroy(() => {
  listener.unsubscribe();
});

It's easy enough to wrap that with a helper function, if you need to use it in multiple components:

import { ondestroy } from 'svelte';

function subscribe(id, callback) {
  const listener = someThirdPartyAPI.subscribe(id, callback);
  ondestroy(() => listener.unsubscribe());
}

// used like so
let friends;
subscribe(id, value => friends = value);

In fact we could implement an equivalent of 'context' in the same way:

function createContext(start, value) {
  const callbacks = [];
  let stop;

  function set(newValue) {
    value = newValue;

    callbacks.forEach(callback => {
      callback(value);
    });
  }

  return {
    subscribe(callback) {
      if (callbacks.length === 0) {
        stop = start(set);
      }

      callbacks.push(callback);
      callback(value);

      ondestroy(() => {
        const index = callbacks.indexOf(callback);
        callbacks.splice(index, 1);

        if (callbacks.length === 0) {
          stop();
          stop = null;
        }
      });
    }
  }
}
// src/contexts.js
import { createContext } from 'svelte';

export const online = createContext(
  function start(set) {
    function handler() {
      set(navigator.onLine);
    }

    window.addEventListener('online', handler);
    window.addEventListener('offline', handler);

    return function stop() {
      window.removeEventListener('online', handler);
      window.removeEventListener('offline', handler);
    };
  },
  navigator.onLine
);
<script>
  import { onlineContext } from './contexts.js';

  let online;
  onlineContext.subscribe(value => online = value);
</script>

(I'm using the name 'context' here since it replicates the functionality of context, but really it's just a take on the concept of observables. And I'm not saying I think we should add this to Svelte, just throwing it out there as a possibility β€” it might be better as a non-Svelte-specific thing, i.e. without ondestroy.)


Some version of 'context' is definitely important β€” the difficulties with using multiple stores in a Svelte 2 app have shown that to be the case. It's made more urgent by the fact that you can't declaratively attach a store to an arbitrary component in this RFC; you can only attach a store at the top-level when you do new App({..., store}).

It would be nice to have unification between whatever concept of 'store' and 'context' we land on, but I do think there is still value in the concept of a single app-level store (particularly one that can easily be referenced in markup), not least because it's turned out to be such a useful way to pass data from server to client (such as user data) in Sapper.


I'm interested in discussing the thoughts around changing directive syntax a bit more. There possibly are some inconsistencies at present.

That said my inclination is to avoid going too far with the curly braces β€” I'm not sure I see the advantage of this...

{#if visible}
  <p in:{fly}={{ y: 50 }} out:{fade}>fades in and out</p>
{/if}

...over the status quo:

{#if visible}
  <p in:fly={ y: 50 } out:fade>fades in and out</p>
{/if}

This is a little harder to explain but I think of the contents of {...} as being read-only views onto expressions that could contain any JS β€” there is a significant difference between foo={bar} (which we can think of as foo <= bar) and bind:foo=bar (which we can think of as foo <=> bar). In the first case we could replace bar with a different expression (foo={42}) and it makes total sense, but bind:foo={42} is in a way equivalent to var 42 = foo (which of course is invalid code).

Directives are probably the least well rationalised part of HTMLx though, so I'm open to changes that make them more consistent without harming read/writeability.

tomcon commented 5 years ago

re. new Store & following on from previous discussion: I'm with @tivac on this one. The (relatively) simple & consistent (ie. same api) implementation of Store in v2 won a lot of people over imo - including me! By all means knock yourself out with other stricter store implementations etc but in some parts of the real world it is very useful indeed

lucasrcosta commented 5 years ago

Great piece! Congrats! I think taking a " SvelteScript" path might be a step a away from the opportunity to be something that feels commonplace instead of being it's own thing, though. "Explicit is better then implicit" (Tim Peters) Hooks seems big, but there's time...

arxpoetica commented 5 years ago

I'm also w/ @tivac

evs-chris commented 5 years ago

This looks pretty awesome! Coming from ractive, I feel like a good store is a pretty important thing, so I'm going to babble incoherently a bit.

I really like the above idea of implementing the store as a sort of adapter (Store { get; set; subscribe; }) with a default implementation provided if the component has requested store access (by referencing it) and none was provided by the user. It is a bit magical, but it could be entirely opt-in magic in the form of a single well-defined macro:

// compiler, I'd like a bit of that sweet sweet magic
import bind from 'svelte/store/bind';

// private state
let { something } = bind();

// public state via instance property
export let { other } = bind();

// I wouldn't hate keypath-style bindings, but I don't know how well that works here :)

The bind macro could then expand to create the local state that pulls from the store, subscribe for updates with the subscription managed by the component lifecycle, and set up participation in the __update cycle for the component with the references in the store. Precedent for slightly magical non-functions already exists in ES in the form of import(), and you could still wire things up manually, if you're magic averse. I believe you'd still need a way to inject a store reference into the component, which could just be an import too? import store from 'svelte/store'; where behind the scenes that injects either the provided store or provides a default. Components could automatically have their owner's store, a store supplied via directive, or a default injected.

At this point, referencing a store variable in a template could create an implicit state let, like the implicit shorthand for regular state, if it's not already declared for API purposes. The __update and subscription mechanisms would also need to ensure that they don't cycle, which is another reason that some sugar that handles it correctly for you would maybe be nice.

Edit: I missed the discussion on <svelte:meta /> and bindings, but that looks quite nice for limiting mysterious magic imports, including any inherited store.

kylecordes commented 5 years ago

solve the problem of reactivity at the language level

Even before considering any of the details, embracing this idea is a profound step in the right direction. This is what we need languages to do, rather than to need app code full of wiring to library level reactivity, to increase productivity.

exported values form the component's contract with the outside world

I'm mentally comparing this with similar use of language features that some other frameworks take, in which you provide a class, and the public members of that class become the contract with the outside world. Although classes are kind of out of fashion, this seems like a case where they are worth consideration, fitting the semantics at hand? But...

On the other hand, the class approach often ends up with an extra layers of language construct. Consider what Angular does: If you follow the Angular style guide, you always write just one component in a source file, which is a JavaScript module. So you have the JavaScript module layer acting mostly as boilerplate, with just the class (component) inside specifying an API for the outside world. The approach described in the RFC avoids this, tightening up the syntax and reducing boilerplate. Arguably there is no need for a class to describe a component, because a JavaScript module already is a somewhat class-like construct that describes a component.

createEventDispatcher

What I don't like adore this approach so far, is that it still necessary to actually execute all possible code paths to find out what events a component might emit. Ideally the list (and shape) of emit-able events would be part of the declared public API of a component. The tooling (compiler) would produce reasonably complete metadata about the outside surface of a component, including the events.

How about an approach that elevates the tick and talk in the example, to become language identifiers that are exported somehow?

export const tickEvent = …

Server-side rendering

Since computer effort is so much cheaper than human effort, perhaps consider scrapping the generate option? The compiler could always emit both, and always require that components be written such that they can successfully compile either way.

(I think the future of higher productivity development will be improved by raising the bar, building slightly less flexible tools in exchange for those tools "always" producing more consistent, coherent output. is it okay to say to Svelte users, "Svelte is intended to make 100% components that work seamlessly in the browser or SSR, if you don't want that, consider one of the 300 older, less mature component mechanisms"?)

Sync vs async rendering

It is a bit sad to lose a point of differentiation, but this change is probably inevitable. I think other frameworks moving in a similar direction have already reached this conclusion as inevitable also.

Svelte would continue to offer a custom element compile target

Along the same lines as earlier, how about if this was not a switch, but rather nailed on?

This would close off the avenue of "create hundreds of components without web component compatibility in mind, or SSR in mind, discover you want those, and realize there is a whole bunch of work to do”. (I don't know if this matches the Rich vision!)

Summary

Hopefully some of my rantings are helpful. Overall this RFC is excellent and I think a design anywhere in the general vicinity will be a great future for Svelte!

PaulMaly commented 5 years ago

Hi! Great RFC and PR! Thanks!

My two cents if you don't mind.

  1. In all examples <script> tag in top of the component. Is this will be mandatory or just for good looking?

  2. Should we really have a differences between component's custom events and component's "lifecycle events"? How about use the same mechanism in both cases? So, perhaps, we don't need such functions like onprops or onupdate described in RFC:

import { on } from 'svelte';

on('destroy', () => { ... });
  1. Meta. How about use default export for this purpose? In v2 all our default export looks like compilator settings. So, maybe we should use it in v3 for the same:
export let foo = 1; // external prop
let bar = 2; // internal state

export default {
  namespace: 'svg',
  tag: 'my-cool-thing',
  immutable: true, 
 /* another compilator settings / meta info */
};

One more benefit is that we'll prevent another way to use default export in components.

  1. Custom events. I think if we'll use vanilla CustomEvent, what if we don't need to have some Svelte-specific apis? Do we really need to have built-in event system? We also can use some userland helper function.
// helpers.js

export function dispatch(el, event, detail) {
  let event = new CustomEvent(event, { detail });
  el.dispatchEvent(event);
}
<script>
  import { dispatch } from './helpers.js';

  export let el;

  dispatch(el, 'something');
</script>

<div ref:el>
   ...
</div>
// outside
comp.el.addEventListener('something', () => {});
  1. Bindings. I belive we should refuse 2-way bindings. It's easy to made the same thing usings standard events:
<input value={val} on:input={e => val = e.target.value}>

oninput or onchange are standard events provided by html input "component". So, I think we can use the same approach for the svelte components - use component's standard event:

<Component foo={foo} on:update={e => foo = e.detail.foo} />

Also I think it would great if we'll have shorthand for event handlers:

<script>
   function update() { ... }
</script>
<Component on:update />
<!-- equals to -->
<Component on:update={update} />
  1. Sync vs async rendering I'm for async things. Maybe microtasks?

  2. If we'll able to have multiple <script> tags in components, I think could albe to have separated scripts for client and server and both:

<script env="server">
 /* works only on server */
</script>

<script env="client">
 /* works only on client */
</script>

<script>
 /* works on both */
</script>
  1. Spread props. I think we can provide all props in event's detail object and use it like this:
<script>
  import { on } from 'svelte';
  import Nested from '...';

  let props = {};

  on('props', e => {
      props = e.detail;
  });
</script>

<Nested {...props} />
  1. Store.

In v3 any expression and any import would be available in HTMLx expressions. So, actually, I'm not sure that we really need to have Svelte-specific solution for store. We can just use 3rd-party or own implementations:

<script>
  import store from 'any-store';
</script>

<div>{store.foo}</div>

A few more thoughts:

These computed properties are 'push-based' rather than 'pull-based', which can result in unnecessary work

Yes, great! Make all things lazy as it possible!

or userland memoization

Compiler magic memoization, please)

A little chaotically, sorry for that. I hope some thoughts will be helpful.

arxpoetica commented 5 years ago

@PaulMaly noooo! They be stealing my two-way bindings! I love the power of two-way bindings. Simple and elegantly solves some very important things without the boilerplate of events.

As for the following, I love this proposal:

import { on } from 'svelte';
on('destroy', () => { ... });
Kiho commented 5 years ago

I like 2 way binding too, on:input, on:update .... does not work well with dynamic component.

Kiho commented 5 years ago

If we just use 3rd party store then I don't think we can have context based sub-store.

PaulMaly commented 5 years ago

Hi @arxpoetica @Kiho!

They be stealing my two-way bindings! I love the power of two-way bindings. Simple and elegantly solves some very important things without the boilerplate of events.

I like 2 way binding too, on:input, on:update .... does not work well with dynamic component.

Actually, I agree with you guys! But in RFC we read:

The reason we can consider removing component bindings is that it's now trivial to pass state-altering functions down to components:

I don't like this idea and for me tags 2way bindings less useful than component bindings. I think if we're removing component bindings, 2way loses meaning.

If we just use 3rd party store then I don't think we can have context based sub-store.

Could you please describe a little detailed what do you mean? In this part, I've guided a principle: "Svelte provide only that features which can't be implemented in userland". Seems v3 gives us more opportunities to use 3rd-party stores. So, I believe we could discuss are we really need own solution.

PaulMaly commented 5 years ago

Improve my initial comment a little bit:

  1. We need only a few helpers functions in svelte. Each function should automatically be associated with the component instance during instantiation:
import { destroy, dispatch, on } from 'svelte';

These helpers should be attached as public interface of component too.

  1. I think good idea to use default export for meta info and compiler settings of component.
export default {
  tag: 'my-component'
};
  1. I believe we should preserve 2way bindings for components or remove this feature completely.

  2. Multiple <script> tags for differences environments is a good idea for Svelte/Sapper, in my opinion. Seems, some kind of operations works really differently on that environments with almost the same result. This is very clean way to describe it.

  3. Event listeners callback using on method should receive apropriate event.detail object with event-specific stuff.

import { on, dispatch } from 'svelte';

on('props', e => {
    console.log(e.detail) // all component props
});

on('update', e => {
    console.log(e.detail) // all component state
});

on('customEvent', e => {
    console.log(e.detail.payload);
});

dispatch('customEvent', { payload: 'something' });
  1. I believe it'll be nice if Svelte become more lazy and async. Maybe more compiler-driven memoization things. I prefer if we move to async rendering and all that things like time-slicing etc.

  2. Seems, we could rethink do we really need to have built-in Store solution?

PaulMaly commented 5 years ago

One more insane idea, perhaps, component's instance could be represented by real DOM element extended by exported props and other stuff. Seems, the closest analogy is Shadow Root.

So, we'll be able to all power of Svelte features and DOM API as well:

import Component from '...';

const comp = new Component({
    /* props here */
});

comp.addEventListener('update', () => {...});

comp.foo = 'update component prop';

document.body.appendChild(comp);

In this case, dispatch & on methods would be unnecessary or become just a sugar:

comp.addEventListener('customevent', () => {...});

comp.dispatchEvent(new CustomEvent('customevent', { detail }));

We also will be able to use MutationObserver for the components directly.

Don't hit me too much, please))

p/s re: shadowroot emulating. Btw, almost the same approach used by Angular with an option ViewEncapsulation.Emulated. Also, we could use :host() special css selector from web components to write css for this element.

Rich-Harris commented 5 years ago

Thanks everyone! Big ol' response dump inbound:

a single well-defined macro

I'm really keen to avoid these :) At the very least, I think they should be a last resort. So far I'm not persuaded that we need them, though I realise we haven't come up with a really compelling solution for a v3 Store.

it still necessary to actually execute all possible code paths to find out what events a component might emit. Ideally the list (and shape) of emit-able events would be part of the declared public API of a component...How about an approach that elevates the tick and talk in the example, to become language identifiers that are exported somehow?

I do like the idea of events being part of a declared API β€” would be great to (for example) get an edit-time warning that on:select should actually be on:selected, or whatever. Having said that, it becomes more complex if events bubble, since the component you're adding the listener to may not be the component that emits the event.

For the majority of cases you could determine which events a component was capable of firing, just by looking at the first argument of calls to dispatch. Not bulletproof (especially if dispatch was passed outside the component), but enough to provide information for your linter etc in most situations.

Since computer effort is so much cheaper than human effort, perhaps consider scrapping the generate option? The compiler could always emit both, and always require that components be written such that they can successfully compile either way.

I love this idea! We could add a static $render named export to components, mirroring the $on and $destroy instance methods. This makes a ton of sense. We'll have to be even more careful than we already are about maintaining tree-shakeability, but aside from the fairly modest overhead of generating some extra JS for bundlers to ignore, this sounds like all win.

I'm not sure if we can do the same thing for web components though. Typically they way you'd use a WC is with an empty import...

import './my-app.html';
document.body.innerHTML = `<my-app></my-app>`;

...which implies that the component is self-registering, defeating tree-shakeability. It could export a $define or $register function instead, but that becomes slightly more cumbersome to consume.

There's also ideas for alternative compile targets (like svelte-gl) kicking around, and we definitely wouldn't want to bundle all those in, so I think it probably does make sense to maintain these as a compiler option.

In all examples Githubissues.

  • Githubissues is a development platform for aggregating issues.