refactor: upstream shared session-page refactors#321
Merged
Conversation
The search schemas captured `new Date()` in a module-level const and fed it to `.catch`/`.default`, so "now" froze at bundle load and drifted in a long-lived tab. Wrap each schema in a `make…SearchSchema(now)` factory that injects a clock, bind the real dependency in the existing singleton export (consumers unchanged), and move the date computation into `.catch`/`.default` thunks so it is recomputed on every parse. - historySearch: `start`/`end` recomputed via startOfDay/endOfDay(now()). - sessionSearch: `trackingStart` transform reads the injected clock (copied so the caller's Date is never mutated). - wrappedSearch: `.catch`/`.default` are thunks; `.max` stays frozen at construction (Zod's `.max` takes a value, not a thunk) with a comment. Tests now inject a fixed clock and assert exact fallback dates, removing the flaky pattern where the test recomputed `new Date()` to mirror the source. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Deploying rainbow with
|
| Latest commit: |
3599307
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://fa656e66.rainbow-ctx.pages.dev |
| Branch Preview URL: | https://session-refactors-upstream.rainbow-ctx.pages.dev |
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request upstreams a set of shared refactors used by the session-related pages: it fixes “now”-drift in long-lived tabs by recomputing schema date defaults per parse, replaces scattered millisecond literals with shared time-unit constants, consolidates duration formatting into a shared helper, and extracts a reusable TrendIcon component for consistent trend rendering.
Changes:
- Introduce
make…SearchSchema(now)factories so Zod.catch/.defaultfallbacks are recomputed per parse (tests updated to pinnowdeterministically). - Add shared
MS_PER_MINUTE/HOUR/DAYconstants and replace inline1000 * 60 * …literals in several call sites. - Add
formatDuration(ms)helper (with unit tests) and replace bespoke duration rendering on the session page; extract repeated trend-arrow logic intoTrendIcon.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/time.ts | Adds shared millisecond-per-unit constants (MS_PER_MINUTE/HOUR/DAY). |
| src/stats/progression.ts | Replaces day calculations using inline literals with MS_PER_DAY. |
| src/queryClient.ts | Uses MS_PER_DAY for React Query persistence max age. |
| src/helpers/duration.ts | Adds shared formatDuration(ms) formatter used by session UI. |
| src/helpers/duration.unit.test.ts | Adds unit coverage for formatDuration behavior (rounding/clamping/rollover). |
| src/components/TrendIcon.tsx | Introduces reusable trend icon component mapping direction → icon. |
| src/routes/session/$uuid.tsx | Adopts TrendIcon, formatDuration, and time constants; removes inline duration renderer. |
| src/routes/session/-uuid.ui.test.tsx | Broadens duration assertion to allow new formats (Xd Yh, Xh YYm, Xm). |
| src/routes/wrapped/$uuid.tsx | Replaces hour/day literals with MS_PER_HOUR/MS_PER_DAY. |
| src/contexts/PlayerVisits/helpers.ts | Uses MS_PER_DAY for “days since visit” weighting. |
| src/contexts/KnownAliases/helpers.ts | Uses MS_PER_DAY for the “aliases resolved within last year” cutoff. |
| src/schemas/historySearch.ts | Adds makeHistoryExploreSearchSchema(now) to recompute date fallbacks per parse. |
| src/schemas/historySearch.unit.test.ts | Pins now and updates expectations for deterministic history search defaults. |
| src/schemas/sessionSearch.ts | Adds makeSessionSearchSchema(now) so tracking-start default is recomputed per parse. |
| src/schemas/sessionSearch.unit.test.ts | Pins now and updates tests for deterministic tracking-start defaults. |
| src/schemas/wrappedSearch.ts | Adds makeWrappedSearchSchema(now) so default/catch year is recomputed per parse. |
| src/schemas/wrappedSearch.unit.test.ts | Pins now and expands wrapped-year validation coverage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Replace the repeated `24 * 60 * 60 * 1000` / `3_600_000` / `1000 * 60 * 60` duration literals with MS_PER_MINUTE/HOUR/DAY from a new #time.ts, so duration maths reads in named units instead of hand-rolled products. Adopted across the existing code: queryClient's gcTime maxAge, the PlayerVisits recency weight, the KnownAliases one-year cutoff, the wrapped page's session-duration and short-year checks, the session-list page's duration/ETA maths, and progression's daysElapsed computations. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The sessions list page had its own renderDuration(end, start) while the detail page (upcoming) needs the same thing. They rounded and worded durations differently, a duplication set up to drift. Consolidate on a single ms-based helper in #helpers/duration.ts. Callers with a start/end pair now subtract at the call site, keeping the parameter direction visible (the old renderDuration took end first). The list page inherits the shared formatter's behaviour: round rather than floor to the minute, clamp negatives to 0m, and roll sessions over 24h into "Xd Yh" instead of "25h 00m". Broadened the sessions-table UI test accordingly, since long sessions now render without a minutes field. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The session page picked a TrendingUp/Down/Flat icon from a direction via inline `direction === ...`/`diff > 0` conditionals repeated in three places (the stat card, the milestone row, and the loading skeleton). Extract a #components/TrendIcon.tsx component taking direction plus color/fontSize (forwarded to the MUI icon) and an sx escape hatch for pixel sizes / palette-path colours, and render <TrendIcon direction=... /> in each spot instead of branching by hand. Colour stays per-call. The direct TrendingUp/Down/Flat imports drop out entirely. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Amund211
force-pushed
the
session-refactors-upstream
branch
from
0a2d18d to
3599307
Compare
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
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.
What
Four self-contained refactors carved out of the upcoming session detail page branch so they can land on their own first. Each is a single commit; none depends on the detail page existing.
refactor: recompute search-schema date defaults per parsemake…SearchSchema(now)factories so date.catch/.defaultfallbacks are recomputed per parse instead of frozen at module load. Fixes a real bug: "now" drifted in long-lived tabs.refactor: add shared time-unit constants and adopt them#time.tswithMS_PER_MINUTE/HOUR/DAY; replaces scattered24 * 60 * 60 * 1000-style literals acrossqueryClient, the contexts, the wrapped page, the session-list page, andprogression.refactor: unify duration formatters into formatDurationrenderDurationonto a shared#helpers/duration.ts.0m, and rolls >24h intoXd Yh. UI test broadened accordingly.refactor: extract a shared TrendIcon componentdirection === …/diff > 0trend-arrow conditionals on the session page with a#components/TrendIcon.tsxcomponent.Why
These are pure cleanups/one bug fix to shared code that stand on their own. Splitting them out shrinks the session-detail PR to just the feature and lets these get reviewed independently.
Notes
#time.ts(2) lands beforeduration.ts(3), which importsMS_PER_MINUTEfrom it.🤖 Generated with Claude Code