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

Unexpected Update Calls #3290

Closed
deviprsd opened this issue Jul 26, 2019 · 6 comments
Closed

Unexpected Update Calls #3290

deviprsd opened this issue Jul 26, 2019 · 6 comments
Labels

Comments

@deviprsd
Copy link

deviprsd commented Jul 26, 2019

Describe the bug
beforeUpdate is called twice when using bind:this to the same variable in logic blocks. Even though afterUpdate is schedule to be called after the last beforeUpdate it isn't

UPDATE: Exists even if bound to two different variables

Logs
REPL log

> Before null 
> After <button class="ui button">
> Before <button class="ui button">

To Reproduce
https://svelte.dev/repl/d85b4344f4044c4489b4e0260eabf475?version=3.6.8 (same variables)
https://svelte.dev/repl/7b1deb63003544358a6c4ee10ae28dcf?version=3.6.8 (different variables)

Expected behavior
If a single cycle isn't achieved, it should complete the cycle

> Before null 
> After <button class="ui button">
> Before <button class="ui button">
> After <button class="ui button">

Stacktraces

Stack trace
Error: "Custom message to generate Stack trace: beforeUpdate #counter-0 elasped-time(ms) 4"
    instance Button.svelte:43
    run index.mjs:18
    run_all index.mjs:24
    init index.mjs:1249
    Button bundle.js:896
    create_else_block$1 bundle.js:1105
    create_fragment$2 bundle.js:1316
    init index.mjs:1250
    SvelteMantic bundle.js:1403
    app index.js:9
    <anonymous> bundle.js:1429
Button.svelte:45:22

Error: "Custom message to generate Stack trace: afterUpdate #counter-1 elasped-time(ms) 18"
    instance Button.svelte:55
    flush index.mjs:570
    init index.mjs:1299
    SvelteMantic bundle.js:1403
    app index.js:9
    <anonymous> bundle.js:1429
bundle.js:826:18

Error: "Custom message to generate Stack trace: beforeUpdate #counter-2 elasped-time(ms) 20"
    instance Button.svelte:43
    run index.mjs:18
    run_all index.mjs:24
    update index.mjs:604
    flush index.mjs:560
    init index.mjs:1299
    SvelteMantic bundle.js:1403
    app index.js:9
    <anonymous> bundle.js:1429
Button.svelte:45:22

Error: "Custom message to generate Stack trace: Flush() elapsed-time(ms) 21"
    flush index.mjs:582
    init index.mjs:1299
    SvelteMantic bundle.js:1403
    app index.js:9
    <anonymous> bundle.js:1429
index.mjs:584:8

Error: "Custom message to generate Stack trace: Flush() elapsed-time(ms) 22"
    flush index.mjs:582
index.mjs:584:8

Information about your Svelte project:

  • Your browser and the version: Firefox 67

  • Your operating system: Ubuntu 18 LTS

  • Svelte version 3.6.8

  • Rollup

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

@bwbroersma
Copy link
Contributor

Probably simpler example:
https://svelte.dev/repl/8a96e6e545244bff9fd0125b152d6840?version=3.6.9

So it seems that the bind:this will create binding_callbacks in flush which will invalidate the bound variable:

while (binding_callbacks.length) binding_callbacks.pop()();

This invalidate will add a dirty component and therefore execute the loop again, triggering an extra beforeUpdate and an extra afterUpdate, but his special seen_callbacks code will prevent the afterUpdate being triggered again for the same cycle:
// then, once components are updated, call
// afterUpdate functions. This may cause
// subsequent updates...
for (let i = 0; i < render_callbacks.length; i += 1) {
const callback = render_callbacks[i];
if (!seen_callbacks.has(callback)) {
callback();
// ...so guard against infinite loops
seen_callbacks.add(callback);
}

This checking on seen_callbacks is in de code since the reactive assignments have been implemented, so basically since Svelte 3.

@deviprsd
Copy link
Author

deviprsd commented Jul 29, 2019

So it seems that the bind:this will create binding_callbacks in flush which will invalidate the bound variable:

while (binding_callbacks.length) binding_callbacks.pop()();

This invalidate will add a dirty component and therefore execute the loop again, triggering an extra beforeUpdate and an extra afterUpdate, but his special seen_callbacks code will prevent the afterUpdate being triggered again for the same cycle:

// then, once components are updated, call
// afterUpdate functions. This may cause
// subsequent updates...
for (let i = 0; i < render_callbacks.length; i += 1) {
const callback = render_callbacks[i];
if (!seen_callbacks.has(callback)) {
callback();
// ...so guard against infinite loops
seen_callbacks.add(callback);
}

This checking on seen_callbacks is in de code since the reactive assignments have been implemented, so basically since Svelte 3.

Exactly, it took me a while to figure it while I was going through my bundle js file but it seems that if the bindings are invalidated before the dirty components are scheduled for an update the beforeUpdate may run multiple times but will end in an afterUpdate, i.e, basically move this

while (binding_callbacks.length) binding_callbacks.pop()();

before (this can lead to unpredictable side effects, but I can't think of any)

while (dirty_components.length) {
const component = dirty_components.shift();
set_current_component(component);
update(component.$$);
}

resulting in

> Before
> Before
> After

or just

> Before
> After

@pckhoi
Copy link

pckhoi commented May 12, 2020

This is pretty severe. Can't build anything serious with Svelte until this is fixed.

@Conduitry
Copy link
Member

This appears to have been fixed as of 3.18.2.

@beomy
Copy link

beomy commented Sep 24, 2020

It still have the same problem.
https://svelte.dev/repl/3381c9b40dc8441a8e4ebfa48fd8c178?version=3.26.0

REPL Logs
"beforeUpdate"
"afterUpdate"
"beforeUpdate"

@larbear
Copy link

larbear commented Feb 3, 2021

Issue seems to still exist. beomy's example replicates it nicely. Commenting in hopes of a re-open.

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