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

Current component is not cleared after flush #4899

Closed
rdb opened this issue May 25, 2020 · 5 comments · Fixed by #4909
Closed

Current component is not cleared after flush #4899

rdb opened this issue May 25, 2020 · 5 comments · Fixed by #4909
Labels

Comments

@rdb
Copy link
Contributor

rdb commented May 25, 2020

Describe the bug
The current component is not being cleared at the end of the flush call. This means that lifecycle callbacks, like onMount, onDestroy, etc. are made available outside of Svelte code, with unpredictable effects (effectively operating on a random component, possibly one that is already destroyed).

This looks like a simple oversight. It looks like a call to set_current_component(null) is missing after this loop:

for (let i = 0; i < dirty_components.length; i += 1) {
const component = dirty_components[i];
set_current_component(component);
update(component.$$);
}

To Reproduce

<script>
  import { get_current_component } from 'svelte/internal';

  let a = 0;

  // Do something to trigger a flush after half a second
  setTimeout(() => {
    a = 1;
  }, 500);

  // Print current component after 1 second - should give error
  // Instead, it prints the last-updated component
  setTimeout(() => {
    console.log(get_current_component());
  }, 1000);
</script>

<p>{a}</p>

Expected behavior
I expected an exception to be thrown after a second (which it indeed does if you comment out the
first setTimeout). Instead, it logs the most recently-updated component, which is not at all guaranteed to be the current one.

Severity
I'm currently stuck on this with a package I'm maintaining. I have found a workaround (importing tick from svelte-internal and then monkey-patching tick().then to reset the component after flush) but it is extremely hacky and I would not want to put this in a hosted library.

Additional context
I'm maintaining a helper package which enables closer integration between Svelte and Meteor. It registers an onDestroy callback to handle cleanup of certain resources, but the function in question needs to be able to work both inside and outside Svelte components, hence the need to be able to trust that the current component is cleared.

@Conduitry
Copy link
Member

Duplicate of #4259. The fix does not seem to be that simple though, as seen in the failing tests in #4864.

@rdb
Copy link
Contributor Author

rdb commented May 25, 2020

I did see #4259, but it is specifically about onMount, and this issue is much more generic than that. If you are going to leave this closed, perhaps you can edit #4259 to include the scope that this bug
report covers?

If set_current_component(null) after the for loop introduces complications, this issue could still be trivially fixed by putting it at the end of the flush call, which would fix this issue while leaving the onMount bug in #4259 unaddressed.

@Conduitry
Copy link
Member

That might be reasonable. Reopening.

@rdb
Copy link
Contributor Author

rdb commented Jun 26, 2020

To anyone else waiting for #4909 to be merged, it is possible to work around this using the following code:

import { set_current_component, tick } from 'svelte/internal';

const promise = tick();
const oldThen = promise.then;
promise.then = function (fn) {
  oldThen.call(promise, () => {
    fn();
    set_current_component(null);
  });
};

rdb added a commit to rdb/svelte-meteor-data that referenced this issue Jun 26, 2020
Without this, unpredictable crashes may occur in seemingly random places.  This will be reverted as soon as sveltejs/svelte#4909 is merged.
@Conduitry
Copy link
Member

That component now throws an exception at runtime in 3.25.0.

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