Skip to content
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

Infinite update cycle when using spread props and also binding to one of the object's array properties #5117

Closed
dimfeld opened this issue Jul 9, 2020 · 3 comments · Fixed by #5126
Labels

Comments

@dimfeld
Copy link
Contributor

dimfeld commented Jul 9, 2020

Describe the bug

Using a spread property and also binding to an array value on the spread object causes an infinite update cycle. This only seems to happen if the bound value is an array or an object.

<script>
	import Component2 from './Component2.svelte';
	let values = [{ x: 5, y: [6] }];
</script>

<!-- Uncomment the below to trigger the bug -->
<!-- <Component2 {...value} bind:y={value.y} /> -->

To Reproduce
REPL: https://svelte.dev/repl/a0b9cf113353432b9242e78099ac8fd8?version=3.24.0

Expected behavior
Catch at compile time if possible. Not sure how feasible that is due to possible aliasing?

Regardless, some way to avoid crashing the page would be good. Maybe remove the key related to the bound variable from the changes object when spreads and binds are being used.

Information about your Svelte project:
REPL with latest Svelte

Severity
Not my bug, saw it mentioned in the Discord. Since there's a non-spread workaround I assume not a blocker but I'll let the original reporter chime in if it is.

Analysis

Looking in the generated code, we see in the bind handler below that it takes care to not add y to component2_changes if updating_y is true.. But get_spread_update returns both x and y regardless of the value of updating_y, so I believe this is what leads to the infinite updates.

const component2_changes = (dirty & /*value*/ 1)
    ? get_spread_update(component2_spread_levels, [get_spread_object(/*value*/ ctx[0])])
    : {};

if (!updating_y && dirty & /*value*/ 1) {
    updating_y = true;
    component2_changes.y = /*value*/ ctx[0].y;
    add_flush_callback(() => updating_y = false);
}

I wonder if adding something like this at the end would fix it:

} else if(updating_y) {
  delete component1_changes.y;
}
@bterrio
Copy link

bterrio commented Jul 11, 2020

In the meantime, if other people run into this, here's one workaround I've found using reduce to manually remove any bound props from the spread. This is assuming there's a need to use the spread operator in order to honor default component values for unspecified props. Of course, ideally there'd be no way to crash the page like this, but this works!

https://svelte.dev/repl/800a3aad804d452cbe7bbf5ea375fb88?version=3.24.0

@dimfeld
Copy link
Contributor Author

dimfeld commented Jul 11, 2020

Incidentally, with modern Javascript you can use this syntax too to get a copy of the object without y:

function getSpreadProps(value) {
  let { y, ...rest } = value;
  return rest;
}

@Conduitry
Copy link
Member

Should be fixed in 3.24.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants