Skip to content

Reactivity does not work for reference values #3197

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

Closed
primos63 opened this issue Jul 7, 2019 · 3 comments
Closed

Reactivity does not work for reference values #3197

primos63 opened this issue Jul 7, 2019 · 3 comments
Labels
awaiting submitter needs a reproduction, or clarification

Comments

@primos63
Copy link

primos63 commented Jul 7, 2019

Related to #2759, #3075

REPLs:
Bind:value example: https://svelte.dev/repl/033f49fa812b48abac0b2fc0096be97c?version=3.6.5
Immutability Example: https://svelte.dev/repl/84bca1af3b1b4aeb8979860bc2c12450?version=3.6.5

Reactivity for reference value variables, arrays and objects, does not work. I detailed why it doesn't in #2759. Basically arrays and objects are reference values; therefore, assigning them to another variable or comparing them only compares their reference and not the value.

This is seen in the comparison function safe_not_equal.

export function safe_not_equal(a, b) {
	return a != a ? b == b : a !== b || ((a && typeof a === 'object') || typeof a === 'function');
}

Both a and b, $$.ctx[key] and $$.ctx[key] = value, refer to the same reference value for arrays and objects; therefore, typeof a === 'object' is always true and the make_dirty method is executed. So reactivity appears to work, but it really does not.

Steps to reproduce:

  1. Open the bind value repl
  2. Open the dev tools console
  3. For each input field select a single letter and type that letter (overwrite with same letter)
  4. Observe that the console indicates a change for the array, object, but not the single value.
  5. Click on the Change Array button, a console message is generated because the entire array is "reactive", but not really.
  6. Click on the Don't Change Object button, a console message is generated even though the object hasn't changed (obj.num is assigned to itself)
  7. Click on Change Value, the console message is generated as expected.

Immutable example:

  1. Open the repl and the dev tools console
  2. Click on wash the car under immutable. The function has been changed to use id + 1.
    The second line is selected and the console message shows the change.
  3. Click on mow the lawn.
  4. The toggle function will only return the existing entries, but a console message is generated when it shouldn't be.

If the intent is to make the entire array or object reactive when binding to an element in the array or object, then a deep copy and deep compare is going to be required. I attempted to fix this for bind:value, but in running the test suite there were numerous failures. I tried to use JSON.parse(JSON.stringify(object)), but this fails when an element within the object is unknown to the JSON methods. For example in element-ref (hydration), there's an object HTMLHeadingElement which the JSON methods turn to Object. There are utility methods in Lodash for a deep copy and compare, but that would add a runtime dependency to Svelte.

@brucou
Copy link

brucou commented Jul 11, 2019

I would argue that this is the way reactivity is implemented in most reactive frameworks. At heart is the notion of identity vs. that of value. The reactive frameworks I know mostly avoid being reactive on the value of a reference. If the simple case of an array [a, b, c] with only primitive values and f reacting on it, what you say makes sense. Now imagine that:

  • a is {relevant: ..., irrelevant:...} and is also being reacted on by one other g Svelte procedure.
  • a change in the relevant section of a should trigger f
  • a change in the irrelevant section of a should not trigger f but trigger g
  • Then any change in a will lead to two reactions f and g when sometimes it should be only one reacting. The thing is the deeper the object hierarchy the more likely this is problematic -- and that is not even mentioning issues with circularity.

So if every changes in the top-level array would result in a reaction from f, and every change in the irrelevant property leads to f not doing anything because there is nothing to do, that is a lot of useless computations and a possible performance drag. The alternative which is to declare upfront what properties of a you want to react on is not very practical either (properties can be dynamically added and not know ahead of time, and then you still have to keep track of all that).

If you want to react on every change (in value) of a variable, then you can associate a change of value to a change in reference -- that is the idea behind immutable data. That way you have the best of both worlds, with a single simple reactive rule.

PS: I am not part of the Svelte team, and this is only my humble opinion

@primos63
Copy link
Author

Appreciate the input. Perhaps I didn't communicate the issue well enough. It's not about being reactive on a[3] or a.b it's that a is a reference value and using it in a comparison (previous vs current) is meaningless when

  1. it's set to the reference value of current
  2. a new reference is created when no data has changed

These lead to the update function being triggered solely because of a test for the typeof a === 'object' not because of a change in reference value or content.

The code calling the comparison function, not_equal, is

$$.ctx = instance
	? instance(component, props, (key, value) => {
		if ($$.ctx && not_equal($$.ctx[key], $$.ctx[key] = value)) {
			if ($$.bound[key]) $$.bound[key](value);
			if (ready) make_dirty(component, key);
		}
	})
	: props;

and could be changed to

$$.ctx = instance
	? instance(component, props, (key, value) => {
		if ($$.ctx && not_equal($$.ctx[key], value)) {
                        $$.ctx[key] = value;
			if ($$.bound[key]) $$.bound[key](value);
			if (ready) make_dirty(component, key);
		}
	})
	: props;

but I don't think you'd want to do the comparison if the contents of $$.ctx[key] are equal to those of value. It's not dirty.

So maybe...

$$.ctx = instance
	? instance(component, props, (key, value) => {
		if ($$.ctx && not_deepEqual($$.ctx[key], value)) {
                        $$.ctx[key] = value;
			if ($$.bound[key]) $$.bound[key](value);
			if (ready) make_dirty(component, key);
		}
	})
	: props;

where not_deepEqual would compare the contents (taking into account circular references). That would be my easiest iteration yet!

@antony
Copy link
Member

antony commented Apr 9, 2020

This is a documented behaviour of Svelte - you need to reassign object and array values in order to trigger reactivity events.

@antony antony closed this as completed Apr 9, 2020
@antony antony added the wontfix label Apr 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting submitter needs a reproduction, or clarification
Projects
None yet
Development

No branches or pull requests

4 participants