Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR centralizes Firebase authentication state management and server-side session cookie renewal into a single AuthSessionProvider component. Previously, onAuthStateChanged listeners and anonymous login logic were duplicated across App.tsx, useFeedsSearch.ts, and the auth saga. The new provider uses onIdTokenChanged with a 5-minute interval to keep the session cookie fresh, leveraging localStorage for cross-tab deduplication.
Changes:
- Introduced
AuthSessionProviderwithuseAuthReadyhook, replacing scatteredonAuthStateChangedlisteners and the saga-based session cookie setter - Enhanced
session-service.tswith localStorage-based freshness tracking (isCookieFresh) to avoid redundant/api/sessionPOSTs across tabs - Minor unrelated changes: spacing fix in
HomePage.tsx, removal ofworkflow_dispatchfrom prod deployment workflow, and cleanup of unused leaflet dependencies
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/app/components/AuthSessionProvider.tsx |
New centralized auth provider with onIdTokenChanged, interval-based cookie renewal, and useAuthReady context |
src/app/components/AuthSessionProvider.spec.tsx |
Comprehensive unit tests for the new provider |
src/app/services/session-service.ts |
Added localStorage-based freshness check to skip redundant session POSTs |
src/app/providers.tsx |
Wraps children with AuthSessionProvider |
src/app/App.tsx |
Removed duplicated auth state listener, now uses useAuthReady |
src/app/[locale]/feeds/lib/useFeedsSearch.ts |
Replaced local useFirebaseAuthReady with shared useAuthReady |
src/app/store/saga/auth-saga.ts |
Removed saga-based session cookie setting on login |
src/app/[locale]/components/HomePage.tsx |
Fixed spacing around translated text segments |
.github/workflows/vercel-prod-on-release.yml |
Removed workflow_dispatch trigger |
yarn.lock |
Removed unused leaflet/react-leaflet dependencies |
You can also share your feedback on Copilot code review. Take the survey.
| * and manages a single `onAuthStateChanged` listener that: | ||
| * | ||
| * 1. Triggers anonymous sign-in when no user exists. | ||
| * 2. Re-establishes the `md_session` cookie on return visits (Firebase | ||
| * restores auth from IndexedDB but the 1-hour cookie has expired). | ||
| * 3. Schedules the next renewal at exactly `expiresAt - 5 min` using | ||
| * a setTimeout derived from the value stored in localStorage. |
| on: | ||
| workflow_dispatch: | ||
| release: |
|
*Lighthouse ran on https://mobilitydatabase-93s48x1zh-mobility-data.vercel.app/ * (Desktop)
*Lighthouse ran on https://mobilitydatabase-93s48x1zh-mobility-data.vercel.app/feeds * (Desktop)
*Lighthouse ran on https://mobilitydatabase-93s48x1zh-mobility-data.vercel.app/feeds/gtfs/mdb-2126 * (Desktop)
*Lighthouse ran on https://mobilitydatabase-93s48x1zh-mobility-data.vercel.app/feeds/gtfs_rt/mdb-2585 * (Desktop)
*Lighthouse ran on https://mobilitydatabase-93s48x1zh-mobility-data.vercel.app/feeds/gbfs/gbfs-flamingo_porirua * (Desktop)
|
Summary:
Addition of server side cookie token renewal
Centralized the state change hook
Expected behavior:
Note:
When a user enters the application
onIdTokenChangedhook will trigger setting the encrypted user data in a secure cookie. When the cookie is set, it will store it's expiration time in localstorage so that it's expiration time synchronizes with other tabs the user could open. If the user opens another tabonIdTokenChangedwill fire, but instead of resetting the token it will do an expiration check using the value in localstorage and return early if the token is still valid. On each tab, the expiration of the token will check every 5 minutesThis is not the most efficient solution but the simplest. By doing interval checks every 5 mins with early returns, it assures that the token will always be valid (The token will revalidate 5 minutes before expiration). I was exploring other options of setting the timeout interval to the exact expiration of the token, but synchronizing that with the other tabs + considering race conditions increased the complexity of the code so much it wasn't worth it
Testing tips:
Basic testing: Go in the application and use it like normal: emphasize feeds search, feed detail page, sign in, sign out. Do this with many tabs open
Advanced testing:
(long)
Go onto the application take note of the expirationAt in localstorage (wait 1 hour), come back to the application after 1 hour and see that that same value was updated
Go onto the application, see that a network call was made to
api/session, open a new tab and see that there is no new call made toapi/session. Sign in, you should see a call a call toapi/session, same thing if you logoutPlease make sure these boxes are checked before submitting your pull request - thanks!
yarn testto make sure you didn't break anything