NotificationsYou must be signed in to change notification settings
Fork4.6k
Star83.4k

fix: add setter to`$derived` class fields#15628

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

Rich-Harris merged 1 commit intosveltejs:mainfromOcean-OS:derived-setter

Mar 31, 2025

Merged

fix: add setter to`$derived` class fields#15628

Rich-Harris merged 1 commit intosveltejs:mainfromOcean-OS:derived-setter

Mar 31, 2025

Conversation

Copy link

Contributor

Ocean-OS commentedMar 27, 2025

Fixes#15621 by adding a setter to$derived class fields.

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 withfeat:,fix:,chore:, ordocs:.
This message body should clearly illustrate what problems it solves.
Ideally, include a test that fails without this PR but passes with it.
If this PR changes code withinpackages/svelte/src, add a changeset (npx changeset).

Tests and linting

Run the tests withpnpm test and lint the project withpnpm lint

init

03d035a

Copy link

changeset-botbot commentedMar 27, 2025

🦋 Changeset detected

Latest commit:03d035a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
svelte	Patch

Not sure what this means?Click here to learn what changesets are.

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

Copy link

svelte-docs-botbot commentedMar 27, 2025

Preview:https://svelte-dev-git-preview-svelte-15628-svelte.vercel.app/

Copy link

Contributor

github-actionsbot commentedMar 27, 2025

Playground

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

Copy link

Member

dummdidumm commentedMar 28, 2025

Looks good - add a test and we can merge this 👍

Copy link

ContributorAuthor

Ocean-OS commentedMar 29, 2025

Just realized I haven't implemented this on the server, how should I go about doing that? I'm not sure if we should allow setting it, or if we should have a helper function for it. I'm thinking more towards a helper function, that'd look like this:

functionserver_derived(fn){letvalue;letcomputed_wv=1;letwrite_wv=1;letlast_computed_value;letwv=1;functionread(){if(computed_wv<write_wv){letval=fn();if(val!==last_computed_value){value=val;computed_wv=++wv;last_computed_value=val;}}if(write_wv===1){if(computed_wv===1){value=fn();computed_wv=++wv;last_computed_value=value;}else{letval=fn();if(val!==last_computed_value){value=val;computed_wv=++wv;last_computed_value=val;}}}returnvalue;}functionwrite(next){write_wv=++wv;value=next;}return[read,write];}

Copy link

Member

dummdidumm commentedMar 31, 2025

@paoloricciuti started something related in#14977, maybe it's best to not worry about writable derived class fields for now (if you eagerly update them during server rendering, then wtf are you even doing) and incorporate that into#14977 instead.