Skip to content

fix: stabilize capture scheduling and React 19 ref lifecycle - #679

Open
OskarEichler wants to merge 1 commit into
gre:masterfrom
OskarEichler:codex/viewshot-react-lifecycle
Open

fix: stabilize capture scheduling and React 19 ref lifecycle#679
OskarEichler wants to merge 1 commit into
gre:masterfrom
OskarEichler:codex/viewshot-react-lifecycle

Conversation

@OskarEichler

@OskarEichler OskarEichler commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

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 6acbec50a5e3cab7d668711d757c52cfdc791c76 and 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:

  • Existing JS suite: 4 suites, 46 tests pass.
  • TypeScript, ESLint (zero warnings), full Prettier check and library build pass.
  • Existing Android unit suite: 62 tests pass; Android Debug example build passes.
  • Existing Windows managed helper suite: 16 tests pass on .NET 8.
  • Unsigned iOS Simulator example build passes. Native tests: 13/14 pass; the one intentional old-behavior assertion conflict is described below.
  • Android and iOS production Metro bundles pass. Web production build passes with three bundle-size warnings.
  • React 18.3.1: all 21 applicable JS diagnostic controls pass; React 19 includes callback-ref cleanup coverage.

The separate iOS cleanup patch intentionally makes the existing test named testReleaseCapture_currentlyDeletesPrefixOnlyImposterDirectories_KNOWN_LOOSE_GUARD fail 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.tsx

Unrelated audit changes are submitted separately.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 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, and continuous capture 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.

@gre

gre commented Sep 5, 2026

Copy link
Copy Markdown
Owner

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.

@wcandillon

Copy link
Copy Markdown

Yes we have ideas there, my apologies for not getting to it yet, but this is definitely on my list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants