fix: stabilize capture scheduling and React 19 ref lifecycle - #679
fix: stabilize capture scheduling and React 19 ref lifecycle#679OskarEichler wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
🔵 Needs a closer look
The changes touch nuanced lifecycle and scheduling behavior (StrictMode replay, React 19 ref cleanup, continuous RAF orchestration) that warrants final human validation in real render/device scenarios.
Pull request overview
This PR updates ViewShot’s internal scheduling to behave more predictably under React StrictMode and React 19’s ref-callback lifecycle, while reducing capture-loop churn and avoiding some redundant native cleanup work.
Changes:
- Refactors first-layout gating to keep the “first layout” promise and its resolver paired through StrictMode effect replay.
- Reworks
mount,update, andcontinuouscapture scheduling to avoid duplicate mount captures, distinguish real updates from effect replay, and only schedule the next continuous frame after a capture settles. - Prevents releasing the previous capture when the newly produced URI is the same as the last captured URI, and forwards React 19 callback-ref cleanup when provided.
File summaries
| File | Description |
|---|---|
| src/index.tsx | Adjusts first-layout gating, ref forwarding/cleanup behavior, and capture scheduling for mount/update/continuous modes. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
I understand the idea to wait the last frame before starting capturing the next frame, it could help on backpressure (sending lot of request and if the phone don't have time to process) but i also see it could delay a bit more the captures and could possibly lower the FPS of them. I think it's still acceptable because we probably want to favorize better CPU perf and backpressure also don't help on FPS. we don't have benchmark today in place, but we may want to have some to study a bit this. also, in future we'll consider way better implementation for such "continous" usecase, implementing on JS side is definitely not the best idea & we recently brainstormed with @wcandillon on solutions for this in future. |
|
Yes we have ideas there, my apologies for not getting to it yet, but this is definitely on my list. |
Fixes
Keep the first-layout promise paired with its resolver during StrictMode replay, avoid duplicate mount captures, distinguish actual updates from effect replay, schedule continuous frames only after capture settles, handle automatic rejections, preserve React 19 callback-ref cleanup, and avoid releasing a repeated current URI.
Compatibility / observable changes
Imperative capture still rejects on native failure. Automatic failures invoke onCaptureFailure without an unhandled rejection. Continuous capture retries on a later frame after failure and does not spin animation frames while a capture is pending. Legacy callback refs still receive null. No new API or dependency requirements.
Verification
Fourteen actual React-rendered checks include StrictMode, callback cleanup, repeated URI, deferred native work, mode changes, unmount and failure controls.
This patch was applied independently to upstream commit
6acbec50a5e3cab7d668711d757c52cfdc791c76and passed its targeted external actual-source diagnostics. Across the focused patches, 119 such checks pass. Java/Objective-C diagnostics use controlled bridge/platform doubles or owned filesystem fixtures; they are not substitutes for physical-device rendering tests.Combined branch checks:
The separate iOS cleanup patch intentionally makes the existing test named
testReleaseCapture_currentlyDeletesPrefixOnlyImposterDirectories_KNOWN_LOOSE_GUARDfail because it asserts the unsafe old behavior. That patch is kept in a separate draft PR. No checked-in test/spec or snapshot files were added, modified, regenerated or disabled.Windows/Expo native builds, physical-device video/PixelCopy output, and full Detox suites were not run. The full unchanged Playwright suite was run against both baseline and patched builds: both have 9 passes and the same 3 failures. Two Linux-reference screenshot mismatches have byte-identical baseline/patched actual images on macOS. The CORS fixture hard-codes port 3000, occupied by an unrelated local backend; the isolated example uses another port. No snapshots or assertions were changed. React Doctor also reports existing example suggestions and a React-18 ref-cleanup warning; the latter was checked against actual React 18 legacy-ref and React 19 cleanup behavior. No rules were suppressed.
Scope
src/index.tsxUnrelated audit changes are submitted separately.