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: address $effect feedback #15107

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

docs: address $effect feedback #15107

wants to merge 12 commits into from

Conversation

benmccann
Copy link
Member

We've seen that $effect is being widely / wildly overused as Svelte 5 is adopted. This PR doesn't change the contents of the $effect docs much, but hopefully makes it easier to notice and retain some key pieces of information.

This makes a few changes:

  • moves $effect down below $props in the sidebar. This sidebar seems to be sorted by order of importance, so we can reorder to signal that $effect should be used less than $props
  • remove sentence about how Svelte uses effects internally. It's not important for helping understand how to use an effect
  • add a note regarding when effects run as this is the other thing users are frequently encountering difficulty with. Moved this section down a bit both because it seems to make sense to show an example first and we don't want two notes in close succession
  • moved down details about running in a microtask and being batched as these seem like more technical details that are better introduced a bit later

Copy link

changeset-bot bot commented Jan 24, 2025

⚠️ No Changeset found

Latest commit: b27ca05

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

@svelte-docs-bot
Copy link

Copy link
Contributor

Playground

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

@trueadm
Copy link
Contributor

trueadm commented Jan 24, 2025

Do we need a section to say that effects don't run on the server, and also they don't run in places where you need an active effect already, like an event listener or module instance?

@dummdidumm
Copy link
Member

These are good changes! I'm not too sure about the ordering change in the side bar but I don't mind either way.

they don't run in places where you need an active effect already

Do you mean that they need an effect parent. That is already stated in the docs pretty far up.

I also think we should add something along the line of @paoloricciuti's recent "state in a derived" example to the "when not to use effects" section

@max-got
Copy link

max-got commented Jan 25, 2025

imo to prevent the overuse of $effect the docs/Tutorial should have examples, that showcase common alternatives where people would use $effect otherwise. the docs should have best practice examples

@sacrosanctic
Copy link
Contributor

sacrosanctic commented Jan 25, 2025

Should have a list of all the places where svelte has encouraged the use of $effect

  • in use:action
  • it being equivalent to onMount

@sacrosanctic
Copy link
Contributor

If an API is documented, it's generally assumed it's available for use. So the first line should include a warning explicitly stating that it should not be used, rather than beginning with a standard paragraph explaining its functionality.

@benmccann
Copy link
Member Author

in places where you need an active effect already, like an event listener or module instance?

Is this an exhaustive list? If not, how would users understand where you need an active effect?

Do you mean that they need an effect parent. That is already stated in the docs pretty far up.

The term "parent effect" appears three times, but is never defined. Is a parent/child effects when you do something like $effect(() => { $effect(() => {}) })? Why/when would you do that?

I also think we should add something along the line of @paoloricciuti's recent "state in a derived" example to the "when not to use effects" section

Yeah, that makes sense unless we end up adding $state.link, but could probably be its own PR as it seems like a sizable update by itself.

Should have a list of all the places where svelte has encouraged the use of $effect
in use:action
equivalent to onMount

The use: docs mention this, which I think is a good place to put it. I'll update the onMount docs as well. (side note, onMount is equivalent to $effect(() => { untrack(fn) })`` rather than just $effect`)

If an API is documented, it's generally assumed it's available for use. So the first line should include a warning explicitly stating that it should not be used, rather than beginning with a standard paragraph explaining its functionality.

It is available for use. It's just important to use it as intended. The lightbulb callout has been moved up two paragraphs and now is right at the beginning, so I think it will be pretty prominent

@Ocean-OS
Copy link
Contributor

in places where you need an active effect already, like an event listener or module instance?

Is this an exhaustive list? If not, how would users understand where you need an active effect?

That is the use case for $effect.active, but for now we could just say something like "if the code isn't executed when a component is running, you can't use an $effect."

Do you mean that they need an effect parent. That is already stated in the docs pretty far up.

The term "parent effect" appears three times, but is never defined. Is a parent/child effects when you do something like $effect(() => { $effect(() => {}) })? Why/when would you do that?

I'm pretty sure "parent effect" refers to $effect.root.

@trueadm
Copy link
Contributor

trueadm commented Jan 26, 2025

I'm pretty sure "parent effect" refers to $effect.root.

The effect root can be the parent bit so can other $effect and $effect.pres. In fact internally Svelte creates lots of different types of effect for things like if blocks and each blocks. These too can be parents.


Most of the effects in a Svelte app are created by Svelte itself — they're the bits that update the text in `<h1>hello {name}!</h1>` when `name` changes, for example.
> [!NOTE] Your application will become a nightmare to deal with almost immediately if you try to update state inside an effect. If you're tempted to use an effect to update state, see [when not to use `$effect`](#When-not-to-use-$effect) to learn about alternative approaches.
Copy link
Member

Choose a reason for hiding this comment

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

this wording feels a bit strong. if we're so adamant that you shouldn't update state inside an effect, why do we allow it? it also feels quite vague — what is the nightmare exactly? it would be good if we could articulate why we warn against this

Copy link
Member Author

Choose a reason for hiding this comment

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

if we're so adamant that you shouldn't update state inside an effect, why do we allow it?

I would love if someone could answer why we allow it. It causes so many problems and we could save users from themselves

Copy link
Member

Choose a reason for hiding this comment

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

Because it is unavoidable in some scenarios. Angular actually had a setting where writing inside effects was disallowed and they removed it because it caused so many headaches (and didn't actually help, most people just turned on the option)

Copy link
Member Author

Choose a reason for hiding this comment

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

It might be useful to have an example of where it is unavoidable. I have yet to see such a scenario - though it can sometimes be tricky to figure out how to write the code in such a way where you avoid it (e.g. needing the trick Paolo was sharing) and I frequently see people using it as a shortcut

Copy link

@ahkelly ahkelly Feb 6, 2025

Choose a reason for hiding this comment

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

What is the correct solution then to sveltejs/kit#12999 if not 3.2? I would never need to use $effect() if this was solved. Offering the boiler plate "use a callback" as an option has not worked for me in very specific situations.

Copy link
Member

Choose a reason for hiding this comment

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

So yeah, you cannot write directly to data...but with my suggestion you don't have to write to data, you have to write to the writable_data that is the returned from the $derived.by...the $derived.by dance is only important to make sure that whenever data is invalidated by sveltekit it will reset to the "right" value.

Copy link

Choose a reason for hiding this comment

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

I understand what you are saying but I am not writing to data but reading from the data. Using $derived.by is not picking up the newest invalidated data in certain circumstances like I note above. In these situations only $effect is picking up the change. In 95%+ of situations simply using data.variable_name will always give you the reactive data value like it did 100% of the time in svelte 4. In the other 5%, which you can find numerous reports in the discord of the same since the svelte 5 migration only using $effect works as noted in the issue I provided as well as here #14536 (comment).

I only waste bandwidth here on this as the maintainers seem hellbent on removing $effect (and seemingly bind:), which I am ok with but they need to first fix these lingering edge cases that are new to svelte 5/kit. I apologize for the interruption and wasting of your time on this, please don't waste anymore time on it as I am sure others (than me) with more influence/understanding of the issue will eventually solve/revert it.

Copy link
Member

Choose a reason for hiding this comment

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

Using $derived.by is not picking up the newest invalidated data in certain circumstances like I note above

What i'm asking you then is to open an issue with the reproduction of this...because to me it doesn't make sense since the dependency resolution for both derived and effect is the same. If you can provide such example it would be very valuable since if this is a bug we need to fix it. The steps to reproduce you provided are vague and i don't see why doing them with derived instead of effect should change anything.

Copy link

Choose a reason for hiding this comment

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

Completely understand and thank you. I will endeavour to come up with a generic reproduction but not sure how I simulate the data request to our backend in a repl.

Copy link
Member

Choose a reason for hiding this comment

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

You can use https://sveltelab.dev to make a sveltekit reproduction...and obviously your backend should not be involved in the minimal reproduction. Just try to reproduce the "async behavior" you are seeing

@Rich-Harris
Copy link
Member

I prefer the existing order, because it covers reactivity as a group and then moves onto component composition.

Something lost in this PR is the explanation that everything Svelte does on your behalf (updating expressions in the template etc) is also an effect. I think it's helpful to keep that in some form, because it helps clarify what effects are

@sacrosanctic
Copy link
Contributor

Users often cannot distinguish between an effect and a side effect. Why not define them?

"In the context of Svelte's reactivity system, changes to $state is an effect, and changes outside of that is a side effect. $effect is used for side effects like call a third-party library, draw on a <canvas> element, or make a network request. The HTML markup in a component is also a side effect. And svelte wraps that in an $effect under the hood.

@dummdidumm
Copy link
Member

Something lost in this PR is the explanation that everything Svelte does on your behalf (updating expressions in the template etc) is also an effect. I think it's helpful to keep that in some form, because it helps clarify what effects are

I disagree - I find the new wording with the given examples (drawing on canvas etc) better. It's a nice-to-know but ultimately an implementation detail.

@@ -32,9 +28,13 @@ Your effects run after the component has been mounted to the DOM, and in a [micr
<canvas bind:this={canvas} width="100" height="100" />
```

Re-runs are batched (i.e. changing `color` and `size` in the same moment won't cause two separate runs), and happen after any DOM updates have been applied.
When Svelte runs an effect function, it tracks which pieces of state (and derived state) are accessed (unless accessed inside [`untrack`](svelte#untrack)), and re-runs the function when that state later changes. Internally, Svelte uses effects to manage changes to the template — e.g. to update the text in `<h1>hello {name}!</h1>` when `name` changes.
Copy link
Member

Choose a reason for hiding this comment

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

That sentence feels weird there. If Rich is keen on keeping it I would put it in a separate "did you know?" note box, like

> [!NOTE] Internally, Svelte uses effects to manage changes to the template — e.g. to update the text in `<h1>hello {name}!</h1>` when `name` changes.
Suggested change
When Svelte runs an effect function, it tracks which pieces of state (and derived state) are accessed (unless accessed inside [`untrack`](svelte#untrack)), and re-runs the function when that state later changes. Internally, Svelte uses effects to manage changes to the template — e.g. to update the text in `<h1>hello {name}!</h1>` when `name` changes.
When Svelte runs an effect function, it tracks which pieces of state (and derived state) are accessed (unless accessed inside [`untrack`](svelte#untrack)), and re-runs the function when that state later changes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should be called markup per https://svelte.dev/docs/svelte/svelte-files

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that would draw even more attention to it and I'm not sure it's something users need to know at all. I'd rather include it a bit more inconspicuously if we really need to keep it


Most of the effects in a Svelte app are created by Svelte itself — they're the bits that update the text in `<h1>hello {name}!</h1>` when `name` changes, for example.
> [!NOTE] Updating state inside an effect is considered an anti-pattern and may lead to never-ending cycles of state updates. If you're tempted to use an effect to update state, see [when not to use `$effect`](#When-not-to-use-$effect) to learn about alternative approaches.
Copy link
Contributor

Choose a reason for hiding this comment

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

change anti-pattern to escape hatch to align with existing docs usage.

@kwangure
Copy link
Contributor

kwangure commented Feb 3, 2025

As a side note, the $effect page feels relatively "complete" now. Because it has received a lot of attention (for good reason), it puts pages like $inspect and others to shame.

I already know these things about $effect from usage and experience, but it's nice to read them and pocket confident explanations for the $effect rituals I do.

I know "things" about $inspect and friends too, but can Claude exhaustively use $inspect in all relevant contexts correctly based on the documentation and its general knowledge? Probably not.

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.

10 participants