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 loop caused by two-way binding #398

Closed
TehShrike opened this issue Mar 20, 2017 · 13 comments
Closed

Infinite loop caused by two-way binding #398

TehShrike opened this issue Mar 20, 2017 · 13 comments

Comments

@TehShrike
Copy link
Member

I have a root component with a piece of data.

This root component includes several different intermediate components.

Each of these intermediate components uses the same leaf component.

All of these components use two-way binding to share the same piece of data, from the root, all the way down to all the leaf components.

When the leaf component changes the data that is shared with two-way binding, an infinite loop occurs.

Reproduction is at https://github.com/TehShrike/svelte-repro/tree/two-way-binding-infinite-loop

To reproduce,

  1. npm run build
  2. open the index.html file
  3. move the mouse over any of the visible number elements
  4. hear your fan spin up!

The issue only happens if both IntermediateComponent and EvenMoreIntermediate are included in TestComponent. Including multiple instances of just IntermediateComponent or just EvenMoreIntermediate does not cause the issue.

@Conduitry
Copy link
Member

I can reproduce this, but I'm not actually sure whether it's related to just the nested object setup. I removed the on:mouseleave handler, leaving just the on:mouseenter handler, and everything looks like it's working fine. There's probably something differently Svelte could be doing here, but perhaps it's more related to something like attempting to handle an event while currently handling another event.

For debugging this (with both event handlers in place), it doesn't help that my browser freezes even with breakpoints inside both event handlers! I didn't even know that could happen.

@Conduitry
Copy link
Member

Actually, it looks like what's causing the problem is specifically the null in on:mouseleave="set({ currentIdentifier: null })". If I use another value in there instead of null, it seems to work fine. Any value I've tried, even other falsy values, even undefined. It seems to somehow be specifically triggered by null, which is quite mysterious to me.

@Conduitry
Copy link
Member

As far as I can tell, this is related to typeof null being 'object', which is one of those fun javascript nuances I'd forgotten about. Changing dispatchObservers to continue the loop in that case seems to resolve this, but I'm not sure what else is going on or whether that's the best way to address this.

@TehShrike
Copy link
Member Author

It must have something to do with the component structure - removing this line fixes the issue, too.

@Conduitry
Copy link
Member

Okay, yeah, looks like my solution doesn't help if the value being assigned is actually a normal object, there's still the same infinite loop. Checking for null was just putting a bandaid on this one specific case.

@paulocoghi
Copy link
Contributor

@TehShrike

It must have something to do with the component structure - removing this line fixes the issue, too.

But in this case, you do not get the structure you originally planned, right?

@TehShrike
Copy link
Member Author

@paulocoghi
Copy link
Contributor

I was just pointing that, when you remove that line, you are not rendering the exact/same structure and "sequence" of components.

Although it fixes the issue, in a real scenario, if someone needs to render an specific sequence of components similarly to what you had done initially, removing a component would not be a solution.

@TehShrike
Copy link
Member Author

I only meant to point out that the infinite loop issue appears to depend on this specific component structure.

@paulocoghi
Copy link
Contributor

Ah okay, I understood.

@TehShrike
Copy link
Member Author

I pushed a simpler/more specific reproduction to https://github.com/TehShrike/svelte-repro/tree/two-way-binding-infinite-loop .

Root contains multiple As, A contains multiple Bs, and B contains a list of Cs.

When C sets the bound value to null, the infinite loop occurs.

Now, like @Conduitry noticed, using some other falsey value instead of null fixes the issue here - but in my actual app, using some other value like false does not fix the issue. So I'm guessing that the null is a red herring and the bug is actually something trickier.

@TehShrike
Copy link
Member Author

I wrote a test that I thought would be a straight copy of the reproduction above (which I tested in Chrome, Firefox, and Safari): TehShrike@f6f4c22

But the tests passed! I wasn't able to reproduce the infinite loop in the existing test structure.

TehShrike added a commit to TehShrike/revelation-visualized that referenced this issue Mar 21, 2017
Using a custom component closure until sveltejs/svelte#398 is fixed
@Rich-Harris
Copy link
Member

Man, that was a real Heisenbug. Think I've got it beat though — happily, the solution is straightforward and only adds bytes to component binding code: #407

Rich-Harris added a commit that referenced this issue Mar 26, 2017
prevent infinite loops caused by pathological component bindings
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants