Repository navigation
feat: Phase 0 foundations — auth/toast contexts, theme tokens, deps, route skeleton - #1
devin-ai-integration[bot] wants to merge 5 commits into
Conversation
Port AuthContext (JWT/Bearer) and ToastContext from cineflix-app web. Add ProtectedRoute primitive, brand/glass/surface theme tokens, and zustand + expo-video deps. Wire providers into root layout. Co-Authored-By: Mohamed Amine Abid <ma5741913@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
| useEffect(() => { | ||
| const checkAuth = async () => { | ||
| try { | ||
| const stored = await AsyncStorage.getItem('auth_token'); | ||
| if (stored) { | ||
| setAuthToken(stored); | ||
| await refreshUser(); | ||
| } | ||
| } finally { | ||
| setIsLoading(false); | ||
| } | ||
| }; | ||
| checkAuth(); | ||
| }, [refreshUser]); |
There was a problem hiding this comment.
📝 Info: Duplicate token hydration between api.ts module-level read and AuthContext
Both services/api.ts:7 and contexts/AuthContext.tsx:62 independently read auth_token from AsyncStorage. The module-level read at api.ts:7 fires on import and directly assigns to the module variable (authToken = t), while AuthContext reads the same key and calls setAuthToken(stored). In practice there's no race condition because the module import happens before the component mounts, and both AsyncStorage reads resolve to the same value. However, this dual-read pattern is redundant and could become confusing if the storage key ever diverges. The module-level read in api.ts is pre-existing code not changed by this PR, so this is just a maintenance note.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Acknowledged — both reads resolve the same auth_token key and the module import precedes mount, so no race. The AuthContext read stays as the canonical hydration point (it gates the isLoading spinner); the module-level read in api.ts is pre-existing. Left as-is to keep this PR scoped to foundations.
| const showToast = useCallback((message: string, type: ToastType = 'info', duration: number = 3000) => { | ||
| const id = Math.random().toString(36).substring(2, 9); | ||
| setToasts((prev) => [...prev, { id, message, type, duration }]); | ||
|
|
||
| if (duration > 0) { | ||
| setTimeout(() => hideToast(id), duration); | ||
| } |
There was a problem hiding this comment.
📝 Info: Toast setTimeout is never cleaned up on manual dismiss
In ToastContext.tsx:63-64, when a toast is shown with a positive duration, a setTimeout is scheduled to auto-dismiss it. If the user manually dismisses the toast via the X button before the timer fires, the timer still runs and calls hideToast(id) again. This is harmless because hideToast uses a filter (prev.filter(...)) which is a no-op when the toast is already removed. Additionally, since ToastProvider is mounted at the root layout and never unmounts, there's no risk of state updates on an unmounted component. Not a bug, but adding timer cleanup via useRef + clearTimeout would be cleaner.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Acknowledged — harmless as noted (filter no-op, provider never unmounts). Leaving as-is for the foundation PR; can add useRef/clearTimeout cleanup if we later allow manual dismissal mid-timer at scale.
| glass: { | ||
| background: 'rgba(19, 19, 43, 0.7)', | ||
| 'background-dark': 'rgba(10, 10, 31, 0.85)', | ||
| border: 'rgba(255, 255, 255, 0.08)', | ||
| hover: 'rgba(255, 255, 255, 0.12)', | ||
| }, | ||
| surface: { | ||
| background: '#0A0A1F', | ||
| card: '#13132B', | ||
| accent: '#1F1F35', | ||
| elevated: '#252540', | ||
| }, |
There was a problem hiding this comment.
📝 Info: New top-level Tailwind color groups (glass, brand, surface) don't conflict with existing netflix.glass keys
The new glass color group at tailwind.config.js:34-39 adds utilities like bg-glass-background, text-glass-border, etc. The pre-existing netflix.glass key (line 23) generates bg-netflix-glass. These are distinct utility names so there's no collision. However, bg-glass by itself won't work (no DEFAULT key in the glass object), which is fine since the intended usage is via sub-keys like glass-background. The naming could cause initial confusion for developers expecting bg-glass to work as a standalone utility.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Correct — no collision with netflix.glass (distinct utility names). The scales mirror web's tailwind.config.js and are used via sub-keys (bg-surface-card, border-glass-border, etc.); there's intentionally no DEFAULT, matching web. No action needed.
refreshUser now only clears the token on 401/403 (explicit auth rejection); network/5xx failures keep the stored token. apiRequest surfaces HTTP status. Wrap AuthContext value in useMemo. Co-Authored-By: Mohamed Amine Abid <ma5741913@gmail.com>
Add stub screens for every web route missing on mobile (movies, tv-shows, new-popular, browse, continue-watching, login, signup, watch/[type]/[id]) and register them in the root Stack. This lets each feature be built in an isolated file without touching shared layouts, enabling safe parallel work. Co-Authored-By: Mohamed Amine Abid <ma5741913@gmail.com>
Co-Authored-By: Mohamed Amine Abid <ma5741913@gmail.com>
Co-Authored-By: Mohamed Amine Abid <ma5741913@gmail.com>
| } catch (error) { | ||
| return { success: false, error: error instanceof Error ? error.message : 'Network error' }; |
There was a problem hiding this comment.
🟡 apiRequest catch block loses HTTP status, preventing token cleanup on non-JSON 401/403 responses
The PR adds status to ApiResponse and uses it in refreshUser (contexts/AuthContext.tsx:52) to detect 401/403 and clear invalid tokens. However, when response.json() at services/api.ts:35 throws (e.g., a 401/403 from a reverse proxy returning an HTML error page instead of JSON), execution falls to the catch block at services/api.ts:38-39, which returns { success: false, error: ... } without the status field. The response object (which holds .status) is scoped inside the try block and is inaccessible from catch. As a result, refreshUser cannot detect auth rejection, the stale token is never removed from AsyncStorage, and every subsequent app launch wastes a network request trying to validate the dead token.
Prompt for agents
In services/api.ts, the apiRequest function's catch block (lines 38-39) does not propagate the HTTP status code when response.json() throws. This happens because the response variable is declared inside the try block and is inaccessible in catch.
The fix is to capture the HTTP status before calling response.json(), so it's available in the catch block. For example, declare a variable like let httpStatus: number | undefined before the try block, assign it after the fetch (httpStatus = response.status), and include it in the catch return: { success: false, error: ..., status: httpStatus }.
This ensures that refreshUser in contexts/AuthContext.tsx can detect 401/403 errors even when the response body is not valid JSON (e.g., HTML error pages from reverse proxies).
Was this helpful? React with 👍 or 👎 to provide feedback.
| } catch { | ||
| // Network error — keep the token and current user state intact. | ||
| } |
There was a problem hiding this comment.
📝 Info: Dead catch block in refreshUser — apiRequest never rejects
The refreshUser function wraps authApi.getMe() in a try-catch (contexts/AuthContext.tsx:56-58) with a comment saying "Network error — keep the token and current user state intact." However, authApi.getMe() calls apiRequest() which has its own internal try-catch (services/api.ts:23-40) and always returns a resolved promise — it never throws. This means the catch block in refreshUser is dead code. The actual network error handling works correctly regardless (the response will have success: false without a status, so the token is preserved), but the misleading comment could confuse future developers into thinking this catch is providing meaningful error handling.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const data = await response.json(); | ||
| if (!response.ok) return { success: false, error: data.error || `HTTP error ${response.status}` }; | ||
| if (!response.ok) return { success: false, error: data.error || `HTTP error ${response.status}`, status: response.status }; | ||
| return data; |
There was a problem hiding this comment.
📝 Info: apiRequest returns raw backend JSON on success — no status field guaranteed
On the success path (services/api.ts:37), apiRequest returns data (the raw parsed JSON from the server) directly, not a constructed ApiResponse<T> object. This means the status field added to ApiResponse<T> is only reliably present on error responses (line 36). If any caller checks response.status after a successful response, they'd get whatever the backend happened to include (or undefined). Currently refreshUser only checks status on the failure path (after response.success is false), so this isn't a bug today — but the asymmetric contract is worth being aware of for future callers.
Was this helpful? React with 👍 or 👎 to provide feedback.
Summary
Phase 0 establishes the shared foundation every other feature in the web→mobile port depends on, so the remaining work can be built in parallel without colliding on shared files. No user-facing screen is implemented here yet — this is plumbing + a route skeleton.
What's included:
contexts/AuthContext.tsx): RN port of the webAuthContext. Token persisted inAsyncStorage; full method surface (login/register/googleLogin/logout/updateProfile/changePassword/refreshUser).refreshUseronly clears the token on an explicit401/403— transient/offline/5xx failures keep the session:contexts/ToastContext.tsx): RN port of the web toast system using Reanimated + lucide icons, type-specific styling, auto-dismiss.components/ProtectedRoute.tsx: auth-gating primitive (spinner while hydrating → redirect to/loginif unauthenticated).services/api.ts: addedgoogleLoginendpoint and surfaced HTTPstatusonApiResponseso auth can distinguish auth-failure from network-failure.app/_layout.tsx: wraps the app inAuthProvider→ToastProviderand registers the full Stack route skeleton.tailwind.config.js: addedbrand/glass/surfacetoken scales to match the web theme.zustand,expo-video.movies,tv-shows,new-popular,browse,continue-watching,login,signup,watch/[type]/[id]— all registered in the root Stack. Each is an isolated file so subsequent feature PRs can be developed in parallel without touching shared layouts.Verification:
npx tsc --noEmitpasses clean. No lint/test scripts exist in this repo.Link to Devin session: https://app.devin.ai/sessions/a6b2d78a5bc6487cb1ce9c6547791321
Requested by: @simoabid