sveltejs / rfcs

RFCs for changes to Svelte
274 stars 81 forks source link

Reactive declarations #8

Closed Rich-Harris closed 5 years ago

Rich-Harris commented 5 years ago

View formatted RFC

tivac commented 5 years ago

There has been some discussion about whether we should make props available immediately, rather than waiting until the first update cycle.

Props not being available immediately has me pretty worried, I strongly think that should be reconsidered. What's the argument in favor of not having them until beforeUpdate?

Rich-Harris commented 5 years ago

Mostly that it's surprising — the wtf factor is high with code like this:

export let todos;
console.log(todos); // ['walk the dog', 'do the laundry']

It may also just be hard to do (e.g. in cases of destructuring assignments, though maybe they don't make sense anyway), I haven't fully explored it

tivac commented 5 years ago

That example you posted works exactly how I'd expect props to work though. I guess that explains the disconnect...

timhall commented 5 years ago

Wrote down my thoughts on how this could be pull-based with immediate timing here: https://gist.github.com/timhall/81b599cdce819a1599298825c83d40af

Generally: computed: becomes pull-based and is kept in-sync on the js-side based on access and reactive: is used for blocks / effects that are run on initialize and before beforeUpdate


I agree with @tivac, I think it'd be intuitive if initial props were available and I think they could be transformed to the following:

export let a = 20;

// becomes
let { a = 20 } = $$initial_props;

I think it'd be very confusing to have a value you seemingly just set not be available until update. Rendering should be asynchronous, but with pull-based reactives, I think the js can run in a synchronous update fashion so that console.log, Assert.AreEqual, and such will behave as expected.

let a = 1;

let b;
b = a + 1;
console.log(b); // -> 2

// compared to

let b;
compute: b = a + 1;
console.log(b); // -> undefined...?

For automatically using source/store values with $... (like in templates), something explicit like the following might work:

import { get } from 'svelte/source';
import { todos } from './stores';

let $todos;
subscribe: $todos = get(todos);

// would expand roughly to
let $todos;
onDestroy(derived([todos], () => { $todos = get(todos); }));

get(...) to get the current value may not be necessary in all cases (and could be todos.get()), but it allows for the statement to be generic, with all retrieved variables passed into derived.

RedHatter commented 5 years ago

I think the "wtf factor" of initial prop values depends on how you think of the script block. If you think of it like the constructor of a class then the props would be the arguments passed in, and having the props immediately available would be expected. On the other hand if you consider the script block like a module then the props are more like exports and having them initially set would be very confusing.

I think it really depends how we teach v3. Personally I would consider the script block more like a module, but that's not necessarily the best way to think about it.

Rich-Harris commented 5 years ago

@timhall thanks, those gists are very interesting.

A concern I have — and this also applies to eagerly initialising props, to some extent — is that there's a much greater distance between the code the user writes and the code that actually runs. I think there's a lot of value in having output that's recognisable to the author of the input; the further we get from that, the less confident component authors will be that they understand what's going on. It's not a binary thing so much as a balance we should try to strike.

I don't know where I land on the props thing. Using a default value in a destructuring assignment like you suggested is way smarter than the alternative (not least because it means the fallback value wouldn't even be evaluated). I'm still torn about which behaviour is more intuitive. Lemme create a poll...

Rich-Harris commented 5 years ago

Ok, time for some democracy

This isn't strictly related to this RFC, but it's arisen in the context of it.

There are two ways that props could be treated. Option 1 — the status quo, according to the current v3 branch — is that the contents of <script> run more or less as written, aside from some modest instrumentation. This means that passed-in props...

<Widget foo={42}/>

...are not available until the first lifecycle function:

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

  export let foo = -1;
  let bar = foo * 2;
  console.log(foo, bar); // logs -1, -2

  onMount(() => {
    console.log(foo, bar); // logs 42, -2
  });  
</script>

Option 2 is that Svelte transforms the code so as to inject initial props at the declaration site:

<script>
  export let foo = -1;
  let bar = foo * 2;
  console.log(foo, bar); // logs 42, 84
</script>

Option one is more burdensome, but it is less shocking. Option 2 is shocking, but reduces your risk of RSI. Which is preferable?

ansarizafar commented 5 years ago

My vote is for Option 2.

arxpoetica commented 5 years ago

@Rich-Harris Q. If one doesn't specify "export" does that mean the prop never lands inside the component?

tivac commented 5 years ago

@arxpoetica If you don't export the variable it's not able to be set externally, so it'll always be whatever the code inside the component has set it to as a default.

mindrones commented 5 years ago
<script>
  import { onMount } from 'svelte';

  export let foo = -1;
  let bar = foo * 2;
  console.log(foo, bar); // logs -1, -2

  onMount(() => {
    console.log(foo, bar); // logs 42, -2
  });  
</script>

This IS indeed shocking: why at onMount would we log 42, -2 and not 42, 84? (a typo?)

As I see it Option 2 is way better because you read it as a function:

function widget (foo = -1) {
  let bar = 2 * foo;

  console.log(foo, bar);
  return {foo, bar}
}

widget() // logs -1, -2
widget(42) // logs 42, 84
<Widget /> <!-- logs -1, -2 -->
<Widget foo={42}/> <!-- logs 42, 84 -->

Option 1 would be:

<Widget /> <!-- logs -1, -2 -->
<Widget foo={42}/>
<!-- logs:
-1, -2
42, 84
-->

Which is counterintuitive: you don't expect to always execute with default values and eventually also with the passed arguments.

nsivertsen commented 5 years ago

Are there any practical drawbacks to option 2, other than it being shocking? It does save quite a bit of boilerplate, so if it's just a matter of educating people how to think about the execution context of script tags, then I'm all for it.

Rich-Harris commented 5 years ago

@mindrones not a typo, no. bar isn't reactive (no $) so its value is set to whatever that expression evaluates to upon initialisation, which is -2. If you wanted 84 you would need to run that line in onMount instead, if we kept option 1

silentworks commented 5 years ago

Option 1 feels like React's componentDidMount, but I agree with @mindrones that Option 2 feels more natural.

timhall commented 5 years ago

I think the distinction for props is whether export ... a = 1 is an initial value or a default value. My mental model had been that they were default values, but from a js and consistency point of view I'm thinking it makes more sense (and is more correct) to think of them as an initial value. When props are set, either externally or internally, it is a reactive assignment that components can pick up via the various lifecycle methods / reactive declarations.

// <Component a={42} />

let a = 1;
console.log(a); // -> 1

export { a }
console.log(a); // -> 1 (42 would be a surprising side-effect)

beforeUpdate(() => { console.log(a); }) // -> 42
$: { console.log(a); } // -> 42
Rich-Harris commented 5 years ago

If props were injected, it would be at the variable declaration site, not the export declaration site. So:

// <Component a={42} />

let a = 1;
-console.log(a); // -> 1
+console.log(a); // -> 42

export { a }
-console.log(a); // -> 1 (42 would be a surprising side-effect)
+console.log(a); // -> 42

beforeUpdate(() => { console.log(a); }) // -> 42
$: { console.log(a); } // -> 42
Rich-Harris commented 5 years ago

The comparison @silentworks makes is interesting, now that I think about it. React recently moved from a mechanism were props aren't available at first (i.e. in the component's constructor), to one where they are (functional components). (I say 'recently'; functional components were around for a while, but it's only with the advent of Hooks that they became useful for actual work.)

Perhaps there's a lesson there, notwithstanding the fact that they're two very different designs.

vijithassar commented 5 years ago

This explicit syntax strikes me as lot clearer than overloading the assignment operator as suggested in RFC1. But — jQuery aside — what is the syntactical advantage of using labels $: x over functions $(x) or even telling the compiler to interpret a special comment x // $? Is the $: label just an alternative token from standard JavaScript syntax that nonetheless leaves room for clever proprietary interpretation solely because it is rarely used?

I'm also not sure I follow this concern about the function syntax:

Since compute is just a function, I would expect to be able to compose it, curry it, pass it outside the component and so on, none of which are true.

Apologies if I am misunderstanding something about the implementation here, but since compute is just a function, you actually can do all those things, in theory. Rather, the problem is just that the nature of what compute does when it executes means it is usually not a good candidate for composition, currying, or exporting, which is very a different criticism about the functionality (which actually may never be composable) rather than the syntax used to invoke it.

Rich-Harris commented 5 years ago

For the avoidance of doubt, this isn't an alternative to reactive assignments — it's a companion to it. Reactive assignments are the thing that tells the framework it needs to re-evaluate reactive declarations (and which ones).

I don't think code comments are a good solution to this sort of problem — it's too easy to skip over them when reading code, and there's no real precedent for comments affecting runtime behaviour that I can think of (though you're more well-travelled then I am and may have some thoughts on that). Besides, you have to do a lot of work to recover structure from comments, whereas labels are easy to pick out as you traverse an AST (not that implementation details should guide design decisions).

There's no real syntactical advantage to using labels over functions, though the label form is easier on the fingers, and I'd argue the visual distinctiveness provides important clarity. But functions can't really be used here anyway. This has come up a few times so I'll try and make this answer canonical:

Why we can't use functions for this

There are basically two proposals that people have made:

import { compute } from 'svelte';

let foo = 1;

// compute returns a value
let bar = compute(() => foo * 2);

// compute runs automatically
let bar;
compute(() => {
  bar = foo * 2;
});

Neither works without transforming the code in surprising ways. (I'm not saying that labels aren't surprising, but I think there's a big difference between 'I'm surprised because I didn't have pre-existing expectations of what this code would do' and 'I'm surprised because this actively contradicts my prior expectations of what this code would do'.)

In either case, the compute CallExpression would need to be augmented with information about the expression's dependencies if the expression were to be reactive at run time:

// compute returns a value
-let bar = compute(() => foo * 2);
+let bar = compute(() => foo * 2, ['foo']);

// compute runs automatically
let bar;
compute(() => {
  bar = foo * 2;
-});
+}, ['foo']);

In the first case, it's not obvious what the return value should be. If it's the result of the computation, then we need to take the entire statement and transform it into the second form. If it's some kind of reactive value wrapper object, then we need to transform every instance of it outside of that statement.

For either transformation to take place, we need to identify compute (or foo, if we did something like import { compute as foo } from 'svelte' as a magic function that has distinct behaviour. Which brings us to:

since compute is just a function, you actually can do all those things, in theory.

In fact, you couldn't even rename the function:

import { compute } from 'svelte';

const makeReactive = compute;

let foo = 1;
let bar = makeReactive(() => foo * 2);

The compiler simply wouldn't know what to do with the bar declaration; it would be unable to make the necessary code transformations. Much less could you apply any kind of composition or use compute from a helper that was defined outside a component (Svelte's equivalent of React's custom Hooks). You also wouldn't be able to use compute in e.g. a setTimeout, despite appearances, because each call would need to be hooked into the component's lifecycle, meaning it has to happen upon instantiation.

Using syntax solves the problem much better because it doesn't create any illusions about what you can do. It doesn't give you enough rope to hang yourself with; $: is simply a marker that says 'the framework will decide when to run this statement, it's out of your control, and that's ok'.

kzc commented 5 years ago

If we could use the destiny operator in components, the Svelte 3 compiler would be able to do exactly what it does with Svelte 2 computed properties — update b whenever a changes. Unfortunately we can't, because that would be invalid JavaScript, and it's important for many reasons that everything inside a component's <script> block be valid JS.

What are those reasons? To use Acorn and Code Mirror without modification?

The label proposal is just syntax compatible with ES, not in behavior. Like Svelte HTML, the script is going to be transpiled anyway. Use the density operator. Be bold!

Rich-Harris commented 5 years ago

@kzc I like the sentiment! I wish it was just a matter of creating a new Acorn plugin. Sadly, there's so much more to it than that — it's about whether people's syntax highlighters will work, whether their components will become a sea of red squigglies, whether we can apply arbitrary preprocessors, how easy it is to add TypeScript support, and so on. As soon as we allow syntactically invalid JS into components, we make it impossible to interop with any of the existing tooling out there, present or future.

If we were, say, Facebook we'd do it anyway, and let the open source community bear the cost of creating integrations with everything. But we're the scrappy underdogs so instead we have to improvise, adapt and overcome.

kzc commented 5 years ago

I dunno. I think shoehorning the density operator into ES syntax is not very intuitive and can lead to odd scenarios like this:

let a = 1;
let b;
$: b = a + 1;
b = "something else";   // ambiguous - what should happen here?
console.log(b);

You really want to mark the density declaration itself, not the first assignment.

This would be more intuitive but it's not valid ES syntax to label a declaration:

let a = 1;
$: const b = a + 1;
b = "something else";  // throws: TypeError: Assignment to constant variable
console.log(b);

If you are constrained by existing ES syntax and do not want to use comment annotations, you might consider this alternative to mark the density declaration site:

$:; const b = a + 1;

which admittedly is also weird, but is closer in spirit to the original density operator in my opinion:

const b <= a + 1;
Rich-Harris commented 5 years ago

If the main concern is the ambiguity you identified, I think that's easily solved - the compiler can easily identify which values are assigned in the context of a reactive declaration and treat any other assignments to those variables as a warning or error, which is what currently happens with computed properties in Svelte 2 (albeit only at run time and only in dev mode)

kzc commented 5 years ago

Ambiguity was just one point. The inability to mark the density declaration directly and the extra verbosity of repeating the symbol twice in let b; $: b = a + 1; was the main point.

Rich-Harris commented 5 years ago

We've been talking about making the let b implicit in that case — if you provide it yourself, fine, but if not the compiler could do it for you easily enough.

kzc commented 5 years ago

The problem with an implicit let b is lack of scoping. Presumably you'd want to mirror ES6 semantics for editors that can intelligently replace JS variable names.

But if you're going to pursue that implicit route you might as well just use a magic prefix $$_b (or whatever) for the density declaration and all its uses and abandon the label altogether. That way you could declare const $$_b = a + 1; console.log($$_b);. A magic prefix for density variables also has the advantage of instant distinguishability from uses of regular (non-computed) variables.

Anyway just throwing some ideas into the mix. Feel free to ignore.

vijithassar commented 5 years ago

there's a big difference between 'I'm surprised because I didn't have pre-existing expectations of what this code would do' and 'I'm surprised because this actively contradicts my prior expectations of what this code would do'

Agreed, just trying to pick less confusing of two evils! Arguably, functions may be a better candidate for mysterious shenanigans such as reactivity because it is generally understood that they do things. For example, this would be useless but not unexpected given what we can assume most JavaScript developers will already understand about how functions behave:

// svelte
export const compute = () => 'hello world';
// user
import { compute } from 'svelte';

let foo = 1;
let bar = compute(() => foo * 2); // 'hello world'

Who knows what happened inside the black box of the imported function? You don't know for sure unless you look, because that is just how functions always work! And so the behavior is actually less surprising, in the sense that you already know you can always be surprised at this juncture unless you actually audit compose. Reactivity is very powerful and perhaps warrants this stronger flag.

in fact, you couldn't even rename the function

This could be prevented with an error at compile time, which is roughly comparable to preventing use of the reserved $ label with a more conventional loop, which will probably be necessary with a label syntax implementation.