Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

feat: add$state.opaque rune#14639

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

Closed
trueadm wants to merge14 commits intomainfromopaque-rune
Closed

feat: add$state.opaque rune#14639

trueadm wants to merge14 commits intomainfromopaque-rune

Conversation

trueadm
Copy link
Contributor

@trueadmtrueadm commentedDec 9, 2024
edited
Loading

This PR adds the$state.opaque rune. This is a special kind of rune designed to solve problems with handling and managing state from external sources/libraries.

Specifically, for cases where Svelte is not involved in understanding that of the reactivity of the thing you're passing in – thus being opaque to Svelte. In order to let Svelte 5 know that something has changed, an invalidate function is provided so you can manually control letting Svelte know it should invalidate any reactive dependencies on this piece of state:

<script>let [count, invalidate_count]=$state.opaque(0);</script><buttononclick={()=>count++ }>+</button><buttononclick={()=>invalidate_count() }>invalidate</button><div>{count}</div>

This means thatreassignments andmutations to opaque state will not trigger reactive updates. Youalways need to invoke the invalidate function. Furthermore, this means that you will likely need to adopt the recent function bindings feature if you intend to use opaque state withbind:something.

Closes#14520.

gterras, jjones315, mjadobson, whalderman, olivierchatry, fozcode, rafaelavd, knd775, Jerboas86, edsunman, and 7 more reacted with thumbs up emojigyurielf reacted with rocket emojiOcean-OS, xeho91, and yurivish reacted with eyes emoji
@changeset-botchangeset-bot
Copy link

changeset-botbot commentedDec 9, 2024
edited
Loading

🦋 Changeset detected

Latest commit:21b0865

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

This PR includes changesets to release 1 package
NameType
svelteMinor

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

@Rich-Harris
Copy link
Member

preview:https://svelte-dev-git-preview-svelte-14639-svelte.vercel.app/

this is an automated message

@trueadmtrueadm changed the titlefeat: add$state.opqaue runefeat: add$state.opaque runeDec 9, 2024
@github-actionsGitHub Actions
Copy link
Contributor

Playground

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

@trueadm
Copy link
ContributorAuthor

TODO: when we land$inspect.trace we need to add a call-site to this state so it's name is traced too.

@Rich-Harris
Copy link
Member

Given the syntactical requirements, I assume there's no way to make this work with classes?

@trueadm
Copy link
ContributorAuthor

Given the syntactical requirements, I assume there's no way to make this work with classes?

Nope. I think that’s fine though

Ocean-OS reacted with thumbs up emoji

@Ocean-OS
Copy link
Contributor

Ocean-OS commentedDec 12, 2024
edited
Loading

Given the syntactical requirements, I assume there's no way to make this work with classes?

Nope. I think that’s fine though

Couldn't something like this be done? (apologies for the terrible spacing)

classThing{constructor(){([this.stuff,this.invalidate]=$state.opaque(stuff));}}

Couldn't the private property for the$state object be added in compile time?
There could be a separate internalopaque_state function that specifically works with classes.

@dummdidumm
Copy link
Member

@trueadm
Copy link
ContributorAuthor

I don't think we should optimise for the usage of$state.opaque on classes, as long as there are ways to make it work (even if fugly) then that's fine IMO.

@Leonidaz
Copy link
Contributor

Leonidaz commentedDec 14, 2024
edited
Loading

What if $state.opaque is allowed to work with both a destructured assignment and also with a direct assignment, e.g.

// direct assignment that compiles into an object usage:letcount=$state.opaque(0);// changing state would be:// the prop name could  be named differently// but that's what's being used in Svelte in other places, e.g. `value`count.current++;// update,// the default name could by the variable name with pre-pended `update`, e.g. `updateCount`count.update();// or object destructure with optional renaming, this will generate the same code as it does now in this prlet{current:count,update:updateCount}=$state.opaque(0);count++;updateCount();

class for the non-destructured declaration would be: (otherwise, people can use a constructor with destructured)

// from:classThing{count=$state.opaque(0);// if count is declared as private `#count`, nothing is generated as per usual.}// to:classThing{  #_count=$state.opaque(0);getcount(){returnthis.#_count.current;}setcount(v){this.#_count.current=v;}// if `update` already exists could name it `updateCount`// a bit more complicated but `$state.opaque()` can take a second parameter `options`// e.g. $state.opaque(myState, {updateName: 'updateCount', propName: 'current'})update(){this.#_count.update();}}

Typescript will show the prop names and it would be documented.

What are the benefits vs the current approach?

When using a non-destructured variant, it will allow people to keep the update method together with the state.

As a side-effect, if kept non-destructured, this would keep the reactivity intact for crossing function boundaries.See$state.unwrap() below, if svelte can make this a pattern.

It's possible to generate a class definition.

What are the cons vs the current array destructure approach

If destructuring and renaming variables, it's a bit more verbose.

Later, maybe also introduce a rune$state.unwrap() that could take the object return of $state.opaque(0) and destructure it. Also, could work with other objects created by other state runes. The generated code would still refer to the original state though to have one synchronized state.

// somewhereletcount=$state.opaque(0);// somewhere elselet{current:count,update:updateCount}=$state.unwrap(count);// but in reality the generated code would still be referring to and updating `count.current`// so only one state exist and there is no de-synchronization.

other states:

// somewhereletsomething=$state({count:0})// somewhere elselet{ count}=$state.unwrap(something);// the generated code would still be referring to and updating `something.count`

@trueadm
Copy link
ContributorAuthor

What if $state.opaque is allowed to work with both a destructured assignment and also with a direct assignment, e.g.

That's far too many new APIs for something we don't want to promote as a core pattern.

@olivierchatry
Copy link

It's great, even if I still think a simple$state.signal(theState) would solve the issue with a lot less code. Will it works with passed props as well ?

@ThePaSch
Copy link

ThePaSch commentedDec 19, 2024
edited
Loading

It seems a little odd to me to provide this as a rune when there's a similar pattern that doesn't seem all that difficultto implement manually at the moment. I may be missing something, but what's the benefit of using$state.opaque over this sort of helper class?

The real kicker, to me, would be an implementation of$state.from that has been proposed (#12956). It seems to me like a much more flexible way to provide an escape hatch for interfacing with external libraries. In conjunction with the above code for the helper class (or theopaque rune if it's a better solution), by providing a setter that calls the invalidate function, you couldalmost treat external state like native state while still requiring you to be explicit about said external state, which would be vastly superior DX to having to manually callinvalidateFoo every single time you updatefoo.

@olivierchatry
Copy link

olivierchatry commentedDec 19, 2024 via email
edited
Loading

I think you might be missing the core issue here. This approach wouldintertwine your own data handling intricately with Svelte, similar to howyou'd have to encapsulate everything in Ember Data. This could becomeproblematic for users who manage their data separately from Svelte. Thebeauty of pre-version 5 Svelte was its neutrality—it didn't impose any datamanagement patterns on you. I'm increasingly leaning towards not upgrading,as I see no tangible benefits in doing so. It feels like Svelte is driftingtowards other frameworks, prioritizing ideology over practicality which might bevery good for other people, but very bad for somes.
On Thu, Dec 19, 2024, 08:54 Pascal Schuster ***@***.***> wrote: It seems a little odd to me to provide this as a rune when there's a similar pattern that doesn't seem all that difficult to implement manually at the moment <#10560 (comment)>. I may be missing something, but what's the benefit of using $state.opaque over this sort of helper class? The real kicker, to me, would be an implementation of $state.from that has been proposed (#12956 <#12956>). It seems to me like a much more flexible way to provide an escape hatch for interfacing with external libraries. In conjunction with the above code for the helper class (or the opaque rune if it's a better solution), by providing a setter that calls the invalidate function, you could *almost* treat external state like native state, which would be vastly superior DX to having to manually call invalidateFoo every single time you update foo. — Reply to this email directly, view it on GitHub <#14639 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJVOG6FDSFXPIVIYKDYKRD2GJ3RRAVCNFSM6AAAAABTJ3QHQ2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKNJTGAYDCMJXGA> . You are receiving this because you commented.Message ID: ***@***.***>
taylorhadden reacted with rocket emoji

@ThePaSch
Copy link

Leaving aside that that doesn't really have anything to do with this PR, I fail to understand how this sort of approach would link your data handling inextricably with Svelte? The entire point of theopaque rune - as well as my proposition for an alternative approach - is to enable you tonot have to resort to inextricably tying all of your library code to Svelte by placing runes all over it.

Yes, it was much simpler in Svelte 4, and yes, I agreesomething should be done to allow for at least the emulation of said simplicity. But that's what this PR is ultimately about, is it not?

@olivierchatry
Copy link

olivierchatry commentedDec 19, 2024
edited
Loading

It kind of does, I will let you read the task this PR closes :#14520

@Ocean-OS
Copy link
Contributor

I have found a minor issue with this: With regular$state and$state.raw declarations, if the variable being used for the$state isn't ever used in a reactive way, it is turned into a regular declaration, removing the$state call. However, with$state.opaque,this isn't the case.

@trueadm
Copy link
ContributorAuthor

trueadm commentedDec 27, 2024
edited
Loading

I have found a minor issue with this: With regular$state and$state.raw declarations, if the variable being used for the$state isn't ever used in a reactive way, it is turned into a regular declaration, removing the$state call. However, with$state.opaque,this isn't the case.

That’s because re-assignment isn’t a way to update state with thus rune. The only way is using the invalidate function.

Ocean-OS reacted with thumbs up emoji

@olivierchatry
Copy link

Is this getting merged ?

regexident and codercms reacted with thumbs up emoji

@regexident
Copy link

@trueadm would you mind giving a brief context as to why this is getting closed?

We were very much waiting for a feature like this to unblock us with respect to integrating third-party APIs that are out of our control.

whzard, eclipher, and sean-zlai reacted with thumbs up emoji

@trueadm
Copy link
ContributorAuthor

@regexident Whilst a solution is needed for this, the team agreed a while ago that this API didn't feel as ergonomic as we'd like.

ThePaSch, regexident, techniq, and Ocean-OS reacted with thumbs up emojiolivierchatry reacted with heart emoji

sean-zlai added a commit to zipline-ai/chronon that referenced this pull requestMar 5, 2025
…469)## SummaryBy extracting `series` from the markup (which made it reactive) andintentionally NOT using `$derived`, rendering performance of`PercentileLineChart` improved by over 20x (`>1100ms` down to `<50ms`).This makes navigating to the Summary tab MUCH better (before we wouldsometimes hit the browser "Are you sure you want to wait" prompts).`$state.opaque()` runes was being[considered](sveltejs/svelte#14639) that couldhave helped here, but it was decided to not be the right abstraction.(I'm waiting on a solution to fix LayerChart's force simulations thathave a perf regression in Svelte 5 due to granular reactivity / proxycreation)## Beforehttps://github.com/user-attachments/assets/9e93f4eb-5247-4bff-a56d-413564e50022## Afterhttps://github.com/user-attachments/assets/8dbe075d-0479-4d5e-a3f8-45fb1d350402## Checklist- [ ] Added Unit Tests- [ ] Covered by existing CI- [ ] Integration tested- [ ] Documentation update<!-- This is an auto-generated comment: release notes by coderabbit.ai-->## Summary by CodeRabbit- **Refactor**- Optimized the chart component's data handling to improve performanceand overall clarity.- Adjusted the internal structure by moving data definitions outsideinline usage and refining type specifications for better consistency.<!-- end of auto-generated comment: release notes by coderabbit.ai --><!-- av pr metadataThis information is embedded by the av CLI when creating PRs to trackthe status of stacks when using Aviator. Please do not delete or editthis section of the PR.```{"parent":"main","parentHead":"","trunk":"main"}```-->---------Co-authored-by: Sean Lynch <techniq35@gmail.com>
@techniq
Copy link

One use case for managing external state is integrating withd3-force. LayerChart has aForceSimulation component and it performed very well insvelte@4 and even early versions ofsvelte@5 such as5.0.2 but has since regressed in later versions (ex.5.13.0 and latest).

I haven't attempted to identify the root cause / versions yet as I feel like the root cause is the deep reactivity via proxy generation and thus a full fix will require a new construct similar to$state.opaque.

I've captured some of my investigation in anissue with PR preview examples to witness the performance.

Note: we're in the process of migrating the library to Svelte 5 state so these components are still using Svelte 3/4$: andlet assignments so this could also be attributed to legacy backwards compatibility. We're making a strong push now on the migration and hope to have most completed by EoM if things go smoothly.

knd775 and regexident reacted with thumbs up emoji

@dummdidumm
Copy link
Member

Legacy mode does not use proxies.$state does, but it sounds like you don't want its capabilities so you could use$state.raw instead. That should be the most performant for these kinds of scenarios.

techniq and regexident reacted with thumbs up emoji

@Ocean-OSOcean-OS mentioned this pull requestApr 3, 2025
6 tasks
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees

@trueadmtrueadm

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

add a way to force update a state variable
9 participants
@trueadm@Rich-Harris@Ocean-OS@dummdidumm@Leonidaz@olivierchatry@ThePaSch@regexident@techniq

[8]ページ先頭

©2009-2025 Movatter.jp