refactor(Worklets): extract SynchronizableDynamic from Synchronizable - #10293
refactor(Worklets): extract SynchronizableDynamic from Synchronizable#10293tjzel wants to merge 3 commits into
Conversation
|
Warning Review limit reached
Next review available in: 33 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
💤 Files with no reviewable changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughChangesThe PR separates the abstract Synchronizable refactor
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This localized class refactor introduces no evidenced user-facing regression or actionable merge-blocking risk, so it is merge-ready after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant JSIWorkletsModuleProxy
participant Synchronizable
participant SynchronizableDynamic
JSIWorkletsModuleProxy->>SynchronizableDynamic: Create synchronizable value
JSIWorkletsModuleProxy->>Synchronizable: Extract synchronizable object
Synchronizable->>SynchronizableDynamic: Dispatch read, write, lock, or unlock
SynchronizableDynamic-->>JSIWorkletsModuleProxy: Return or update value
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
…l/worklets/synchronizable-dynamic
Note
This PR description is AI-generated.
Summary
Depends on #10292.
I made
Synchronizablea pure virtual class and moved itsshared_ptr<Serializable>storage into a newSynchronizableDynamicsubclass. The base class keeps the shared unpacking logic intoJSValueand became header-only. The accessors keep their previousSerializablesignatures and dispatch virtually - the proxy still converts values at the JSI boundary. There are no behavior or API changes - this prepares the class hierarchy for a fixed-type Synchronizable implementation.I also dropped the commented out
setDirtyand its TODO. The TODO saidsetDirtymakes little sense while the member isn't atomic, and #10287 made it atomic.Test plan
Existing
memory/synchronizable.testruntime tests cover the refactor.