-
Notifications
You must be signed in to change notification settings - Fork 120
feat: add animate in MTS #1534
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 animate in MTS #1534
Conversation
🦋 Changeset detectedLatest commit: b62236a 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 |
📝 WalkthroughWalkthroughAdds an imperative animation API: KeyframeEffect and Animation classes, Element.animate implementation that delegates to a global __ElementAnimate, type declarations for AnimationOperation and __ElementAnimate, a changelog entry, and tests covering START/PLAY/PAUSE/CANCEL flows. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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.
Actionable comments posted: 2
🧹 Nitpick comments (10)
packages/react/worklet-runtime/src/types/elementApi.d.ts (3)
79-83
: Clarify doc: reference ElementNode, not FiberElementThe param doc references FiberElement, which isn’t declared here and is inconsistent with the rest of this file’s ElementNode usage.
- * @param element - The DOM element to animate (FiberElement reference) + * @param element - The element wrapper to animate (ElementNode reference)
53-58
: Avoid enum duplication drift with runtime AnimationOperationYou’ve re-declared AnimationOperation here while also defining it in src/api/animation/animation.ts. This can drift silently.
Options:
- Import the type in this d.ts: import type { AnimationOperation } from '../api/animation/animation.js' and reuse it (wrap ambient declarations in declare global if needed).
- Or, document clearly that these numeric values must match the runtime enum, and add tests asserting the numeric mapping.
If the ambient import approach complicates bundling, at least add a comment linking to the runtime enum and stating the values must stay in sync.
63-71
: Consider narrowing option types to align with CSS animation specTightening types improves DX and catches typos:
- iterationCount: number | 'infinite'
- fillMode: 'none' | 'forwards' | 'backwards' | 'both'
- direction: 'normal' | 'reverse' | 'alternate' | 'alternate-reverse'
Example:
interface AnimationTimingOptions { name?: string; // Animation name (optional, auto-generated if not provided) duration?: number | string; // Animation duration delay?: number | string; // Animation delay - iterationCount?: number | string; // Number of iterations (can be 'infinite') - fillMode?: string; // Animation fill mode + iterationCount?: number | 'infinite'; // Number of iterations + fillMode?: 'none' | 'forwards' | 'backwards' | 'both'; // Animation fill mode timingFunction?: string; // Animation timing function - direction?: string; // Animation direction + direction?: 'normal' | 'reverse' | 'alternate' | 'alternate-reverse'; // Animation direction }.changeset/weak-animals-search.md (2)
5-5
: Wording and spacing nitAdd a space before the acronym and tighten wording.
-Add `animate` API in Main Thread Script(MTS), so you can now control a CSS animation imperatively +Add `animate` API in Main Thread Script (MTS), so you can control CSS animations imperatively
7-19
: Specify a language for the fenced code blockThis satisfies markdownlint (MD040) and improves syntax highlighting.
-``` +```js function startAnimation() { 'main thread' const animation = ele.animate([ { opacity: 0 }, { opacity: 1 }, ], { duration: 3000 }) animation.pause() }</blockquote></details> <details> <summary>packages/react/worklet-runtime/__test__/api/element.test.js (5)</summary><blockquote> `148-157`: **Strengthen assertion: ensure the START call is the first call** Using toHaveBeenNthCalledWith makes ordering explicit and avoids false positives if more calls are added later. ```diff - expect(globalThis.__ElementAnimate).toHaveBeenCalledWith('element-instance', [ + expect(globalThis.__ElementAnimate).toHaveBeenNthCalledWith(1, 'element-instance', [ AnimationOperation.START, animation.id, [{ opacity: 0 }, { opacity: 1 }], { duration: 1000 }, ]);
159-167
: Prefer last-call assertion for CANCELMake it explicit we’re asserting the final call in this test.
- expect(globalThis.__ElementAnimate).toHaveBeenCalledWith('element-instance', [ + expect(globalThis.__ElementAnimate).toHaveBeenLastCalledWith('element-instance', [ AnimationOperation.CANCEL, animation.id, ]);
169-177
: Prefer last-call assertion for PAUSE- expect(globalThis.__ElementAnimate).toHaveBeenCalledWith('element-instance', [ + expect(globalThis.__ElementAnimate).toHaveBeenLastCalledWith('element-instance', [ AnimationOperation.PAUSE, animation.id, ]);
179-193
: Assert call order for START then PLAYThis clarifies the expected sequence.
- expect(globalThis.__ElementAnimate).toHaveBeenCalledWith('element-instance', [ + expect(globalThis.__ElementAnimate).toHaveBeenNthCalledWith(1, 'element-instance', [ AnimationOperation.START, animation.id, [{ opacity: 0 }, { opacity: 1 }], { duration: 1000 }, ]); animation.play(); - expect(globalThis.__ElementAnimate).toHaveBeenCalledWith('element-instance', [ + expect(globalThis.__ElementAnimate).toHaveBeenNthCalledWith(2, 'element-instance', [ AnimationOperation.PLAY, animation.id, ]);
179-193
: Add tests for numeric and omitted options to cover the new signatureIf you adopt the optional/number options refactor, add tests to prevent regressions.
// Add before the closing `});` it('should accept numeric duration as options', () => { const element = new Element('element-instance'); const animation = element.animate([{ opacity: 0 }, { opacity: 1 }], 500); expect(globalThis.__ElementAnimate).toHaveBeenNthCalledWith(1, 'element-instance', [ AnimationOperation.START, animation.id, [{ opacity: 0 }, { opacity: 1 }], { duration: 500 }, ]); }); it('should accept missing options (defaults to {})', () => { const element = new Element('element-instance'); const animation = element.animate([{ opacity: 0 }, { opacity: 1 }]); expect(globalThis.__ElementAnimate).toHaveBeenNthCalledWith(1, 'element-instance', [ AnimationOperation.START, animation.id, [{ opacity: 0 }, { opacity: 1 }], {}, ]); });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (6)
.changeset/weak-animals-search.md
(1 hunks)packages/react/worklet-runtime/__test__/api/element.test.js
(3 hunks)packages/react/worklet-runtime/src/api/animation/animation.ts
(1 hunks)packages/react/worklet-runtime/src/api/animation/effect.ts
(1 hunks)packages/react/worklet-runtime/src/api/element.ts
(2 hunks)packages/react/worklet-runtime/src/types/elementApi.d.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/react/worklet-runtime/src/api/animation/effect.ts
- packages/react/worklet-runtime/src/api/animation/animation.ts
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/react/worklet-runtime/__test__/api/element.test.js (1)
packages/react/worklet-runtime/src/api/element.ts (1)
Element
(7-96)
packages/react/worklet-runtime/src/api/element.ts (2)
packages/react/worklet-runtime/src/api/animation/animation.ts (1)
Animation
(13-48)packages/react/worklet-runtime/src/api/animation/effect.ts (1)
KeyframeEffect
(6-20)
🪛 LanguageTool
.changeset/weak-animals-search.md
[grammar] ~5-~5: There might be a mistake here.
Context: ...js/react': patch --- Add animate
API in Main Thread Script(MTS), so you can now...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
.changeset/weak-animals-search.md
7-7: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build / Build (Windows)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: test-rust / Test (Ubuntu)
- GitHub Check: CodeQL Analyze (actions)
- GitHub Check: CodeQL Analyze (javascript-typescript)
CodSpeed Performance ReportMerging #1534 will not alter performanceComparing Summary
|
React Example#4322 Bundle Size — 237.04KiB (0%).b62236a(current) vs 837e6fa main#4320(baseline) Bundle metrics
|
Current #4322 |
Baseline #4320 |
|
---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
4 |
4 |
|
158 |
158 |
|
64 |
64 |
|
45.86% |
45.86% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
Current #4322 |
Baseline #4320 |
|
---|---|---|
145.76KiB |
145.76KiB |
|
91.28KiB |
91.28KiB |
Bundle analysis report Branch f0rdream:mt_animate Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#4315 Bundle Size — 366.38KiB (0%).b62236a(current) vs 837e6fa main#4313(baseline) Bundle metrics
Bundle size by type
|
Current #4315 |
Baseline #4313 |
|
---|---|---|
234.38KiB |
234.38KiB |
|
100.16KiB |
100.16KiB |
|
31.84KiB |
31.84KiB |
Bundle analysis report Branch f0rdream:mt_animate Project dashboard
Generated by RelativeCI Documentation Report issue
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.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/react/worklet-runtime/src/types/elementApi.d.ts (1)
84-95
: Rename “name” to “id” and update comments; allow PAUSE/PLAY/CANCEL branch docs to match.This aligns the typings with actual usage where the animation’s runtime id is passed, and updates docs to reflect CANCEL support. Mirrors prior feedback.
declare function __ElementAnimate( element: ElementNode, args: [ operation: AnimationOperation, // Animation operation type - name: string, // Animation name + id: string, // Runtime animation id keyframes: Keyframe[], // Array of keyframes options?: AnimationTimingOptions, // Timing and configuration options ] | [ - operation: AnimationOperation.PAUSE | AnimationOperation.PLAY | AnimationOperation.CANCEL, - name: string, // Animation name to pause/play + operation: AnimationOperation.PAUSE | AnimationOperation.PLAY | AnimationOperation.CANCEL, // PAUSE, PLAY or CANCEL + id: string, // Runtime animation id to pause/play/cancel ], ): void;
🧹 Nitpick comments (4)
.changeset/weak-animals-search.md (2)
5-5
: Tighten wording and fix spacing around “MTS”.Minor grammar/style improvement and add a space before the parenthetical.
-Add `animate` API in Main Thread Script(MTS), so you can now control a CSS animation imperatively +Add an `animate` API for Main Thread Script (MTS), enabling imperative control of CSS animations.
7-19
: Add code fence language for the example (markdownlint MD040).Specify the language to improve readability and satisfy linting.
-``` +```js function startAnimation() { 'main thread' const animation = ele.animate([ { opacity: 0 }, { opacity: 1 }, ], { duration: 3000 }) animation.pause() }</blockquote></details> <details> <summary>packages/react/worklet-runtime/src/api/animation/effect.ts (1)</summary><blockquote> `6-20`: **Align types with the declared Animation types and prefer readonly views.** Use the shared Keyframe and AnimationTimingOptions types from the ambient declarations, and prefer readonly to avoid accidental mutation. ```diff export class KeyframeEffect { public readonly target: Element; - public readonly keyframes: Record<string, number | string>[]; - public readonly options: Record<string, number | string>; + public readonly keyframes: ReadonlyArray<Keyframe>; + public readonly options: Readonly<AnimationTimingOptions>; constructor( target: Element, - keyframes: Record<string, number | string>[], - options: Record<string, number | string>, + keyframes: ReadonlyArray<Keyframe>, + options: Readonly<AnimationTimingOptions>, ) { this.target = target; this.keyframes = keyframes; this.options = options; } }
If mutation from callers is a concern, consider shallow-freezing in the constructor:
this.keyframes = Object.freeze([...keyframes]); this.options = Object.freeze({ ...(options || {}) });packages/react/worklet-runtime/src/types/elementApi.d.ts (1)
79-83
: Fix incorrect parameter doc for element type.Comment references FiberElement, but the signature uses ElementNode.
- * @param element - The DOM element to animate (FiberElement reference) + * @param element - The element to animate (ElementNode reference)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
.changeset/weak-animals-search.md
(1 hunks)packages/react/worklet-runtime/__test__/api/element.test.js
(3 hunks)packages/react/worklet-runtime/src/api/animation/animation.ts
(1 hunks)packages/react/worklet-runtime/src/api/animation/effect.ts
(1 hunks)packages/react/worklet-runtime/src/api/element.ts
(2 hunks)packages/react/worklet-runtime/src/types/elementApi.d.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/react/worklet-runtime/test/api/element.test.js
- packages/react/worklet-runtime/src/api/animation/animation.ts
- packages/react/worklet-runtime/src/api/element.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/react/worklet-runtime/src/api/animation/effect.ts (1)
packages/react/worklet-runtime/src/api/element.ts (1)
Element
(7-99)
🪛 LanguageTool
.changeset/weak-animals-search.md
[grammar] ~5-~5: There might be a mistake here.
Context: ...js/react': patch --- Add animate
API in Main Thread Script(MTS), so you can now...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
.changeset/weak-animals-search.md
7-7: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (1)
packages/react/worklet-runtime/src/api/animation/effect.ts (1)
6-20
: LGTM overall; the class is a clean, focused data holder.Simple, readable, and fits the intended role as a transport between Element.animate and the animation driver.
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.
Actionable comments posted: 0
♻️ Duplicate comments (3)
packages/react/worklet-runtime/src/types/elementApi.d.ts (3)
76-77
: Rename ambient Keyframe to avoid collision with lib.dom’s KeyframeThis ambient Keyframe can collide with the DOM lib’s Keyframe in consumer projects. Rename/scoped type to prevent global conflicts and update the __ElementAnimate signature accordingly.
-type Keyframe = Record<string, string | number>; +type WorkletKeyframe = Record<string, string | number>; @@ declare function __ElementAnimate( element: ElementNode, args: [ operation: AnimationOperation, // Animation operation type - name: string, // Animation name - keyframes: Keyframe[], // Array of keyframes + id: string, // Runtime animation id + keyframes: WorkletKeyframe[], // Array of keyframes options?: AnimationTimingOptions, // Timing and configuration options ] | [ operation: AnimationOperation.PAUSE | AnimationOperation.PLAY | AnimationOperation.CANCEL, - name: string, // Animation name to pause/play + id: string, // Runtime animation id to pause/play/cancel ], ): void;Also applies to: 84-95
88-94
: Use id (runtime animation id) instead of name for clarity and parity with call sitesRuntime uses Animation.id. Rename tuple param and comments to id, and note CANCEL is also valid.
- name: string, // Animation name + id: string, // Runtime animation id @@ - operation: AnimationOperation.PAUSE | AnimationOperation.PLAY | AnimationOperation.CANCEL, - name: string, // Animation name to pause/play + operation: AnimationOperation.PAUSE | AnimationOperation.PLAY | AnimationOperation.CANCEL, + id: string, // Runtime animation id to pause/play/cancel
53-58
: Prefer a const enum to avoid runtime references — safe to inlineConfirmed: the repo build uses tsc (root package.json: "build": "tsc --build") and I found no "preserveConstEnums": true in any tsconfig, so a const enum will be erased/inlined by the TypeScript build.
Files to update / review:
- packages/react/worklet-runtime/src/types/elementApi.d.ts — change ambient enum to const enum.
- packages/react/worklet-runtime/src/api/animation/animation.ts — currently declares
export enum AnimationOperation
(non-const) which emits a runtime object; consider making itexport const enum
or unify by importing the const enum from packages/web-platform/web-constants.- packages/web-platform/web-constants/src/types/Element.ts — already
export const enum AnimationOperation
(matches the const-enum approach).Suggested diff (elementApi.d.ts):
-declare enum AnimationOperation { +declare const enum AnimationOperation { START = 0, // Start a new animation PLAY = 1, // Play/resume a paused animation PAUSE = 2, // Pause an existing animation CANCEL = 3, // Cancel an animation }
🧹 Nitpick comments (5)
packages/react/worklet-runtime/src/api/element.ts (2)
60-66
: Strengthen inferred type of normalizedOptions to avoid wideningMinor enhancement: explicitly type normalizedOptions to the expected shape for clarity and to prevent future widening if the logic changes.
- const normalizedOptions = typeof options === 'number' ? { duration: options } : options ?? {}; + const normalizedOptions: Record<string, number | string> = + typeof options === 'number' ? { duration: options } : options ?? {};
60-66
: Consider aligning option types with AnimationTimingOptions across the APIFor stronger typings and parity with the __ElementAnimate surface, consider migrating option types from Record<string, number | string> to AnimationTimingOptions throughout the animation stack (Element.animate and KeyframeEffect).
Follow-up (outside this hunk):
- In KeyframeEffect (packages/react/worklet-runtime/src/api/animation/effect.ts), change the options type to AnimationTimingOptions for target, property, and constructor param.
If you want, I can provide a PR-ready patch updating effect.ts and usages.
packages/react/worklet-runtime/src/types/elementApi.d.ts (1)
80-80
: Fix parameter doc to match the actual type (ElementNode)The comment still mentions FiberElement; the parameter is ElementNode.
- * @param element - The DOM element to animate (FiberElement reference) + * @param element - The DOM element to animate (ElementNode reference).changeset/weak-animals-search.md (2)
5-5
: Tighten wording and spacingSmall grammar/spacing fix improves readability.
-Add `animate` API in Main Thread Script(MTS), so you can now control a CSS animation imperatively +Add the `animate` API in Main Thread Script (MTS), allowing you to control CSS animations imperatively.
7-19
: Specify a language for the fenced code blockAdd a language hint to satisfy markdownlint and improve syntax highlighting.
-``` +```js function startAnimation() { 'main thread' const animation = ele.animate([ { opacity: 0 }, { opacity: 1 }, ], { duration: 3000 }) animation.pause() }</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: .coderabbit.yaml** **Review profile: CHILL** **Plan: Pro** **💡 Knowledge Base configuration:** - MCP integration is disabled by default for public repositories - Jira integration is disabled by default for public repositories - Linear integration is disabled by default for public repositories You can enable these sources in your CodeRabbit configuration. <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 20b3f28a4a6e4ad254722c057ecf5dcdd2b41237 and 6f15f61b2e66fe646c1c00ee98a815eed770306d. </details> <details> <summary>📒 Files selected for processing (6)</summary> * `.changeset/weak-animals-search.md` (1 hunks) * `packages/react/worklet-runtime/__test__/api/element.test.js` (3 hunks) * `packages/react/worklet-runtime/src/api/animation/animation.ts` (1 hunks) * `packages/react/worklet-runtime/src/api/animation/effect.ts` (1 hunks) * `packages/react/worklet-runtime/src/api/element.ts` (2 hunks) * `packages/react/worklet-runtime/src/types/elementApi.d.ts` (1 hunks) </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (3)</summary> * packages/react/worklet-runtime/src/api/animation/effect.ts * packages/react/worklet-runtime/src/api/animation/animation.ts * packages/react/worklet-runtime/__test__/api/element.test.js </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🧬 Code Graph Analysis (1)</summary> <details> <summary>packages/react/worklet-runtime/src/api/element.ts (2)</summary><blockquote> <details> <summary>packages/react/worklet-runtime/src/api/animation/animation.ts (1)</summary> * `Animation` (13-48) </details> <details> <summary>packages/react/worklet-runtime/src/api/animation/effect.ts (1)</summary> * `KeyframeEffect` (6-20) </details> </blockquote></details> </details><details> <summary>🪛 LanguageTool</summary> <details> <summary>.changeset/weak-animals-search.md</summary> [grammar] ~5-~5: There might be a mistake here. Context: ...js/react': patch --- Add `animate` API in Main Thread Script(MTS), so you can now... (QB_NEW_EN) </details> </details> <details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> <details> <summary>.changeset/weak-animals-search.md</summary> 7-7: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> </details> <details> <summary>🔇 Additional comments (3)</summary><blockquote> <details> <summary>packages/react/worklet-runtime/src/api/element.ts (2)</summary> `4-5`: **ESM import extensions look correct; LGTM** Using .js extensions in TS source is consistent with NodeNext/ESM output. No issues spotted. --- `60-66`: **Confirm whether a flush is required after starting an animation** animate() triggers __ElementAnimate via Animation.start(), but unlike style/attribute setters it doesn’t flush. If the bridge enqueues UI ops that require a flush tick, you may need to call flushElementTree. If the animation op flushes internally, then this is fine—just confirming intent. Would you validate with existing tests or a manual check that animations start reliably without an explicit flush on low-traffic frames? </details> <details> <summary>packages/react/worklet-runtime/src/types/elementApi.d.ts (1)</summary> `92-92`: **CANCEL correctly included in control-ops union; LGTM** </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
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.
Actionable comments posted: 0
♻️ Duplicate comments (3)
packages/react/worklet-runtime/src/types/elementApi.d.ts (3)
78-83
: Fix function name and parameter doc; align with ElementNode.The doc header references “ElementAnimate” (missing underscores) and mentions “DOM element (FiberElement)”. The actual type is
ElementNode
, and in worklet context it’s not a DOM Element.Apply this diff:
-/** - * ElementAnimate function - controls animations on DOM elements - * @param element - The DOM element to animate (FiberElement reference) - * @param args - Animation configuration array - * @returns undefined - */ +/** + * __ElementAnimate function - controls animations for worklet ElementNodes. + * @param element - The worklet runtime ElementNode to animate + * @param args - Animation configuration tuple (see overloads below) + * @returns void + */
84-95
: Rename tuple param from “name” to “id” and update comments (also mention CANCEL).Runtime semantics (tests) indicate the second tuple item is the animation id, not a CSS name. The comment for the narrow variant should include cancel. This is a doc/type-name clean-up; no runtime behavior changes.
Apply this diff:
declare function __ElementAnimate( element: ElementNode, args: [ operation: AnimationOperation, // Animation operation type - name: string, // Animation name + id: string, // Runtime animation id - keyframes: Keyframe[], // Array of keyframes + keyframes: WorkletKeyframe[], // Array of keyframes options?: AnimationTimingOptions, // Timing and configuration options ] | [ operation: AnimationOperation.PAUSE | AnimationOperation.PLAY | AnimationOperation.CANCEL, - name: string, // Animation name to pause/play + id: string, // Runtime animation id to pause/play/cancel ], ): void;
76-76
: Rename ambient Keyframe to avoid collision with lib.dom’s KeyframeThis .d.ts declares a global
Keyframe
type which can collide with the DOM lib. Rename it and update usages.Files to change:
- packages/react/worklet-runtime/src/types/elementApi.d.ts
- line 76:
type Keyframe = Record<string, string | number>;
- line 89:
keyframes: Keyframe[],
Apply this diff:
-type Keyframe = Record<string, string | number>; +type WorkletKeyframe = Record<string, string | number>; @@ - keyframes: Keyframe[], // Array of keyframes + keyframes: WorkletKeyframe[], // Array of keyframesI searched the repo and found no other
Keyframe
declarations/usages. After applying the change, run the TypeScript typecheck/build to ensure no other references remain.
🧹 Nitpick comments (2)
.changeset/weak-animals-search.md (2)
5-5
: Polish the sentence and spacing around the acronym.Add the missing article and space before the parenthetical to improve readability.
Apply this diff:
-Add `animate` API in Main Thread Script(MTS), so you can now control a CSS animation imperatively +Add the `element.animate` API to Main Thread Script (MTS), enabling imperative control of CSS animations
7-19
: Specify the fenced code language.markdownlint flagged MD040 (fenced-code-language). Annotate the block as JavaScript.
Apply this diff:
-``` +```js function startAnimation() { 'main thread' const animation = ele.animate([ { opacity: 0 }, { opacity: 1 }, ], { duration: 3000 }) animation.pause() } -``` +```
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
.changeset/weak-animals-search.md
(1 hunks)packages/react/worklet-runtime/__test__/api/element.test.js
(3 hunks)packages/react/worklet-runtime/src/api/animation/animation.ts
(1 hunks)packages/react/worklet-runtime/src/api/animation/effect.ts
(1 hunks)packages/react/worklet-runtime/src/api/element.ts
(2 hunks)packages/react/worklet-runtime/src/types/elementApi.d.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/react/worklet-runtime/test/api/element.test.js
- packages/react/worklet-runtime/src/api/animation/effect.ts
- packages/react/worklet-runtime/src/api/element.ts
- packages/react/worklet-runtime/src/api/animation/animation.ts
🧰 Additional context used
🪛 LanguageTool
.changeset/weak-animals-search.md
[grammar] ~5-~5: There might be a mistake here.
Context: ...js/react': patch --- Add animate
API in Main Thread Script(MTS), so you can now...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
.changeset/weak-animals-search.md
7-7: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (2)
packages/react/worklet-runtime/src/types/elementApi.d.ts (2)
53-58
: Good: ambient enum avoids runtime emission.Using
declare enum AnimationOperation
in a.d.ts
is correct and prevents unwanted JS output. Names and values look aligned with tests (START/PLAY/PAUSE/CANCEL).
63-71
: Add WAAPI-compatible aliases to AnimationTimingOptions — safe/non-breakingrg search shows the type is only defined and used in packages/react/worklet-runtime/src/types/elementApi.d.ts (definition + local usage), so adding optional WAAPI aliases is additive and non-breaking.
- Location:
- packages/react/worklet-runtime/src/types/elementApi.d.ts — interface AnimationTimingOptions
Apply this diff:
interface AnimationTimingOptions { name?: string; // Animation name (optional, auto-generated if not provided) duration?: number | string; // Animation duration delay?: number | string; // Animation delay iterationCount?: number | string; // Number of iterations (can be 'infinite') + iterations?: number | string; // Alias of iterationCount (Web Animations API) fillMode?: string; // Animation fill mode + fill?: string; // Alias of fillMode (Web Animations API) timingFunction?: string; // Animation timing function + easing?: string; // Alias of timingFunction (Web Animations API) direction?: string; // Animation direction }
Is there any document/RFC for |
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.
Actionable comments posted: 0
🧹 Nitpick comments (8)
.changeset/weak-animals-search.md (2)
5-5
: Polish phrasing and spacing for MTS acronym; consider linking to docs/RFC.Recommend tightening the sentence, adding the missing space before the acronym, and linking to docs/RFC for
__ElementAnimate
to answer the PR comment and aid discoverability.Apply this diff:
-Add `animate` API in Main Thread Script(MTS), so you can now control a CSS animation imperatively +Add the `animate` API in Main Thread Script (MTS), allowing you to control CSS animations imperatively. + +See also: [link to docs/RFC for __ElementAnimate]
7-19
: Add code-fence language and clarify theele
variable in the example.Explicit language tag fixes MD040; adding a brief comment prevents confusion about what
ele
is.-``` +```js function startAnimation() { 'main thread' + // assume `ele` is an Element obtained via querySelector or passed in const animation = ele.animate([ { opacity: 0 }, { opacity: 1 }, ], { duration: 3000 }) animation.pause() }</blockquote></details> <details> <summary>packages/react/worklet-runtime/__test__/api/element.test.js (6)</summary><blockquote> `147-157`: **Strengthen assertion: verify call order and count.** Use nth-call assertions to ensure START is dispatched exactly once upon creation. ```diff - expect(globalThis.__ElementAnimate).toHaveBeenCalledWith('element-instance', [ + expect(globalThis.__ElementAnimate).toHaveBeenNthCalledWith(1, 'element-instance', [ AnimationOperation.START, animation.id, [{ opacity: 0 }, { opacity: 1 }], { duration: 1000 }, ]); + expect(globalThis.__ElementAnimate).toHaveBeenCalledTimes(1);
159-168
: Nit: test name grammar and stronger assertion.Minor wording polish; also assert the first call and call count.
- it('should start animation when pass number as option', () => { + it('should start animation when passing a number as options', () => { const element = new Element('element-instance'); const animation = element.animate([{ opacity: 0 }, { opacity: 1 }], 1000); - expect(globalThis.__ElementAnimate).toHaveBeenCalledWith('element-instance', [ + expect(globalThis.__ElementAnimate).toHaveBeenNthCalledWith(1, 'element-instance', [ AnimationOperation.START, animation.id, [{ opacity: 0 }, { opacity: 1 }], { duration: 1000 }, ]); + expect(globalThis.__ElementAnimate).toHaveBeenCalledTimes(1); });
170-179
: Stronger assertion: verify START happens once even without options.This protects against accidental duplicate dispatch.
- expect(globalThis.__ElementAnimate).toHaveBeenCalledWith('element-instance', [ + expect(globalThis.__ElementAnimate).toHaveBeenNthCalledWith(1, 'element-instance', [ AnimationOperation.START, animation.id, [{ opacity: 0 }, { opacity: 1 }], {}, ]); + expect(globalThis.__ElementAnimate).toHaveBeenCalledTimes(1);
181-189
: Verify full sequence for cancel: START then CANCEL; assert total calls.Asserting order reduces false positives if unexpected calls occur.
const element = new Element('element-instance'); const animation = element.animate([{ opacity: 0 }, { opacity: 1 }], { duration: 1000 }); + expect(globalThis.__ElementAnimate).toHaveBeenNthCalledWith(1, 'element-instance', [ + AnimationOperation.START, + animation.id, + [{ opacity: 0 }, { opacity: 1 }], + { duration: 1000 }, + ]); animation.cancel(); - expect(globalThis.__ElementAnimate).toHaveBeenCalledWith('element-instance', [ + expect(globalThis.__ElementAnimate).toHaveBeenNthCalledWith(2, 'element-instance', [ AnimationOperation.CANCEL, animation.id, ]); + expect(globalThis.__ElementAnimate).toHaveBeenCalledTimes(2);
191-199
: Verify full sequence for pause: START then PAUSE; assert total calls.const element = new Element('element-instance'); const animation = element.animate([{ opacity: 0 }, { opacity: 1 }], { duration: 1000 }); + expect(globalThis.__ElementAnimate).toHaveBeenNthCalledWith(1, 'element-instance', [ + AnimationOperation.START, + animation.id, + [{ opacity: 0 }, { opacity: 1 }], + { duration: 1000 }, + ]); animation.pause(); - expect(globalThis.__ElementAnimate).toHaveBeenCalledWith('element-instance', [ + expect(globalThis.__ElementAnimate).toHaveBeenNthCalledWith(2, 'element-instance', [ AnimationOperation.PAUSE, animation.id, ]); + expect(globalThis.__ElementAnimate).toHaveBeenCalledTimes(2);
201-215
: Prefer ordered assertions for START/PLAY and check total invocations.This ensures no extra host calls are made.
- expect(globalThis.__ElementAnimate).toHaveBeenCalledWith('element-instance', [ + expect(globalThis.__ElementAnimate).toHaveBeenNthCalledWith(1, 'element-instance', [ AnimationOperation.START, animation.id, [{ opacity: 0 }, { opacity: 1 }], { duration: 1000 }, ]); animation.play(); - expect(globalThis.__ElementAnimate).toHaveBeenCalledWith('element-instance', [ + expect(globalThis.__ElementAnimate).toHaveBeenNthCalledWith(2, 'element-instance', [ AnimationOperation.PLAY, animation.id, ]); + expect(globalThis.__ElementAnimate).toHaveBeenCalledTimes(2);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
.changeset/weak-animals-search.md
(1 hunks)packages/react/worklet-runtime/__test__/api/element.test.js
(3 hunks)packages/react/worklet-runtime/src/api/animation/animation.ts
(1 hunks)packages/react/worklet-runtime/src/api/animation/effect.ts
(1 hunks)packages/react/worklet-runtime/src/api/element.ts
(2 hunks)packages/react/worklet-runtime/src/types/elementApi.d.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/react/worklet-runtime/src/types/elementApi.d.ts
- packages/react/worklet-runtime/src/api/animation/effect.ts
- packages/react/worklet-runtime/src/api/element.ts
- packages/react/worklet-runtime/src/api/animation/animation.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/react/worklet-runtime/__test__/api/element.test.js (1)
packages/react/worklet-runtime/src/api/element.ts (1)
Element
(7-99)
🪛 LanguageTool
.changeset/weak-animals-search.md
[grammar] ~5-~5: There might be a mistake here.
Context: ...js/react': patch --- Add animate
API in Main Thread Script(MTS), so you can now...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
.changeset/weak-animals-search.md
7-7: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (2)
packages/react/worklet-runtime/__test__/api/element.test.js (2)
6-6
: Good: importing the enum keeps tests resilient to numeric changes.
25-26
: Good: mocking the host bridge__ElementAnimate
is the right seam.
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @lynx-js/[email protected] ### Patch Changes - fix `withInitDataInState` got wrong state in 2nd or more times `defaultDataProcessor`, now it will keep its own state. ([#1478](#1478)) - change `__CreateElement('raw-text')` to `__CreateRawText('')` to avoid `setNativeProps` not working ([#1570](#1570)) - Fix wrong render result when using expression as `key`. ([#1541](#1541)) See [#1371](#1371) for more details. - fix: `Cannot read properties of undefined` error when using `Suspense` ([#1569](#1569)) - Add `animate` API in Main Thread Script(MTS), so you can now control a CSS animation imperatively ([#1534](#1534)) ```ts import type { MainThread } from "@lynx-js/types"; function startAnimation(ele: MainThread.Element) { "main thread"; const animation = ele.animate([{ opacity: 0 }, { opacity: 1 }], { duration: 3000, }); // Can also be paused // animation.pause() } ``` ## @lynx-js/[email protected] ### Patch Changes - Support caching Lynx native events when chunk splitting is enabled. ([#1370](#1370)) When `performance.chunkSplit.strategy` is not `all-in-one`, Lynx native events are cached until the BTS chunk is fully loaded and are replayed when that chunk is ready. The `firstScreenSyncTiming` flag will no longer change to `jsReady` anymore. - Support exporting `Promise` and function in `lynx.config.ts`. ([#1590](#1590)) - Fix missing `publicPath` using when `rspeedy dev --mode production`. ([#1310](#1310)) - Updated dependencies \[[`aaca8f9`](aaca8f9)]: - @lynx-js/[email protected] - @lynx-js/[email protected] ## [email protected] ### Patch Changes - Add `@lynx-js/preact-devtools` by default. ([#1593](#1593)) ## @lynx-js/[email protected] ### Patch Changes - Bump @clack/prompts to v1.0.0-alpha.4 ([#1559](#1559)) ## @lynx-js/[email protected] ### Patch Changes - Support using multiple times in different environments. ([#1498](#1498)) - Support caching Lynx native events when chunk splitting is enabled. ([#1370](#1370)) When `performance.chunkSplit.strategy` is not `all-in-one`, Lynx native events are cached until the BTS chunk is fully loaded and are replayed when that chunk is ready. The `firstScreenSyncTiming` flag will no longer change to `jsReady` anymore. - Updated dependencies \[[`f0d483c`](f0d483c), [`e4d116b`](e4d116b), [`d33c1d2`](d33c1d2)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Support using multiple times in different environments. ([#1498](#1498)) - Alias `@lynx-js/preact-devtools` to `false` to reduce an import of empty webpack module. ([#1593](#1593)) ## @lynx-js/[email protected] ### Patch Changes - Support `lynx.createSelectorQuery().select()` and `setNativeProps` API ([#1570](#1570)) ## @lynx-js/[email protected] ### Patch Changes - fix: globalThis is never accessible in MTS ([#1531](#1531)) - Updated dependencies \[]: - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - fix: fake uidisappear event ([#1539](#1539)) - Updated dependencies \[[`70863fb`](70863fb)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - fix: globalThis is never accessible in MTS ([#1531](#1531)) - Updated dependencies \[[`70863fb`](70863fb)]: - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - fix: globalThis is never accessible in MTS ([#1531](#1531)) - Updated dependencies \[[`70863fb`](70863fb)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Add new `LynxCacheEventsPlugin`, which will cache Lynx native events until the BTS chunk is fully loaded, and replay them when the BTS chunk is ready. ([#1370](#1370)) - Updated dependencies \[[`aaca8f9`](aaca8f9)]: - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Updated dependencies \[[`aaca8f9`](aaca8f9)]: - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Updated dependencies \[[`aaca8f9`](aaca8f9)]: - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Updated dependencies \[[`aaca8f9`](aaca8f9)]: - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Always inline the background script that contains rspack runtime module. ([#1582](#1582)) - Updated dependencies \[[`aaca8f9`](aaca8f9)]: - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Add `lynxCacheEventsSetupList` and `lynxCacheEvents` to RuntimeGlobals. It will be used to cache Lynx native events until the BTS chunk is fully loaded, and replay them when the BTS chunk is ready. ([#1370](#1370)) ## [email protected] ## @lynx-js/[email protected] ## @lynx-js/[email protected] ## @lynx-js/[email protected] Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This commit add
element.animate
in Main Thread Script (MTS), so we can have ablity to initite or control an animation in CSS in MTS.Summary by CodeRabbit
Checklist