mirror of
https://github.com/stablyai/orca.git
synced 2026-10-23 16:03:49 +00:00
fix(mobile): keep the create form when a render is thrown away
The session key was built from counters mutated during render. A blurred screen suspends this subtree (react-native-screens freezes via react-freeze), so React runs the component and then discards that render — but the counter bumps survive it. The next committed render then produced a new key and remounted the form, throwing away the picked source for a host switch or a close that never committed. Hold the open epoch in state, which React discards with the render that set it, and put the host in the key directly instead of counting host changes.
This commit is contained in:
@@ -1,4 +1,4 @@
|
||||
import { createElement } from 'react'
|
||||
import { createElement, Suspense, type ReactElement } from 'react'
|
||||
import { act, create, type ReactTestRenderer } from 'react-test-renderer'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { RpcClient } from '../transport/rpc-client'
|
||||
@@ -420,6 +420,84 @@ describe('NewWorktreeModal project targets', () => {
|
||||
])
|
||||
})
|
||||
|
||||
// react-native-screens freezes a blurred screen by suspending its subtree
|
||||
// (react-freeze), so React runs this component and then throws that render away.
|
||||
// Anything the render mutated in place outlives the work React discarded.
|
||||
function suspendableTree(frozen: () => boolean, child: ReactElement) {
|
||||
const never = new Promise<void>(() => {})
|
||||
const Freezer = () => {
|
||||
if (frozen()) {
|
||||
throw never
|
||||
}
|
||||
return null
|
||||
}
|
||||
// The modal renders first, so its render completes before the freeze throws.
|
||||
return createElement(Suspense, { fallback: null }, child, createElement(Freezer, null))
|
||||
}
|
||||
|
||||
it('keeps the form when a host switch renders but never commits', async () => {
|
||||
setCachedRepos('host-2', repos)
|
||||
const client = {
|
||||
sendRequest: vi.fn().mockImplementation(() => new Promise(() => {}))
|
||||
} as unknown as RpcClient
|
||||
const modalProps = { visible: true, client, onCreated: () => {}, onClose: () => {} }
|
||||
let frozen = false
|
||||
const tree = (hostId: string) =>
|
||||
suspendableTree(() => frozen, createElement(NewWorktreeModal, { ...modalProps, hostId }))
|
||||
|
||||
await act(async () => {
|
||||
renderer = create(tree('host-1'))
|
||||
})
|
||||
act(() => sourceInputs(renderer)[0]!.props.onChangeText('keep-me'))
|
||||
|
||||
frozen = true
|
||||
await act(async () => {
|
||||
renderer.update(tree('host-2'))
|
||||
})
|
||||
frozen = false
|
||||
await act(async () => {
|
||||
renderer.update(tree('host-1'))
|
||||
})
|
||||
|
||||
// host-2 never committed, so host-1's session was never superseded.
|
||||
expect(sourceInputs(renderer).map((input) => input.props.value)).toEqual(['keep-me', 'keep-me'])
|
||||
})
|
||||
|
||||
it('keeps the form when a close renders but never commits', async () => {
|
||||
const client = {
|
||||
sendRequest: vi.fn().mockImplementation(() => new Promise(() => {}))
|
||||
} as unknown as RpcClient
|
||||
const modalProps = {
|
||||
client,
|
||||
hostId: 'host-1',
|
||||
onCreated: () => {},
|
||||
onClose: () => {}
|
||||
}
|
||||
let frozen = false
|
||||
const tree = (visible: boolean) =>
|
||||
suspendableTree(() => frozen, createElement(NewWorktreeModal, { ...modalProps, visible }))
|
||||
|
||||
await act(async () => {
|
||||
renderer = create(tree(true))
|
||||
})
|
||||
act(() => sourceInputs(renderer)[0]!.props.onChangeText('keep-me-too'))
|
||||
|
||||
frozen = true
|
||||
await act(async () => {
|
||||
renderer.update(tree(false))
|
||||
})
|
||||
frozen = false
|
||||
await act(async () => {
|
||||
renderer.update(tree(true))
|
||||
})
|
||||
|
||||
// The drawer never committed a closed state, so this is not a reopening.
|
||||
expect(sourceInputs(renderer).map((input) => input.props.value)).toEqual([
|
||||
'keep-me-too',
|
||||
'keep-me-too'
|
||||
])
|
||||
})
|
||||
|
||||
it('starts a fresh form session when the modal switches hosts', async () => {
|
||||
setCachedRepos('host-2', repos)
|
||||
const client = {
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { useMemo, useRef, useState } from 'react'
|
||||
import { useMemo, useState } from 'react'
|
||||
import { Keyboard } from 'react-native'
|
||||
import { getComposerRepoWorktreeBranches } from '../../../src/shared/composer-branch-selection'
|
||||
import { getProjectIdentityKey } from '../../../src/shared/project-host-setup-projection'
|
||||
@@ -33,31 +33,25 @@ import { useNewWorkspaceSetupScript } from './use-new-workspace-setup-script'
|
||||
import { useNewWorktreeDrawerNavigation } from './use-new-worktree-drawer-navigation'
|
||||
|
||||
export function NewWorktreeModal(props: NewWorktreeModalProps) {
|
||||
const openEpochRef = useRef(0)
|
||||
const wasVisibleRef = useRef(false)
|
||||
const hostEpochRef = useRef({ hostId: props.hostId, epoch: 0 })
|
||||
|
||||
// Why: each drawer opening is a fresh form session; remounting resets local
|
||||
// form state before paint instead of clearing it in a visible-prop Effect.
|
||||
if (props.visible && !wasVisibleRef.current) {
|
||||
openEpochRef.current += 1
|
||||
// State, not a ref: react-native-screens freezes a blurred screen by suspending
|
||||
// this subtree, and a counter bumped during a render React then throws away
|
||||
// would restart the session for an opening that never committed.
|
||||
const [session, setSession] = useState({ openEpoch: 0, visible: props.visible })
|
||||
if (session.visible !== props.visible) {
|
||||
setSession({
|
||||
openEpoch: props.visible ? session.openEpoch + 1 : session.openEpoch,
|
||||
visible: props.visible
|
||||
})
|
||||
}
|
||||
wasVisibleRef.current = props.visible
|
||||
|
||||
// Why: key the session on the HOST, never on the RpcClient object. A reconnect,
|
||||
// forceReconnect, or foreground revival swaps that object for the same host
|
||||
// (see useHostClient), and keying on it silently remounted this form mid-edit
|
||||
// and threw away the picked source. Every client-scoped hook below already
|
||||
// drops responses from a superseded client, so no remount is needed for that.
|
||||
if (hostEpochRef.current.hostId !== props.hostId) {
|
||||
hostEpochRef.current = { hostId: props.hostId, epoch: hostEpochRef.current.epoch + 1 }
|
||||
}
|
||||
|
||||
return (
|
||||
<NewWorktreeModalContent
|
||||
key={`${openEpochRef.current}:${hostEpochRef.current.epoch}`}
|
||||
{...props}
|
||||
/>
|
||||
)
|
||||
return <NewWorktreeModalContent key={`${session.openEpoch}:${props.hostId}`} {...props} />
|
||||
}
|
||||
|
||||
function NewWorktreeModalContent(props: NewWorktreeModalProps) {
|
||||
|
||||
Reference in New Issue
Block a user