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

Asynchronous assignment operation inside reactivity statement result in endless loop #4420

Closed
niyan-ly opened this issue Feb 16, 2020 · 8 comments
Labels

Comments

@niyan-ly
Copy link

Detail
As the title goes, here is the code or REPL version

It seems like assignment to a value that is managed by svelte as asynchronous operation inside the reactivity declaration is not supported. But which makes me quiet confusing is that if i assign an unmanaged value to a managed value will not result in endless loop.

Is that a bug or just something that i haven't gotten to understand.

<script>
  // pretend as a modal prop
  export let visible = false;

  let dataSource = [];
  let value = "";
  let remoteData = [
    { label: "first", value: 1 },
    { label: "second", value: 22 }
  ];

  $: if (visible) {
    console.log("---called----");
    // it semms like that asynchronous operation will result in endless loop
    setTimeout(() => {
      dataSource = remoteData.map(i => i);
      // but assign an value that is not managed by svelte will works
      // value = remoteData[0].value
      value = dataSource[0].value;
    });
  }
</script>

<button
  on:click="{() => {
    visible = !visible;
  }}"
>
  load from remote
</button>

<ul>
  {#each dataSource as item}
    <li>{item.label}</li>
  {/each}
</ul>
@antony
Copy link
Member

antony commented Feb 16, 2020

A simplified version of your issue is here:

https://svelte.dev/repl/93f936ebd89e45828db81f2d0e833c3a?version=3.18.2

It does indeed seem that any variable assignment happening inside a setTimeout as part of a reactivity statement will cause an infinite loop.

Whilst I'd strongly recommend that you don't use a setTimeout within a reactivity statement like this, I do think this is indicative of a bug, so marking it as such.

@antony antony added the bug label Feb 16, 2020
@niyan-ly
Copy link
Author

Whilst I'd strongly recommend that you don't use a setTimeout within a reactivity statement like this,

Well, but there are situations that we need to perform some asynchronous operation, like fetch. Are there any better approach to do that under the svelte pattern.

@antony
Copy link
Member

antony commented Feb 16, 2020

@poor-branson I would recommend doing fetch as part of a method which can be called from an event handler. That's how I do it. When the value of something changes or a button is clicked to change page, for example:

<input type="number" bind:value={page} on:change={paginate} />
<button on:click={() => { paginate(page + 1) }}>Next Page </button>
<button on:click={() => { paginate(page - 1) }}>Prev Page </button>

I personally use an IntersectionObserver to call a function when triggered.

Attaching heavy functionality to a reactive variable (whilst I agree it should be possible/should be an option) is not something I personally would recommend.

@tanhauhau
Copy link
Member

tanhauhau commented Feb 16, 2020

@poor-branson Check out this fixed repl

what happened is that, the $ reactive statement depends on:

  • visible
  • remoteData, and
  • dataSource,

which are on the right hand side of the assignment statements (LHS = RHS) within the reactive statement.

and the reactive statement will be executed when any of them get reassigned.

so within the reactive statement, you reassigned the dataSource

dataSource = remoteData.map(i => i);

and because the statement itself is depending on dataSource, due to:

      value = dataSource[0].value;

the reactive statement got re-executed, thus the infinite loop.

so you can think that the reactive statement by itself has a "cyclical dependencies".

one way of fixing it is to split the statements,

$: setTimeout(() => {
    dataSource = remoteData.map(i => i);
);

$: value = dataSource[0] ? dataSource[0].value : '';

so reassigning dataSource in reactive statement 1 will only cause the reactive statement 2 to be executed only.

but interestingly, i found that the infinite-loop doesn't happen if dataSource is updated synchronously, it feels like a bug to me instead 🤔

@niyan-ly
Copy link
Author

@tanhauhau Thanks for your solution and explanation. I now get to know what the problem is, but still, that code seems quite qualified, and may spend some others lots of time to find out why. Hope you guys could fix this some day. Also, i will try to dig into svelte to see what i can do to help this.

@vipero07
Copy link

The reactive statements in svelte automatically subscribe to svelte variables internal to the function. Whenever one of these variables changes it indicates to svelte to fire off the reactive statement again. If you were to define the settimeout function outside of the reactive statement it would work fine since it would no longer be part of the dependencies for that reactive statement. E.G.

https://svelte.dev/repl/654688fd61a9415ab254d7769d24adca?version=3.18.2
https://svelte.dev/repl/bd65f2c22e8642dda9e8e812872b54eb?version=3.18.2

If you are familiar with react its similar to how the dependency array for their hooks works except they are managed automatically by the compiler. Meaning the svelte compiler sees variables being set in the reactive statement and automatically adds those to the dependency array. I'm not sure there would be a good fix for this issue as it requires developers to manually indicate which dependencies they want (or don't want). The best solution would be to clarify how the statements work or view value changes in the documentation I think.

@niyan-ly
Copy link
Author

@vipero07 Well, it looks like that, in your code, the svelte compiler will decide the dependency-chain only by where the variable appears, it would require great care to maintain such code. It's true that it is hard to know the exact relations between variables in compile time, especially in dynamic language, but i think tools should be as easy to use as possible. This feature deserves an improvement. Again, thanks for your solution.

@Conduitry
Copy link
Member

I don't think this is a bug, I believe there's a TODO ticket somewhere about adding a section to the site where we get more in depth into how reactive assignments and blocks work, but the short version is that any instance of a local variable inside a $: block (besides instances on the left hand side of an assignment) results in a dependency on that variable, and whenever a variable is updated, we schedule calls to all of the reactive blocks that depend on it. In this case, that results in an infinite loop. If you need to 'hide' a dependency from a reactive block, put it in a function defined outside the block that you call from inside the block.

It's true that there are additional cases that might be nice to handle that it would be possible to identify statically at compile time, but in general we do not want to add these, as it would make the logic for what Svelte sees as dependencies harder and harder to understand and harder and harder to document. Catching all such cases at compile time is impossible, and so rather than having an ever-shifting and nebulous set of edge cases (which of these are bugs and which of these are limitations we need to document somewhere?), we opt for the simplest-to-understand set of rules as we can.

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

No branches or pull requests

5 participants