Fix: always reset the evaluation context upon entering an untracked block

jviide commented 4 months ago

This pull request fixes an issue with multiple levels of mixed effect and untracked calls:

import { signal, effect, untracked } from "@preact/signals-core";

const s = signal(0);

untracked(() => {
  effect(() => {
    untracked(() => {
      console.log(s.value); // <-- s.value gets tracked while inside untracked(...)
    });
  });
});
// 0 gets printed to the console

s.value = 1;
// 1 gets printed to the console

The expected behavior is that the effect should not run when s.value is set to 1, because s.value was only used inside untracked. Therefore the effect should not become dependent on s.value.

The root cause is that only the outermost level of nested untracked() calls sets the evaluation context to undefined (which disables tracking). The fix is to unconditionally set the evaluation context to undefined inside every untracked call. The untrackedDepth counter then becomes unused.

The first commit of this pull request adds a test for the issue. The second commit applies the fix and removes the untrackedDepth counter.

The last commit does some minor refactoring: it just moves all tests related to untracked into their own section in the test file and modifies the test names accordingly. The last commit can be dropped if it feels superfluous.

changeset-bot[bot] commented 4 months ago

⚠️ No Changeset found

Latest commit: e357b4d1451d388a0844ba7a50a07f402ad2c557

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

netlify[bot] commented 4 months ago

Deploy Preview for preact-signals-demo ready!

Name	Link
Latest commit	e357b4d1451d388a0844ba7a50a07f402ad2c557
Latest deploy log	https://app.netlify.com/sites/preact-signals-demo/deploys/65de9cbe94e54900084abb3c
Deploy Preview	https://deploy-preview-512--preact-signals-demo.netlify.app
Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

preactjs / signals