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

beforeUpdate called twice with bound reference #6016

Closed
larbear opened this issue Feb 23, 2021 · 8 comments
Closed

beforeUpdate called twice with bound reference #6016

larbear opened this issue Feb 23, 2021 · 8 comments
Labels
bug compiler Changes relating to the compiler

Comments

@larbear
Copy link

larbear commented Feb 23, 2021

Describe the bug
This is a duplicate of #3290 which was closed, though the issue still exists.

beforeUpdate is called twice when using bind:this.

Logs
No errors were shown in logs

To Reproduce
https://svelte.dev/repl/3381c9b40dc8441a8e4ebfa48fd8c178 (credit to beomy)

<script>
  import { beforeUpdate, afterUpdate } from 'svelte';
  let p;

  beforeUpdate(() => {
    console.log('beforeUpdate');
  })

  afterUpdate(() => {
    console.log('afterUpdate');
  })
</script>

<p bind:this={p}></p>

Open the console to view the behavior. It produces:

beforeUpdate
afterUpdate
beforeUpdate

Expected behavior
beforeUpdate should only be called once during a single update cycle.

beforeUpdate
afterUpdate

Severity
Severity is High, as the the cycle should always end with an afterUpdate and not a repeated beforeUpdate.

Other
#3308, was made to potentially fix this issue but appears that it was never merged. The reason given was that it was fixed in 3.18.2, but the issue still exists. https://svelte.dev/repl/3381c9b40dc8441a8e4ebfa48fd8c178?version=3.18.2.

@deviprsd
Copy link

deviprsd commented Apr 30, 2021

I might not have realized when I submitted them but there is one important concept change to my PR #3308.

It chooses to call the bindings before the beforeUpdate and that fixes the issue but that could have many possible side-effects

@dummdidumm dummdidumm added bug compiler Changes relating to the compiler labels Aug 26, 2021
@RaiVaibhav
Copy link
Contributor

RaiVaibhav commented Oct 13, 2021

It happened because of the update call on the dirty component from schedulers i.e. when variable bind to the this, svelte mark it as a dirty and call the scheduler update

function update($$) {
	if ($$.fragment !== null) {
		$$.update();
		run_all($$.before_update);
		const dirty = $$.dirty;
		$$.dirty = [-1];
		$$.fragment && $$.fragment.p($$.ctx, dirty);

		$$.after_update.forEach(add_render_callback);
	}
}

In the above before_update gets called again and the same will happen if there are any reactive statements.
Since I don't know the reason for calling the before update again, so I will be trying to dig deep into this issue and propose some initial solution.

@mihaon
Copy link

mihaon commented Apr 28, 2022

The issue still exists. Tested on Svelte 3.47.0.

@claudiofelber
Copy link

The issue still exists in Svelte 3.52.0.

@larbear
Copy link
Author

larbear commented Dec 27, 2022

Issue still exists in Svelte 3.55. Quick and hacky work-around by checking if the variable to store reference has been set:

https://svelte.dev/repl/28d44e6712604fe0b52c632e313f2359?version=3.55.0

<script>
  import { beforeUpdate, afterUpdate } from 'svelte';
  let p, count = 0;

  let skipNextUpdate = false;
  beforeUpdate(() => {
    if (!skipNextUpdate) {
      // Do beforeUpdate logic here.
      console.log("beforeUpdate");
    }
    skipNextUpdate = !p;
  })

  afterUpdate(() => {
    console.log("afterUpdate");
  })
</script>

<p bind:this={p}>{count}</p>
<button on:click={() => count++}>Counter Up!</button>

peterpeterparker added a commit to dfinity/gix-components that referenced this issue Dec 30, 2022
…n bindings are used (#142)

# Motivation

We disable the `<InfiniteScroll />` component `beforeUpdate` because we do want to trigger an intersection in case of layout shifting - i.e. we do not want to trigger additional unexpected query and update calls in the proposal list views.

We noticed that under certain circumstances, notably when initialized while enabling at the same time, a race condition can lead to the scroller being disabled.

Root cause of the problem is a Svelte issue [#6016](sveltejs/svelte#6016). Indeed, when bindings are used (`bind:this=my_HTML_element`) within the compoment, the `beforeUpdate` lifecycle is called twice while `afterUpdate` is called only once. Therefore, as we disable the scroll in the first method and enable in the second, the scroller remains disabled.

# Reproduction

See PR #141 [comment](#141 (comment)).

# Changes

- implement a workaround to ignore second `beforeUpdate` call trigger by binding initialization
@larbear
Copy link
Author

larbear commented Jun 24, 2023

This issue still exists 4.0.0.

@yuinchien
Copy link

This issue still exists 4.0.5.

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

Successfully merging a pull request may close this issue.

7 participants