Skip to content

fix: ensure derived effects are recreated when needed #16595

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mtsgrd
Copy link

@mtsgrd mtsgrd commented Aug 10, 2025

Problem

Effects created inside $derived statements are not re-executed when the derived is accessed after being disconnected and reconnected to the reactive dependency graph.

Reproduction scenario:

  1. Component A creates a derived with effects: const result = $derived(/* contains $effect */)
  2. Component A unmounts before effects have a chance to execute
  3. Component B tries to access the same derived
  4. The effects are never executed, even though the derived value is computed correctly

This happens because when a derived is disconnected from the graph (no more reactions), its effects are destroyed via destroy_derived_effects(). When the derived is later reconnected, the effects are not recreated since the derived appears "clean" and doesn't need re-execution.

works here: https://svelte.dev/playground/719a2a834fd34d02a204ac4b7c65973b?version=5.19.4
does not work: https://svelte.dev/playground/719a2a834fd34d02a204ac4b7c65973b?version=5.19.5

Solution

Introduce a HAS_EFFECTS flag that tracks when a derived has contained effects, even after they've been destroyed. During reconnection, if a derived has the HAS_EFFECTS flag but no current effects, we force re-execution to recreate them.

Changes:

  1. Add HAS_EFFECTS = 1 << 24 constant
  2. Set the flag in effects.js when effects are added to deriveds
  3. Check for missing effects in runtime.js and trigger re-execution when needed

Implementation

// In effects.js - mark derived as having effects
var derived = /** @type {Derived} */ (active_reaction);
(derived.effects ??= []).push(effect);
derived.f |= HAS_EFFECTS;

// In runtime.js - detect and fix missing effects
if (is_dirty(derived)) {
    update_derived(derived);
} else if ((derived.f & HAS_EFFECTS) !== 0 && (derived.effects === null || derived.effects.length === 0)) {
    // If the derived once had effects but they're now missing (destroyed),
    // we need to re-execute to recreate them
    update_derived(derived);
}

Testing

Added a comprehensive test that:

  • Creates a derived with effects in one render context
  • Destroys the context before effects execute (simulating component unmount)
  • Accesses the derived from a new render context
  • Verifies effects execute properly on reconnection

Without fix: 0 effect executions (effects lost forever)
With fix: 1 effect execution (effects recreated on reconnection)

Copy link

changeset-bot bot commented Aug 10, 2025

⚠️ No Changeset found

Latest commit: d4cb4e4

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@mtsgrd mtsgrd marked this pull request as draft August 10, 2025 12:05
@mtsgrd
Copy link
Author

mtsgrd commented Aug 10, 2025

Potential fix for #16594

@mtsgrd mtsgrd marked this pull request as ready for review August 10, 2025 12:54
@mtsgrd mtsgrd force-pushed the missing-effects branch 2 times, most recently from 6d94df6 to e2a1d9f Compare August 10, 2025 13:34
@mtsgrd mtsgrd changed the title fix: fix missing effects if derived added, removed, and re-added as dependency fix: missing effects if derived added, removed, and re-added as dependency Aug 10, 2025
@@ -671,6 +672,10 @@ export function get(signal) {

if (is_dirty(derived)) {
update_derived(derived);
} else if ((derived.f & HAS_EFFECTS) !== 0 && (derived.effects === null || derived.effects.length === 0)) {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new test can be verified by adding && false to the end of this if statement and running pnpm test.

@mtsgrd mtsgrd changed the title fix: missing effects if derived added, removed, and re-added as dependency fix: recreate effects in derived values after disconnection/reconnection cycle Aug 10, 2025
Adds HAS_EFFECTS flag to track when deriveds contain side effects and
re-runs computation when effects are missing after reconnection.
@mtsgrd mtsgrd changed the title fix: recreate effects in derived values after disconnection/reconnection cycle fix: ensure derived effects reconnect after disconnection Aug 10, 2025
Copy link
Contributor

Playground

pnpm add https://pkg.pr.new/svelte@16595

@mtsgrd
Copy link
Author

mtsgrd commented Aug 10, 2025

@mtsgrd mtsgrd changed the title fix: ensure derived effects reconnect after disconnection fix: ensure derived effects are recreated when needed Aug 11, 2025
@mtsgrd
Copy link
Author

mtsgrd commented Aug 11, 2025

@Rich-Harris is there any chance you could have a look at this? It's a small but important detail in the reactivity system.

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

Successfully merging this pull request may close these issues.

1 participant