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

docs: clarify example on dynamic dependency tracking #13767

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

BastiDood
Copy link

Description

This PR clarifies the explanation behind the short-circuiting a || b example used in the $effect guide. I personally felt like the original explanation was too short and too handwavy over the fascinating implications of dynamic dependency tracking. So, I expanded the explanation to cover both cases (i.e., when a is false and when a is true).

Before submitting the PR, please make sure you do the following

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • Prefix your PR title with feat:, fix:, chore:, or docs:.
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests and linting

  • Run the tests with pnpm test and lint the project with pnpm lint

Copy link

changeset-bot bot commented Oct 21, 2024

⚠️ No Changeset found

Latest commit: 1e2afef

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

@BastiDood BastiDood force-pushed the conditional-effect-clarification branch 2 times, most recently from 21c7c85 to 01197d6 Compare October 22, 2024 08:09
@max-got
Copy link

max-got commented Oct 22, 2024

Maybe we could add a potential workaround, to further clarify the behaviour?

Example:

Escape Hatch: Forcing Effect Re-Runs on Every State Change

If you really need to ensure that the effect always runs when either a or b changes, regardless of the short-circuit behavior of the || operator, you can explicitly reference both values in the effect:

	let a = $state(false);
	let b = $state(false);

	$effect(() => {
		// Explicitly reference `a` and `b` so this effect re-runs whenever they change
		a;
		b;

		// This will run on every change to either `a` or `b`
		console.log('running');

		if (a || b) {
			console.log('inside if block');
		}
	});

This approach establishes direct dependencies on both a and b by reading them within the effect. It ensures the effect re-runs whenever either value changes, but use it cautiously.

Note: There are usually better ways to fix this issue. With this escape hatch, console.log("running") will trigger on every change to either a or b, possibly causing unnecessary re-renders. (Demo)

@BastiDood BastiDood force-pushed the conditional-effect-clarification branch from 01197d6 to 36116fc Compare October 22, 2024 20:46
@BastiDood
Copy link
Author

We could do something like that. Though, my original scope for this PR was to simply expound on the a || b example in terms of a case analysis to better illuminate why b wouldn't be depended on. I feel that an additional example is already beyond what I intended for this PR specifically and is thus more appropriate as a follow-up PR after this one.

I'll wait on the Svelte team to chime in on how to best proceed here.

5dfnnpp7fp

This comment was marked as spam.

@BastiDood
Copy link
Author

Hello there! Just gonna bump this PR. I'd like to know if there are any concerns with the way I worded the clarification (and possibly improvements if necessary). Thanks again!

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

Successfully merging this pull request may close these issues.

4 participants