-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
chore: use proxies instead of signals for spread/rest props #9801
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
As a result of this PR, the only place we call |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes make sense to me 👍 The one caveat is that this iterates over the spread array on each invocation, compared to on each change to the spread before. But since that spread array (the $.spread_array(() => { dynamic: .. }, { static: .. }, ..)
) is only one or two elements in the common case it doesn't matter in practise.
* use proxy instead of signal in createRoot * DRY * remove for now * lint * chore: use proxies instead of signals for spread/rest props (#9801) * use proxies instead of signals for spread/rest * fix some spread attribute stuff * remove is_signal calls * simplify some more * more * remove some unnecessary unwrapping * another * simplify * simplify * simplify * remove another MaybeSignal * more * remove more unwraps * code-golf, docs --------- Co-authored-by: Rich Harris <[email protected]> Co-authored-by: Simon Holthausen <[email protected]> * add missing jsdoc annotation --------- Co-authored-by: Rich Harris <[email protected]> Co-authored-by: Simon Holthausen <[email protected]>
Continuation of #9799.
As well as simplifying things internally (yes, it's more code, though I expect it to allow us to delete code in future, hopefully including
is_signal
which is brittle and ideally wouldn't be necessary), this makes spread/rest props fine-grained in a way that wasn't previously possible.As an illustration, consider this case:
The text effect that renders
stuff.message
is re-run on everymousemove
event even though it's clearly static. On this branch, that doesn't happen.Before submitting the PR, please make sure you do the following
feat:
,fix:
,chore:
, ordocs:
.Tests and linting
pnpm test
and lint the project withpnpm lint