Persist server URL and gate auto-connect#3
Persist server URL and gate auto-connect#3RobertoVillegas wants to merge 2 commits intoR44VC0RP:mainfrom
Conversation
📝 WalkthroughWalkthroughThis PR integrates AsyncStorage to persist the OpenCode server URL locally, tracking storage readiness and saved URL state. The auto-connect logic now waits for storage initialization before attempting connection, routing is simplified by removing the intermediate connecting state, and dependencies are updated. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant App as App (_layout.tsx)
participant Provider as OpenCodeProvider
participant Storage as AsyncStorage
participant Router as Router (index.tsx)
User->>App: Launch app
App->>Provider: Initialize provider
Provider->>Storage: Load saved server URL
Storage-->>Provider: URL (or null)
rect rgba(76, 175, 80, 0.1)
Note over Provider: Set storageReady = true
end
Provider-->>App: Context updated<br/>(storageReady, hasSavedServerUrl)
alt hasSavedServerUrl && not attempted
rect rgba(33, 150, 243, 0.1)
App->>Provider: Trigger auto-connect
Provider->>Provider: Connect to server
end
Provider-->>App: connected = true
else No saved URL
App->>App: Skip auto-connect
end
App->>Router: Route based on connected state
alt connected = true
Router->>User: Navigate to /(tabs)/sessions
else connected = false
Router->>User: Navigate to /connect
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/providers/OpenCodeProvider.tsx (1)
135-135: Consider using a constant instead of a ref.
storageKeyRefnever changes after initialization. Since the value is static, it could be a module-level constant rather than a ref, which would be more idiomatic and slightly more efficient.🔎 Proposed refactor
Move this outside the component as a constant:
+const STORAGE_KEY = 'opencode.serverUrl'; + export function OpenCodeProvider({ children, defaultServerUrl = 'http://10.0.10.59:9034' }: OpenCodeProviderProps) { // Connection state const [connected, setConnected] = useState(false); const [connecting, setConnecting] = useState(false); const [error, setError] = useState<string | null>(null); const [serverUrl, setServerUrl] = useState(defaultServerUrl); const [storageReady, setStorageReady] = useState(false); const [hasSavedServerUrl, setHasSavedServerUrl] = useState(false); const clientRef = useRef<OpenCodeClient | null>(null); - const storageKeyRef = useRef('opencode.serverUrl');Then update usages of
storageKeyRef.currenttoSTORAGE_KEYon Lines 201, 241.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
app/_layout.tsxapp/index.tsxpackage.jsonsrc/providers/OpenCodeProvider.tsx
🧰 Additional context used
🧬 Code graph analysis (3)
src/providers/OpenCodeProvider.tsx (1)
src/hooks/useOpenCode.ts (1)
OpenCodeClient(10-10)
app/index.tsx (1)
src/providers/OpenCodeProvider.tsx (1)
useOpenCode(638-644)
app/_layout.tsx (1)
src/providers/OpenCodeProvider.tsx (1)
useOpenCode(638-644)
🔇 Additional comments (12)
app/index.tsx (1)
5-11: LGTM! Cleaner routing logic.The simplified approach now relies solely on the
connectedstate, which aligns well with the new storage-aware bootstrap flow. The intermediate connecting state is appropriately handled inapp/_layout.tsx, preventing blank screens during connection attempts.src/providers/OpenCodeProvider.tsx (7)
3-3: LGTM! AsyncStorage import added.The import is correctly added to support server URL persistence.
80-81: LGTM! Context interface extended with storage state.The addition of
storageReadyandhasSavedServerUrlproperly exposes storage initialization state to consumers, enabling conditional auto-connect logic.
131-132: LGTM! Storage state initialized.Both boolean states start as
false, which is appropriate before storage is checked.
161-161: LGTM! Correct type for React Native environment.Changing from
NodeJS.TimeouttoReturnType<typeof setInterval>properly handles the React Native environment wheresetIntervalreturns a number rather than a Node.jsTimeoutobject.
201-202: LGTM! Server URL persisted on successful connection.The URL is correctly persisted after the connection test succeeds, and
hasSavedServerUrlis updated accordingly. Error handling is implicit through the try-catch block.
236-256: LGTM! Bootstrap logic properly initializes storage state.The effect correctly:
- Runs once on mount (empty deps)
- Loads the saved URL from AsyncStorage
- Updates both
serverUrlandhasSavedServerUrlwhen found- Sets
storageReadyin the finally block regardless of success/failure- Silently handles errors, which is acceptable for storage access
607-608: LGTM! Storage state exposed in context.The new state values are correctly included in the context value, making them available to consumers via
useOpenCode().app/_layout.tsx (2)
9-9: LGTM! Storage state integrated into auth flow.The destructuring correctly includes
storageReadyandhasSavedServerUrl, which are now required to determine auto-connect behavior.
16-20: LGTM! Auto-connect properly gated by storage state.The condition now ensures auto-connect only triggers when:
- Not already attempted (
!autoConnectAttempted)- Storage is initialized (
storageReady)- A saved URL exists (
hasSavedServerUrl)The dependency array is correctly updated to include the new state values, ensuring the effect re-evaluates when storage bootstrap completes.
package.json (2)
14-14: AsyncStorage version is compatible with Expo 54 and React Native 0.81.5.Version ^2.1.0 meets the minimum requirement (RN >= 0.65) and is verified to work with Expo SDK 54. Proceed with the code change. Use
npx expo installduring setup to ensure native binaries are correctly aligned.
27-28: Clarify the downgrades—package-lock.json is out of sync.The downgraded versions align with Expo 54 requirements:
react-native-screens ~4.16.0andreact-native-svg 15.12.1are the official bundled versions for Expo SDK 54, and both are compatible with React Navigation v7 in use here (which requires onlyreact-native-screens >= 4.0.0).However,
package-lock.jsoncontains the older versions (^4.19.0and^15.15.1), which is out of sync withpackage.json. Runnpm installornpm cito update the lock file and document the rationale (e.g., "standardizing on Expo 54 bundled versions" or "aligning with native build requirements").
|
This is a great idea! I had similar issues when starting up the app on my machine and had made some quick changes to default the connection url to be my computer's IP address - i like this approach a lot better. Smart to persist it, and skip auto-connecting when no connection urls have been added yet. |
2 participants
Summary
This PR fixes the blank-screen startup flow and makes auto-connect behavior predictable by persisting the server URL and only auto-connecting when a saved URL exists. It also aligns Expo dependency versions to the expected compatibility set, fixes an RN type issue, and keeps the repo on npm by using the npm lockfile.
What changed
Why
Impact
Testing