-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
feat: add onFrame
lifecycle function
#14594
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 0060198 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 |
preview: https://svelte-dev-git-preview-svelte-14594-svelte.vercel.app/ this is an automated message |
|
1 similar comment
|
While it's nice, this does feel more of a thing for a utility or animation library. I wouldn't call it a lifecycle function at least and expose it through |
Adjacent to #12441 |
I'm not sure it is at all. You definitely don't want to be updating the UI with |
* @param {() => NotFunction<T> | Promise<NotFunction<T>> | (() => any)} fn | ||
* @returns {void} | ||
*/ | ||
export function onFrame(fn) { |
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.
I've been thinking more about this and I think its the name I don't like. Maybe we could call it onAnimationFrame
as this is really where you're meant to handle scheduling of animation related things. I worry about onFrame
as it feels very close to onMount
in that people will just want something to happen on the next frame, but in reality it continuously happens.
Just an idle thought I had while washing the dishes: should we have an
onFrame
function? It would mean for example that this demo could be written like this:Alternatively we might want to have some sort of reactive time primitive (other than
SvelteDate
) so that it could be used in deriveds too. Either way not wedded to it, just an idle thought that I thought was worth a PRBefore submitting the PR, please make sure you do the following
feat:
,fix:
,chore:
, ordocs:
.packages/svelte/src
, add a changeset (npx changeset
).Tests and linting
pnpm test
and lint the project withpnpm lint