-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
chore: perf tweaks for actions/styles/classes #12654
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
🦋 Changeset detectedLatest commit: 0348817 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
- check if we really need to add/remove the class (calling `includes` first is cheaper than always setting/removing it) - check if we really need to update a style (calling `getPropertyValue/setProperty` is expensive) - check if we should call the action's update function (this is not only a perf tweak but also a correctness fix) closes #12652
Wow, that was quick! I'll check first thing tomorrow morning! |
@dummdidumm okay I couldn't wait. Am I mistaken in the assumption that this should not happen? REPL with |
How does your action update function look like? Does it take an object or a primitive? Can you give an example REPL using an action where things update which you don't expect to / which don't update in Svelte 4? In general, the update behavior is a bit different in Svelte 5. Since you're using |
I am calling the action with an object containing a mixture of props and some other stuff. |
includes
first is cheaper than always setting/removing it)getPropertyValue/setProperty
is expensive)closes #12652
@peterkogo could you check if these tweaks increase perf for you? From a quick local test it did for me
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