Conversation
- Migrated MatchController and TeamController from Redux to useMatch/useController hooks - Updated TeamController to generate UUIDs for penalties locally - Fixed type mismatches in Clock and HalfStops components - Included pending Context migrations for AssetController and other components found in working tree to ensure build consistency
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #128 +/- ##
===========================================
+ Coverage 52.15% 86.86% +34.71%
===========================================
Files 69 33 -36
Lines 2161 1599 -562
Branches 563 467 -96
===========================================
+ Hits 1127 1389 +262
+ Misses 1034 210 -824
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
- Remove sync prop and hydration guards - Firebase is now single source of truth - All state updates flow through Firebase onValue subscriptions - Add Firebase Emulator support (VITE_USE_EMULATOR=true) - Update CI to run e2e tests against Firebase Emulator - Add docker-compose.yml for local emulator setup - Update AGENTS.md with new architecture documentation
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
- TeamAssetController.spec.tsx: 49 tests covering player lineup management, axios API calls, window.confirm dialogs - Team.spec.tsx: 28 tests covering player CRUD operations with forms and axios persistence - MatchesOnPitch.spec.tsx: 10 tests covering match fetching with window.prompt and axios - PlayerCard.spec.tsx: 19 tests covering canvas text measurement and player rendering All tests use proper TypeScript typing (no eslint-disable, no 'as any'). Established patterns for axios mocks, window API spies, and canvas mocking.
|
Deployed to staging: https://staging.irdn.is |
The button was incorrectly placed next to the post-login Stjórnandi selector in LoginPage. It belongs next to the pre-login Skjár selector in Controller, replacing the auto-navigate onChange behavior. LoginPage reverted to its original onChange auto-select. Co-authored-by: Sisyphus <sisyphus@opencode.ai>
|
Deployed to staging: https://staging.irdn.is |
Display a RingLoader spinner when a screen is selected or user is authenticated but Firebase data hasn't loaded yet. The ready state tracks when all three onValue callbacks (match, controller, view) have fired at least once, preventing stale default state from flashing on page refresh. Co-authored-by: Sisyphus <sisyphus@opencode.ai>
Test that spinner shows when Firebase state is not ready or auth is not loaded, and that it does not show in pre-login state or when everything is loaded. Co-authored-by: Sisyphus <sisyphus@opencode.ai>
|
Deployed to staging: https://staging.irdn.is |
The previous commit moved RefreshHandler to App.tsx as a top-level sibling, causing the orange button to overlap match controls and block pointer events on the Byrja/Pása buttons, breaking e2e tests. Co-authored-by: Sisyphus <sisyphus@opencode.ai>
|
Deployed to staging: https://staging.irdn.is |
…offset sync - Remove compensation block that was replacing Firebase timestamps with Date.now() - 150 - Add Firebase .info/serverTimeOffset subscription to measure client-server clock skew - Implement getServerTime() utility function using serverTimeOffsetRef - Update all timestamp write operations (startMatch, pauseMatch, matchTimeout, buzz) to use getServerTime() - Export getServerTime() via useMatch() hook for consumers
- Update Clock.tsx to use getServerTime() instead of Date.now() - Update TwoMinClock.tsx to use getServerTime() for elapsed time calculation - Update TimeoutClock.tsx to use getServerTime() for timeout duration - Update ScoreBoard.tsx to use getServerTime() for buzzer timing - Add getServerTime to all component dependency arrays
- Add server time offset test suite in FirebaseStateContext.spec.tsx - Test getServerTime availability and default behavior - Test Firebase started timestamp preservation (no Date.now() mutation) - Test write operations use getServerTime for timestamps - Update TimeoutClock.spec.tsx and TwoMinClock.spec.tsx for getServerTime compatibility
…imestamp Compute countdown duration from local time (what the user/UI sees) then place the started timestamp in the getServerTime() coordinate system so Clock.tsx elapsed calculation (getServerTime() - started) gives the correct negative countdown value. Previously countdown() used moment() (Date.now) for the absolute started timestamp while Clock.tsx read getServerTime() (Date.now + offset), causing the countdown to instantly expire when there was any server time offset.
|
Deployed to staging: https://staging.irdn.is |
There was a problem hiding this comment.
Pull request overview
This PR completes the migration from Redux to a Firebase-backed React Context architecture, adding stronger guards against multi-client race conditions and improving type safety, emulator support, and test coverage.
Changes:
- Removed Redux reducers/actions and rewired UI components to
FirebaseStateContext+LocalStateContext. - Added runtime-validated Firebase snapshot parsers and emulator configuration for unit/e2e stability.
- Updated Playwright + CI workflow to run against Firebase emulators and adjusted e2e helpers/tests accordingly.
Reviewed changes
Copilot reviewed 81 out of 100 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| clock/src/reducers/listeners.ts | Removed Redux listeners slice in favor of context-driven Firebase state. |
| clock/src/reducers/listeners.spec.ts | Removed unit tests for deleted Redux reducer. |
| clock/src/reducers/controller.ts | Removed Redux controller reducer in favor of context actions/state. |
| clock/src/reducers/auth.ts | Removed Redux auth reducer in favor of LocalStateContext. |
| clock/src/match/TwoMinClock.tsx | Migrated penalty clock from Redux to context and server-time. |
| clock/src/match/TimeoutClock.tsx | Migrated timeout clock from Redux to context and server-time. |
| clock/src/match/Team.tsx | Removed Redux connector; reads match type from context. |
| clock/src/match/Clock.tsx | Migrated main match clock to context and server-time. |
| clock/src/match/Clock.spec.tsx | Reworked tests to render with context providers (no Redux store). |
| clock/src/match-controller/TeamController.tsx | Removed Redux dispatch; uses context match actions. |
| clock/src/match-controller/MatchController.tsx | Removed Redux connector; uses match/controller contexts. |
| clock/src/match-controller/MatchController.spec.tsx | Reworked tests to use providers; no longer asserts dispatched Redux actions. |
| clock/src/index.tsx | Removed Redux Provider/persist gate; added local+firebase context providers. |
| clock/src/hooks/useGlobalShortcuts.ts | Added hook-based global shortcuts wired to context actions. |
| clock/src/hooks/useFirebaseSync.ts | Removed Redux-based Firebase sync hook (now handled by context). |
| clock/src/firebaseDatabase.ts | Switched state writes to update() and added partial sync helper. |
| clock/src/firebase.ts | Added emulator support and Vite env-based config selection. |
| clock/src/controller/media/MediaManager.tsx | Removed Redux connector; reads auth/prefix from local context. |
| clock/src/controller/media/ImageList.tsx | Removed Redux dispatch; uses controller context actions. |
| clock/src/controller/asset/team/Team.tsx | Migrated team asset editing to controller/match contexts. |
| clock/src/controller/asset/team/MatchesOnPitch.tsx | Migrated to contexts and added local state; removed Redux wiring. |
| clock/src/controller/asset/team/MatchesOnPitch.spec.tsx | Added unit tests with mocked context hooks and axios. |
| clock/src/controller/asset/team/MatchSelector.tsx | Removed Redux connector; uses controller context for match selection. |
| clock/src/controller/asset/Substitution.tsx | Removed Redux connector; reads view state from context. |
| clock/src/controller/asset/RemoveAssetDropzone.tsx | Removed Redux dispatch; uses controller context remove action. |
| clock/src/controller/asset/PlayerCard.tsx | Converted to functional component using view context + memoized font sizing. |
| clock/src/controller/asset/PlayerCard.spec.tsx | Added unit tests for PlayerCard with mocked view context. |
| clock/src/controller/asset/AssetQueue.tsx | Removed Redux connector; uses controller context for queue updates. |
| clock/src/controller/asset/AssetController.tsx | Removed Redux wiring and legacy shortcut component usage. |
| clock/src/controller/asset/Asset.tsx | Removed Redux wiring; uses controller/view/local auth contexts. |
| clock/src/controller/TeamSelector.tsx | Removed Redux wiring; uses match context update. |
| clock/src/controller/RefreshHandler.tsx | Removed Redux wiring; uses controller+local auth contexts. |
| clock/src/controller/RedCardManipulation.tsx | Removed Redux connector; uses match context for red card updates. |
| clock/src/controller/PenaltiesManipulationBox.tsx | Removed Redux connector; uses match context for penalties. |
| clock/src/controller/MatchActions.tsx | Removed Redux connector; uses match/controller contexts + local prefix. |
| clock/src/controller/MatchActionSettings.tsx | Removed Redux connector; uses match/controller/view contexts. |
| clock/src/controller/MatchActionSettings.spec.tsx | Added unit tests with mocked context hooks. |
| clock/src/controller/LoginPage.tsx | Simplified authenticated UI to prefix selector + logout only (no login form). |
| clock/src/controller/HalfStops.tsx | Removed Redux wiring; uses match context setters. |
| clock/src/controller/Controller.tsx | Refactored controller to 3-state UX using local+firebase contexts. |
| clock/src/controller/Controller.spec.tsx | Added unit tests covering the 3 controller states via mocked hooks. |
| clock/src/contexts/firebaseParsers.ts | Added runtime parsing/validation for Firebase snapshot data. |
| clock/src/contexts/LocalStateContext.tsx | Added local context for auth/listenPrefix + authData subscription. |
| clock/src/constants.ts | Centralized controller/view constants and backgrounds. |
| clock/src/actions/view.ts | Removed Redux view actions. |
| clock/src/actions/remote.ts | Removed Redux remote actions. |
| clock/src/actions/match.ts | Removed Redux match actions/thunks. |
| clock/src/actions/global.ts | Removed Redux global actions. |
| clock/src/actions/controller.ts | Removed Redux controller actions. |
| clock/src/StateListener.tsx | Updated connection indicator to rely on local auth context. |
| clock/src/GlobalShortcut.ts | Removed legacy shortcut component (replaced by hook). |
| clock/src/GlobalShortcut.spec.tsx | Removed tests for deleted shortcut component. |
| clock/src/App.tsx | Reworked app boot flow into 3 states and added disconnect/spinner behavior. |
| clock/src/App.spec.tsx | Added unit tests for the 3 App states and spinner behavior with mocked hooks. |
| clock/src/App.spec.jsx | Removed old Redux-based App tests. |
| clock/playwright.config.ts | Adjusted CI worker/webServer behavior for emulator stability. |
| clock/package.json | Removed Redux dependencies/types, aligning deps with context architecture. |
| clock/e2e/penalties.spec.ts | Updated e2e tests to use emulator helpers and deterministic fake clock. |
| clock/e2e/match-flow.spec.ts | Updated e2e flow tests to use emulator helpers and deterministic fake clock. |
| clock/e2e/image-upload.spec.ts | Updated e2e tests to use emulator helpers (removed env credential parsing). |
| clock/e2e/fixtures/test-helpers.ts | Added emulator user/bootstrap, DB cleanup, and fake clock utilities. |
| clock/e2e/basic-navigation.spec.ts | Updated navigation e2e to emulator helpers and deterministic fake clock. |
| clock/e2e/assets.spec.ts | Updated assets e2e to emulator helpers and added team-view describe block. |
| clock/AGENTS.md | Updated architectural docs for Firebase-only context model + emulator/dev notes. |
| FIX_CI_EMULATOR.md | Added investigation notes and mitigation ideas for emulator CI flakiness. |
| AGENTS.md | Updated repo-level docs to reflect new clock architecture/testing approach. |
| .github/workflows/build.yml | Added emulator startup + Vite startup in CI before Playwright e2e run. |
| .firebaserc | Added default Firebase project for emulator/CLI usage. |
Files not reviewed (1)
- clock/pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… code Delete old unused syncState function (duplicate of syncPartialState) and rename syncPartialState to syncState. Both functions had identical update() semantics, making the old syncState dead code. The new name aligns with documentation which already referred to firebaseDatabase.syncState(). - Remove duplicate syncState function (lines 38-43) - Rename syncPartialState → syncState in firebaseDatabase.ts - Update 4 production call sites in FirebaseStateContext.tsx - Update 87 test references in FirebaseStateContext.spec.tsx - Simplify test mock to single syncState function Diff mechanism in applyMatchUpdate depends on update() semantics (not set()). All 540 tests pass.
…e writes Only reconstruct the penalty array that contains the key being modified. Previously both home2min and away2min were always filtered/mapped, creating new references and triggering unnecessary Firebase writes via the diff mechanism. Uses .some() check + conditional spread to only include changed arrays in the update object, reducing penalty operation Firebase writes by ~50%.
…e writes Add useRef guard to ensure buzz() and removeTimeout() fire exactly once when timeout reaches 0. Previously these fired on every 100ms interval tick once the condition was met. Latch resets when timeout changes (new timeout starts).
Add useRef guards for half-stop and countdown-end conditions to ensure pauseMatch() and buzz() fire exactly once when their conditions are met. Previously these fired on every 100ms interval tick once the condition was true. Latches reset when match state changes (started/countdown toggles). Also includes formatting fixes from prettier for previously committed files.
…tiesManipulationBox
|
Deployed to staging: https://staging.irdn.is |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Firebase State Hardening
Summary
This PR hardens the Firebase-only state architecture following the Redux → Context migration. It addresses race conditions, type safety issues, and adds comprehensive guards to prevent state corruption in multi-controller scenarios.
Problem
After migrating from Redux to React Context with Firebase as the single source of truth, several edge cases could still cause state corruption:
states//...creates invalid Firebase pathsSolution
Phase 1: Critical Guards
isMatchHydrated,isControllerHydrated,isViewHydrated) toFirebaseStateContext.tsxlistenPrefixchangeapply*functions whenlistenPrefixis emptyPhase 2: Optimistic Updates
ref.currentAND local state immediately before Firebase sync.catch(console.error)to all Firebase sync calls for error visibilityPhase 3: Type Safety
firebaseParsers.tswith runtime-validated parsers for Match, Controller, View, and Locations dataJSON.parse(JSON.stringify(...))withstructuredClonein 3 placesonValueFirebase subscriptionsPhase 4: Performance
useMemoto prevent unnecessary re-rendersPhase 5: Testing
FirebaseStateContext.spec.tsxwith 7 new unit tests covering:Phase 6: Documentation
clock/AGENTS.mdwith:Files Changed
New Files
clock/src/contexts/firebaseParsers.ts- Type-safe Firebase snapshot parsersclock/src/contexts/FirebaseStateContext.spec.tsx- Unit tests for state contextModified Files
clock/src/contexts/FirebaseStateContext.tsx- Main implementation with guards, optimistic updatesclock/AGENTS.md- Updated architecture documentationSupporting Files
.sisyphus/plans/firebase-hardening.md- Completed plan.sisyphus/notepads/firebase-hardening/learnings.md- Learnings for future referenceVerification
All acceptance criteria met:
pnpm buildpassespnpm testpasses (125 tests)pnpm linthas 0 errorsBreaking Changes
None. All changes are backward compatible.
Testing Notes
For manual testing of multi-controller sync:
listenPrefix(e.g., "viken")The hydration guards ensure that a newly-connected controller won't overwrite the existing match state.