Skip to content
This repository has been archived by the owner on Aug 28, 2020. It is now read-only.

[Reactive declarations] Defining dependencies with compound statements #34

Open
opensas opened this issue Jun 4, 2020 · 25 comments
Open
Labels
good first issue Good for newcomers

Comments

@opensas
Copy link
Contributor

opensas commented Jun 4, 2020

Defining dependencies with compound statements

In Svelte you can declare reactive compound statements with the following syntax:

$: {
  statement1;
  statement2:
  [...]
}

The solution stated in https://github.com/svelte-society/recipes-mvp/blob/master/language.md#defining-dependencies to declare multiple dependencies won't work with compund statements:

given example (works ok):

let one;
let two;
let three;

const someFunc = () => sideEffect();

$: one, two, three, someFunc();

with compound statements (does not work):

let one;
let two;
let three;

$: one, two, three, {
  sideEffect1();
  sideEffect2();
}

It gives an unexpected token error

The solution I found:

let one;
let two;
let three;

const someFunc = () => sideEffect();

$: {
  one, two, three   // dependencies
  sideEffect1();
  sideEffect2():

Question: should I add a pr to the recipe to add this workaround? Or shall I file a bug in Svelte so that it can support $: one, two, three, { syntax???

@opensas opensas added the good first issue Good for newcomers label Jun 4, 2020
@opensas opensas changed the title [Reactive declarations] Defining dependecies with compound statements [Reactive declarations] Defining dependencies with compound statements Jun 4, 2020
@frederikhors
Copy link

Open on sveltejs/issues too IMO.

@swyxio
Copy link
Member

swyxio commented Jun 4, 2020

oh goodness. this is well out of my paygrade. calling in the big guns @pngwn @antony

@pngwn
Copy link
Member

pngwn commented Jun 4, 2020

This isn’t valid javascript syntax so it won’t be supported in svelte. Without the other variables and commas { ... } is a block statement, when you add variables and commas it is an object literal (and can not contain arbitrary expressions). We wouldn’t be able to analyse it as a block statement containing expressions to be executed. Acorn will choke and this and we don’t want implement our own flavour of javascript.

I’m not sure if this pattern should really be encouraged, it is pretty confusing to read. We should really be recommending patterns that can help form reasonable ‘best practices’.

Can you give an example of what situation you might want to use this in? Maybe there is a clearer way of expressing it.

@opensas
Copy link
Contributor Author

opensas commented Jun 5, 2020

Thanks a lot for your reply @pngwn, I completely agree that we shouldn't support nor encourage invalid JavaScript syntax, you have a good point there.

I think we should have a clear and standard way to explicitly define dependencies, and ideally it should work with expressions and block statements.

I do have and example, I'm writing a tutorial about svelte for mdn (which I will kindly ask for your feedback once it's ready ;-)). I'm doing a simple todo app, and I have the following trouble:

I would be very grateful it any of you could help me reason about it.

I've documented it and have a working example in this repl, with a lengthy comment explain what I think is the problem. For short, just click on a checkbox ('xx out of xx items completed' message will not be updated), or filter by active and then click on a checkbox (the updated todo should disappear, and it won't)

In short, I have a todos array. I also have a filter variable (all, active, completed) that reactively defines a filteredTodos that is created from the todos array (filteredTodos = todos.filter(x=>x.completed), for example)

I iterate thru the filteredTodos array using {#each}, and in the same loop I can modify the items when I click on an input checkbox.

As far as I understand, this behaviour is documented here: https://github.com/svelte-society/recipes-mvp/blob/master/language.md#variables-not-values

When I modify filteredTodos I'm indirectly updating the content of todos array, but I'm not updating the actual todos array. Svelte has no way to know that todos depends (or can be modified by) filteredTodos, that's why I have to add it as a dependency.

The solution I found was to explicitly define filteredTodos as a dependecy, doing things like this:

$: filteredTodos, statusMessage = `${todos.filter(t => t.completed).length} out of ${todos.length} items completed`
[...]
$: filteredTodos, filteredTodos = filterTodos(filter, todos)
[...]
   // using a block statement
   $: {
     filter, todos, filteredTodos    // dependencies - with explicit dependencies
     if (filter === 'active' )          filteredTodos = todos.filter(t => !t.completed);
     else if (filter === 'completed' )  filteredTodos = todos.filter(t => t.completed);
     else filteredTodos = todos;
   }

I really don't like having to use explicit dependencies (it feels a bit hacky) on such a simple example.

The problem comes from modifying a variable thru other variable.

BTW, the easiest solution I found was to just get rid of filteredTodos, work on the todos array, and implement the filter with an {#if...} inside the loop. see this repl Can you think on an easier alternative for an introductory tutorial about svelte?

I think this must be a rather common issue, if I stumbled upon it with such a basic example.

Is there some way to tell svelte that changes to filteredTodos should also affect todos? or even better, some way to automatically implement reactivity based on the underlying values and not on the variable being referenced in the assignment.

Thanks a lot for yout time, and excuse me for the lenghty reply

@swyxio
Copy link
Member

swyxio commented Jun 5, 2020

ouch.. yeah nested objects in objects/arrays is one of the pain points of svelte. i dont see any other way to do it unfortunately :( hopefully one of the others with more experience can tell you a more idiomatic way

@pngwn
Copy link
Member

pngwn commented Jun 5, 2020

I’ll take a closer look at this over the weekend when I have more time.

@opensas
Copy link
Contributor Author

opensas commented Jun 5, 2020

don't worry @pngwn , there's no rush, really, thanks a lot

@pngwn
Copy link
Member

pngwn commented Jun 5, 2020

In this case, I'd do something like this

It simplifies the code quite a bit (or at least i think it does) by taking a slightly different approach. This doesn't really solve the problem you mentioned particularly but it is an alternative solution to the problem at hand.

I'll think some more on the problem in general, which is more to do with object references than anything.

@opensas
Copy link
Contributor Author

opensas commented Jun 5, 2020

Very clever indeed.

You avoid declaring a filteredTodos by calling the function from the each loop. I modified my own example to get rid of FfilteredTodos and instead call:

{#each filterTodos(filter, todos)	as t, i (t.id)}

and it works ok

With this change, somehow svelte realizes that operations within the loop affects the todos array. Even though I don't quite understand how it works, the filterTodos function could return anything. I don't know what relates the t var in the loop with the elements of todos...

Anyway, working with filteredTodos the problem boils down to this:
In the loop I'm modyfing filteresTodos[x].completed, but anywhere else I'm listening for changes on todos[x]

As you mentioned the general problem seems to be how to let Svelte handle referential equality, that is knowing when two variables references the same object, and update dependencies accordingly.

That would be reaching true reactivity, I mean reacting when the underlying value changes, regardless the variable used to issue the change.

Perhaps something could be implemented with setters, and tell svelte to generate them with some compiler directive.

@swyxio
Copy link
Member

swyxio commented Jun 5, 2020

id love to document this in a recipe somewhere. it's definitely tripped me up at least once.

@dimfeld also suggested using immer in a store: https://gist.github.com/dimfeld/0ff834a2fcd6f7bf575eeb21471c290e

@pngwn
Copy link
Member

pngwn commented Jun 5, 2020

Absolutely. Object references are probably the biggest foot gun is Svelte’s reactivity system. We should come up with some common workarounds for different cases.

@opensas
Copy link
Contributor Author

opensas commented Jun 5, 2020

I think the issue is very well explained here:

https://github.com/svelte-society/recipes-mvp/blob/master/language.md#variables-not-values

In this example, when we reassign o.num we are updating the value assigned to obj but since we are not updating the actual obj variable SVelte does not trigger an update. Svelte does not trace these kinds of values back to a variable defined at the top level and has no way of knowing if it has updated or not. Whenever you want to update the local component state, any reassignments must be performed on the actual variable, not just the value itself.

I'd like to help documenting the workarounds. Using a function seems a nice one (although it is just a workaround, no the real solution to the issue) but I can't just make sense about how can svelte know what to update from a function call that may return anything.

Excuse my ignorance, but anybody knows how the other frameworks deal with this kind of issues?

@swyxio
Copy link
Member

swyxio commented Jun 5, 2020

React is easy - just rerun everything by default all the time and memoize expensive calculations. React and Svelte are at two ends of a spectrum in this regard

@opensas
Copy link
Contributor Author

opensas commented Jun 5, 2020 via email

@opensas
Copy link
Contributor Author

opensas commented Jun 6, 2020

I've just found this todo app example, it goes like this to avoid duplicated object references:

    function handleToggleComplete(event) {
        const todoIndex = todos.findIndex(todo => todo.id === event.detail.id);
        const updatedTodo = { ...todos[todoIndex], completed: !todos[todoIndex].completed};
        todos = [
            ...todos.slice(0, todoIndex),
            updatedTodo,
            ...todos.slice(todoIndex+1)
        ];
    }

It's way too verbose to my taste, compared to just mutating the property from the each loop, but I think it has a few good points that we could take as recommended practices:

  1. Pick a single source of truth and make sure that every modification goes thru it, in this case the loop uses filteredTodos but the modification is on todos.

  2. Never mutate an object or a property object, assign to a new instance. Unfortunately this is what makes the code so verbose.

I think these could be recommended practices, but you loose that "straight to the point" feeling...

And finally, a last workaround we might suggest, when everything else fails just adding a todos = todos after the update solves the problem, but I don't really like that approach...

@swyxio
Copy link
Member

swyxio commented Jun 6, 2020

yea @halfnelson actually suggested todos=todos and its a oneliner fix that solves everything - https://svelte.dev/repl/d87ac901fd624002bb9d368d6917b51f?version=3.23.0 worth considering?

@opensas
Copy link
Contributor Author

opensas commented Jun 6, 2020 via email

@tanhauhau
Copy link
Collaborator

this feels like a bug, i need to take a deeper look into this..

Related: sveltejs/svelte#2444, fixed in 3.2.1, but broken since 3.16.0

@opensas
Copy link
Contributor Author

opensas commented Jul 6, 2020

Absolutely. Object references are probably the biggest foot gun is Svelte’s reactivity system. We should come up with some common workarounds for different cases.

I've just stumbled upon this article on reddit

It seems like someone developed an alternative change-detector system that can handle object references.

Here's another article about the subject and a sample implementation

I hope this could be a step towards taking care of object references in Svelte's reactivity system.

@dimfeld
Copy link

dimfeld commented Jul 6, 2020

This looks like it's using a watcher system which checks everything being watched on every digest cycle. The advantage is that it's easier to detect changes inside objects and arrays since you're rechecking everything much more often. But having come from AngularJS 1.x, which uses watchers and digest cycles, I can attest that this is a nightmare for performance in more complex applications.

But please correct me if I'm misinterpreting the code.

@tanhauhau
Copy link
Collaborator

Even with change detection that allows mutation on object / array from anywhere to be detected, svelte still is not able to update only the value / element that is change.

for example,

<script>
   let obj = { a: 1, b: 2}
</script>

<div>{obj.a}</div>
<div>{obj.b}</div>

changing obj.a or obj.b will still checks & updates both div, as Svelte sees only 1 state variable, which is obj.

on the other hand, change detection in vue, knows in particularly which value / element has changed, and could optimise in the way that only 1 div is checked and updated.

I feel it maybe a philosophical question on whether to fallback onto change detection in the runtime for mutable objects / arrays, or will only do as much as we can during compile time, and it is a question that only @Rich-Harris can answer

@frederikhors
Copy link

@tanhauhau why do you say this?

Here your code: https://svelte.dev/repl/0b3716f785f04301bec077e9f48f0339?version=3.24.0

issue

@opensas
Copy link
Contributor Author

opensas commented Jul 27, 2020

@tanhauhau

whether to fallback onto change detection in the runtime for mutable objects / arrays, or will only do as much as we can during compile time

Can't we have some kind of annotation or setting that would allow me to tell the compiler how to check for changes? I could tell that I want a specific variable to be watched at runtime, and the compiler could generate the apropiate code only for that variable.

BTW, I'm also a bit confused about svelte not being able to detect which property has changed, looking at @frederikhors example it seems like it can...

@dimfeld
Copy link

dimfeld commented Jul 27, 2020

Svelte doesn't track which property changed, so it will check both the obj.a and obj.b references in this above example. But part of that check is to see if the value has actually changed. So in the above example, it updates the DOM only for obj.a.

On obj.b it does not result in a DOM update, but it still has to run some code to figure out if things have changed. When the template expression is just obj.b it's not a big deal but if it's something computationally intensive (say, sorting a large array and taking the top 10 results) then it may be more beneficial to avoid the check at all, when possible.

Per-property change detection could be done on some level in the compiler. I'm sure there would be additional edge cases to be considered in such a system though, and it would potentially add a lot of complexity to the compiler, so it would have to be carefully designed.

@opensas
Copy link
Contributor Author

opensas commented Aug 7, 2020

Thanks a lot @dimfeld, super clear explanation.

About other way to detect object/arrays changes: I got inspiration from this @tanhauhau 's twit: https://twitter.com/lihautan/status/1284666320970084353

And built this quick and dirty codesanbox

I use reactive and watch from vue to capture changes to an array and notify svelte about it with watch(o1, (value => o1.value = o1.value) )

Perhaps some kind of opt-int directive to add runtime change detection using these kind of libraries could be a good idea, or adding some way to hook into the reactivity system at runtime... I really don't think it's possible to do these kind of things statically parsing the code at runtime.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

6 participants