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: remove section on callbacks and accessor bindings #10750

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

Conversation

Rich-Harris
Copy link
Member

The $effect docs are completely overwhelming. If we want to discourage use of something, we should spend a lot less time talking about it — instead, we currently spend way more time talking about $effect than any other rune.

In particular, I think this section is unnecessary. The example doesn't really resemble anything you'd find in real code, and it makes everything seem a lot more complicated than it needs to be. We should also avoid making up terms like 'writable $derived' without explaining them.

When we update the tutorial to cover Svelte 5, we can afford to spend time in more of these nooks and crannies. Until then, we should remove this.

Copy link

changeset-bot bot commented Mar 10, 2024

⚠️ No Changeset found

Latest commit: b75734a

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

@Rich-Harris Rich-Harris changed the title remove section on callbacks and accessor bindings docs: remove section on callbacks and accessor bindings Mar 11, 2024
Copy link
Member

@dummdidumm dummdidumm left a comment

Choose a reason for hiding this comment

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

Hard disagree. These two specifically were referenced multiple times as a "no don't do it like this, see the docs" in Discord already (the one you want to keep is much more obvious which people don't have as much trouble with). These docs need to be in-depth (for comparison, look at how in-depth the React docs are on useEffect; they have an entire page that is longer than our complete runes page for this). We can certainly reorder things in the final docs, move stuff around (but not into the tutorial), but I vote for keeping it here in the meantime. Not having thorough docs from day 0 means people go down the wrong paths more likely, making them apply workarounds for workarounds etc etc. Putting them on the right track sets them up for success.
Removing it now also means we're much more likely to forget about adding in-depth explanation about this in the final docs.

@benmccann
Copy link
Member

$effect has been very overused leading people to talk about "$effect HELL", etc. Since these docs were added I've seen people answer their questions and correct others bad patterns pointing at these docs and think they've done lots of good in tamping down the overuse of $effect. While the tutorial does allow for covering things in a more in depth fashion, I think it'd still fair to include details in the reference docs about when and when not to use a certain rune. And in the short term, before we have such a tutorial, it helps to have our early adopters using better patterns as they answer questions for other users, create content of their own, etc.

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.

None yet

3 participants