feat(companion): user date/time format preferences via shared @shelf/datetime - #2798
Merged
Conversation
Move the webapp's pure, locale-leak-free date/time formatter into a new `@shelf/datetime` workspace package so the webapp and the companion (Expo/RN) app share ONE implementation and their date rendering never drifts. - New `packages/datetime` (TS source, consumed like `@shelf/database`): the formatter core verbatim, with the two webapp-coupled type imports replaced by local string-union enums (mirroring the Prisma enums member-for-member) and a dependency-free `FormatHints` type in place of `ClientHint`. - `apps/webapp/app/utils/date-format.ts` becomes a thin `export *` re-export, so all ~54 webapp import sites are unchanged. Zero behavior change — the webapp's 57 formatter tests pass against the re-exported package. - Wire the package into both bundlers (webapp `ssr.noExternal`; Metro transpiles the TS source directly, verified via an export build).
Add the four format-preference columns (dateFormat, timeFormat, weekStart, timeZone) to the `requireMobileAuth` user select so `/api/mobile/me` returns them. They flow through `safeUser` unchanged; internal-only fields stay stripped. Additive and backward-compatible — older companion builds ignore the new keys. Add a `requireMobileAuth` test asserting the columns are selected AND returned (the only automated guard against a future refactor dropping them).
Render every date/time in the companion app in the acting user's Shelf format preferences (date format, time format, timezone) instead of device-local, using the shared `@shelf/datetime` formatter. - Resolve the raw prefs from `/api/mobile/me` once in `OrgContext`, with a device-hint fallback (`Intl.DateTimeFormat().resolvedOptions()`) for any unset field and for pre-format-prefs servers, so unknown-user states stay device-local rather than an arbitrary US/UTC default. - New `useDateFormatter()` / `useFormatPrefs()` hooks bind the resolved prefs. - Swap 12 display surfaces (bookings, audits, assets, kits, notes, custom fields) onto the bound formatter. Module-scope helpers take `prefs`; absolute calendar dates (custom-field DATE) render without a timezone shift; the shared `formatDue` gains a `prefs` arg. Device-tz capture for submission and the relative booking countdown are untouched. - Remove the device-local `formatDate`/`formatDateTime` from `lib/constants`. Ships via EAS OTA (pure JS, no native changes).
🩺 React Doctor — webapp✅ No new findings on the files changed by this PR. Run locally with |
🩺 React Doctor — companionFindings on the files changed by this PR:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cbbfea38c7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
There was a problem hiding this comment.
🟡 Not ready to approve
The companion Home booking card date range display changes in a way that may contradict the PR’s stated “format-only” intent (compact label → full default), and should be clarified or adjusted.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR aligns the companion app’s visible date/time rendering with the webapp by extracting the pure preference-aware formatter into a new shared workspace package (@shelf/datetime), exposing user date/time preferences via the mobile /me endpoint, and updating companion screens to format using the resolved user prefs (with device-hint fallback).
Changes:
- Introduce
@shelf/datetimeas a shared, dependency-free formatter package and re-export it from the webapp’s existing~/utils/date-formatentrypoint. - Extend
requireMobileAuth’s user select/response to include raw date/time preference columns for/api/mobile/me, with a regression test. - Add companion-side format-pref resolution in
OrgContextand replace device-local formatting across multiple companion UI surfaces viauseDateFormatter()/useFormatPrefs().
File summaries
| File | Description |
|---|---|
| pnpm-lock.yaml | Adds workspace links for the new @shelf/datetime package in relevant importers. |
| packages/datetime/tsconfig.json | TypeScript config for the new shared datetime package. |
| packages/datetime/src/index.ts | New shared formatter/detector/resolver implementation (moved from webapp, Prisma-decoupled). |
| packages/datetime/package.json | Defines the @shelf/datetime workspace package and its TS-source exports. |
| apps/webapp/vite.config.ts | Ensures Vite SSR bundles @shelf/datetime via ssr.noExternal. |
| apps/webapp/package.json | Adds @shelf/datetime as a webapp dependency. |
| apps/webapp/app/utils/date-format.ts | Replaces formatter implementation with a thin export * re-export of @shelf/datetime. |
| apps/webapp/app/modules/api/mobile-auth.server.ts | Adds date/time preference columns to the mobile-auth user select so /api/mobile/me can return them. |
| apps/webapp/app/modules/api/mobile-auth.server.test.ts | Adds a unit test to guard selection/return of the four preference fields and stripping internal fields. |
| apps/companion/package.json | Adds @shelf/datetime as a companion dependency. |
| apps/companion/lib/use-date-formatter.ts | Introduces hooks to expose resolved prefs and bound formatter helpers to companion components. |
| apps/companion/lib/org-context.tsx | Resolves raw prefs from /api/mobile/me + device hints into concrete formatPrefs stored in context. |
| apps/companion/lib/constants.ts | Removes legacy device-local toLocale* date formatting helpers. |
| apps/companion/lib/audit-format.ts | Updates audit due-date formatting to use the shared formatter with resolved prefs. |
| apps/companion/lib/api/types.ts | Extends the /me response typing to include optional raw date/time preference fields. |
| apps/companion/components/asset-edit/custom-field-input.tsx | Renders DATE custom field display labels using the bound formatter (preserving date-only semantics). |
| apps/companion/components/asset-detail/notes-section.tsx | Formats note timestamps and markdoc {% date %} tokens using user prefs/timezone. |
| apps/companion/components/asset-detail/custom-fields-section.tsx | Formats DATE custom fields using the bound formatter. |
| apps/companion/app/(tabs)/home.tsx | Updates booking/audit card date displays to use resolved prefs-based formatting. |
| apps/companion/app/(tabs)/bookings/new.tsx | Uses the bound formatter for booking start/end picker display labels. |
| apps/companion/app/(tabs)/bookings/index.tsx | Uses the bound formatter for booking list date/time rendering. |
| apps/companion/app/(tabs)/bookings/edit.tsx | Uses the bound formatter for booking edit picker display labels. |
| apps/companion/app/(tabs)/bookings/[id].tsx | Uses the bound formatter for booking detail date/time rendering. |
| apps/companion/app/(tabs)/audits/index.tsx | Uses the shared formatDue with resolved prefs for audit list due labels. |
| apps/companion/app/(tabs)/audits/[id].tsx | Uses the bound formatter for audit detail date/time rendering. |
| apps/companion/app/(tabs)/assets/kits/[id].tsx | Uses the bound formatter for kit detail date rendering. |
| apps/companion/app/(tabs)/assets/[id].tsx | Uses the bound formatter for asset detail date rendering. |
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
- Files reviewed: 26/27 changed files
- Comments generated: 1
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe PR adds the shared ChangesDatetime preference flow
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant MobileAuth
participant OrgContext
participant CompanionScreen
participant Datetime
MobileAuth->>OrgContext: return raw formatting preferences
OrgContext->>Datetime: resolve preferences with device hints
CompanionScreen->>OrgContext: read resolved formatPrefs
CompanionScreen->>Datetime: format displayed date or time
Datetime-->>CompanionScreen: return formatted value
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (2)
packages/datetime/src/index.ts (1)
599-682: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftConsider unit tests for this package.
formatDateis now the single formatter for two apps and carries substantial branching: date-only strings,localeOnly, name-month preferences, additiveincludeWeekday, preset folding, and both clocks. No test file accompaniespackages/datetime. A small table-driven suite over the fiveDateFormatPreferencevalues crossed with the common option shapes would lock the behavior the PR promises to preserve.Do you want me to generate that test suite?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/datetime/src/index.ts` around lines 599 - 682, Add a focused, table-driven unit-test suite for formatDate covering all five DateFormatPreference values and common option shapes, including date-only strings, localeOnly, name-month preferences, additive includeWeekday, preset folding, and both 12-hour and 24-hour clocks. Use stable timezone/locale inputs and assert the promised formatted output without changing production behavior.apps/webapp/app/modules/api/mobile-auth.server.test.ts (1)
288-301: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider a factory for the user row.
dbRowhardcodes every field inline. The coding guidelines ask for factories with field overrides instead of hardcoded test data. If a user-row factory already exists in the webapp test helpers, reuse it and override only the four preference fields.As per coding guidelines: "Use factories for consistent, realistic test data, allow field overrides, and avoid hardcoded test data where practical."
#!/bin/bash # Description: Look for existing user/test factories in the webapp test helpers. fd -H -t f -i 'factor' apps/webapp -E node_modules rg -nP -i 'export (const|function) (create|make|build)\w*User' apps/webapp -g '*.ts' -g '*.tsx'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/webapp/app/modules/api/mobile-auth.server.test.ts` around lines 288 - 301, Replace the hardcoded dbRow object definition with a call to an existing user-row factory from the webapp test helpers, passing field overrides for only the four preference fields (dateFormat, timeFormat, weekStart, timeZone) while relying on the factory to provide the base user data with id, email, firstName, lastName, profilePicture, onboarded, deletedAt, and lastMobileActiveAt. Preserve the test's behavior by ensuring the factory returns the same data shape when called with these overrides.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/companion/app/`(tabs)/bookings/edit.tsx:
- Around line 65-66: Update the editable booking flow around useDateFormatter,
DateTimePicker state, and toLocalWire submission so display, picker values, and
submitted times all use the same timezone. Prefer device-local formatting for
the picker button labels, or explicitly convert values from the preference
timezone before passing them to the picker and submission, preserving consistent
from/to values throughout editing.
In `@apps/companion/components/asset-edit/custom-field-input.tsx`:
- Around line 376-384: Update the display-value logic near useDateFormatter and
the displayValue assignment to validate the raw value before calling formatDate.
Reuse the component’s existing parsed/validated date result when available, and
fall back to an empty or safe display value for invalid dates such as 2026-02-30
so formatting cannot normalize them into another calendar day.
In `@apps/companion/lib/audit-format.ts`:
- Line 68: Update formatDue to perform all calendar-day comparisons and
neutral-date cutoff calculations in prefs.timeZone, reusing the timezone-aware
boundary logic from formatDate rather than device-local date math. Preserve the
existing today, tomorrow, overdue, and neutral labels while making them
consistent across UTC and non-UTC environments, and add boundary coverage for
both timezone cases.
In `@apps/webapp/app/modules/api/mobile-auth.server.test.ts`:
- Around line 302-312: Reset the module-scoped database spies before each test
by adding a beforeEach setup and clearing the relevant mocks. In the test
asserting requireMobileAuth, update the findUnique call inspection to use the
most recent recorded call rather than mock.calls[0], while preserving the
existing select regression checks.
In `@packages/datetime/package.json`:
- Around line 13-16: Update the TypeScript devDependency in apps/companion from
~5.9.3 to ^6.0.2, matching the workspace packages’ version used by
packages/datetime and the other referenced packages.
In `@packages/datetime/src/index.ts`:
- Around line 303-322: Update resolveFormatPrefs in
packages/datetime/src/index.ts to narrow dateFormat, timeFormat, and
weekStartsOn against their known member sets before returning them, and select
the first valid timezone candidate from userPrefs, detected, or the hardcoded
default before applying the fallback. In apps/companion/lib/api/types.ts lines
39-42, rely on resolveFormatPrefs’ runtime narrowing rather than the declared
union; leave the DATE_ORDER fallback in packages/datetime/src/index.ts line 650
as defense in depth.
- Around line 303-322: Update the timezone selection around candidateTimeZone to
choose the first valid value from userPrefs?.timeZone, detected?.timeZone, and
HARDCODED_DEFAULT_PREFS.timeZone, rather than using nullish coalescing alone.
Reuse isValidTimeZone for each candidate in priority order, then return
candidateTimeZone directly in the result while preserving the existing default
behavior.
- Around line 521-526: Update the wantTime computation in the relevant formatter
to treat opts.includeTime === true as sufficient on its own, removing the
dateStyle exclusion so includeTime appends time when used with a dateStyle.
Preserve the existing onlyTime, timeStyle, hour, and minute conditions.
- Around line 577-583: Update the date-formatting branch for “MM_DD_YYYY” and
“MMM_DD_YYYY” so it returns only rendered.year when the filtered monthDay value
is empty, avoiding a leading comma for year-only requests. Preserve the existing
comma-separated output when monthDay contains a value.
- Around line 684-698: Update the H12 formatting branch in the wantTime logic to
honor n.hourStyle by applying pad2 to the hour when it is "2-digit", while
preserving the Intl-derived hour for other styles. Keep the existing minute and
day-period formatting unchanged so H12 produces consistent zero-padding with the
H24 branch.
---
Nitpick comments:
In `@apps/webapp/app/modules/api/mobile-auth.server.test.ts`:
- Around line 288-301: Replace the hardcoded dbRow object definition with a call
to an existing user-row factory from the webapp test helpers, passing field
overrides for only the four preference fields (dateFormat, timeFormat,
weekStart, timeZone) while relying on the factory to provide the base user data
with id, email, firstName, lastName, profilePicture, onboarded, deletedAt, and
lastMobileActiveAt. Preserve the test's behavior by ensuring the factory returns
the same data shape when called with these overrides.
In `@packages/datetime/src/index.ts`:
- Around line 599-682: Add a focused, table-driven unit-test suite for
formatDate covering all five DateFormatPreference values and common option
shapes, including date-only strings, localeOnly, name-month preferences,
additive includeWeekday, preset folding, and both 12-hour and 24-hour clocks.
Use stable timezone/locale inputs and assert the promised formatted output
without changing production behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b031f735-9b0e-41ad-aeb4-468d808c8c69
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (26)
apps/companion/app/(tabs)/assets/[id].tsxapps/companion/app/(tabs)/assets/kits/[id].tsxapps/companion/app/(tabs)/audits/[id].tsxapps/companion/app/(tabs)/audits/index.tsxapps/companion/app/(tabs)/bookings/[id].tsxapps/companion/app/(tabs)/bookings/edit.tsxapps/companion/app/(tabs)/bookings/index.tsxapps/companion/app/(tabs)/bookings/new.tsxapps/companion/app/(tabs)/home.tsxapps/companion/components/asset-detail/custom-fields-section.tsxapps/companion/components/asset-detail/notes-section.tsxapps/companion/components/asset-edit/custom-field-input.tsxapps/companion/lib/api/types.tsapps/companion/lib/audit-format.tsapps/companion/lib/constants.tsapps/companion/lib/org-context.tsxapps/companion/lib/use-date-formatter.tsapps/companion/package.jsonapps/webapp/app/modules/api/mobile-auth.server.test.tsapps/webapp/app/modules/api/mobile-auth.server.tsapps/webapp/app/utils/date-format.tsapps/webapp/package.jsonapps/webapp/vite.config.tspackages/datetime/package.jsonpackages/datetime/src/index.tspackages/datetime/tsconfig.json
💤 Files with no reviewable changes (1)
- apps/companion/lib/constants.ts
Follow-up to the shared @shelf/datetime work, resolving reviewer findings: - Booking new/edit picker labels now render DEVICE-local (`localeOnly`) to match the native picker + `toLocalWire` submission, instead of the preferred timezone, while still honoring the user's date/time format. (CodeRabbit) - Audit "Due today/tomorrow" relative labels compute the calendar-day delta in `prefs.timeZone` via a new tested `calendarDayIndex` helper, so they agree with the timezone-formatted absolute date. (Codex + CodeRabbit) - `resolveFormatPrefs` now narrows every field against its known member set and picks the first VALID timezone candidate, so a corrupted/untrusted stored value degrades to the device hint instead of passing through (or falling to UTC). This matters now that the companion feeds unvalidated JSON into it. (CodeRabbit) - Home booking card keeps its compact month/day range (no year), still tz-aware. (Copilot) - Custom-field DATE display shares one validated date with the picker, so an invalid stored value renders "Select date..." rather than an overflowed date. (CodeRabbit) - Formatter hardening: `includeTime` is additive with `dateStyle`; a year-only month-name request no longer emits a leading comma; the H12 clock honors `hour: "2-digit"` padding like H24. (CodeRabbit) - mobile-auth test resets the shared spy and reads the latest call. (CodeRabbit) Adds 9 regression tests to date-format.test.ts (all green).
Contributor
There was a problem hiding this comment.
🟡 Not ready to approve
The companion DATE custom-field input still treats invalid stored values as “selected” for accessibility/styling, producing inconsistent UI and an incorrect accessibility label.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (2)
apps/companion/components/asset-edit/custom-field-input.tsx:452
- When
displayValueis empty (invalid stored date), the text still usespickerSelectedTextstyling because it’s keyed offvalue. This makes the placeholder render with “selected” styling. Key the styling offdisplayValueso invalid values look like the empty state.
<Text
style={value ? styles.pickerSelectedText : styles.pickerPlaceholder}
>
{displayValue || "Select date..."}
</Text>
apps/companion/components/asset-edit/custom-field-input.tsx:444
- The button’s
accessibilityLabeltreats any non-emptyvalueas valid, butdisplayValueis intentionally empty when the storedYYYY-MM-DDis invalid. This can produce an a11y label like"<label>: , tap to change"for corrupted values; it should key offdisplayValueinstead.
This issue also appears on line 448 of the same file.
accessibilityLabel={
value
? `${accessibilityLabel}: ${displayValue}, tap to change`
: `${accessibilityLabel}, tap to choose`
}
- Files reviewed: 27/28 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/companion/components/asset-edit/custom-field-input.tsx`:
- Line 408: Update the date display and selected-state logic in the custom field
input to use the validated parsed date consistently: base displayValue and the
related selected text styling and “tap to change” announcement on parsedDate !==
null rather than the raw value, while preserving empty output and unselected
behavior for invalid dates.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6f0bfd55-3284-4cdd-a06d-d0aeb3226764
📒 Files selected for processing (8)
apps/companion/app/(tabs)/bookings/edit.tsxapps/companion/app/(tabs)/bookings/new.tsxapps/companion/app/(tabs)/home.tsxapps/companion/components/asset-edit/custom-field-input.tsxapps/companion/lib/audit-format.tsapps/webapp/app/modules/api/mobile-auth.server.test.tsapps/webapp/app/utils/date-format.test.tspackages/datetime/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/companion/app/(tabs)/home.tsx
- apps/companion/app/(tabs)/bookings/edit.tsx
- apps/webapp/app/modules/api/mobile-auth.server.test.ts
- apps/companion/app/(tabs)/bookings/new.tsx
- packages/datetime/src/index.ts
…tyling + a11y The DATE custom-field input keyed its "selected" text styling and accessibility label off the raw `value`, but `displayValue` is already validated. A present- but-invalid stored value (e.g. "2026-02-30") therefore showed selected styling and announced "tap to change" with an empty label. Key both off the validated `parsedDate` so an invalid value reads as unselected everywhere, consistent with the display text and the picker. (Copilot + CodeRabbit re-review, PR #2798.)
Contributor
There was a problem hiding this comment.
🟡 Not ready to approve
A new JSDoc link in @shelf/datetime references a helper that is not exported/defined by the package, so the documentation is currently incorrect/broken.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (1)
packages/datetime/src/index.ts:82
- JSDoc references
detectFormatPrefsForPersistence, but that helper is not exported/defined in@shelf/datetime(it lives in the webapp’s~/utils/client-hints.tsx). As-is, the link is broken and it reads like the function should exist in this package.
/**
* Nullable for PERSISTENCE. The timezone comes from the `CH-time-zone` COOKIE,
* which is absent on a user's first authenticated request (and on
* server-established sessions like SSO/OAuth callbacks that never render
* `ClientHintCheck` first). Persisting the "UTC" fallback then is
* indistinguishable from a real UTC and permanently blocks the lazy backfill
* from ever writing the true zone — so persistence paths
* ({@link detectFormatPrefsForPersistence}) set this to null when the cookie is
* absent, leaving the column null for a later retry. The read path
* ({@link detectFormatPrefsFromHints}) always yields a concrete zone.
- Files reviewed: 27/28 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
`DetectedFormatPrefs.timeZone` linked `{@link detectFormatPrefsForPersistence}`,
which lives in the webapp's client-hints, not in @shelf/datetime — so the link
dangled once the formatter moved into the package. Reference it in prose instead;
the in-package `{@link detectFormatPrefsFromHints}` stays a link. (Copilot
re-review, PR #2798.)
Contributor
There was a problem hiding this comment.
🟡 Not ready to approve
calendarDayIndex can currently throw on invalid timeZone/invalid dates and should be hardened consistently with the rest of the shared formatter to avoid crashing callers.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (1)
packages/datetime/src/index.ts:180
calendarDayIndexconstructsIntl.DateTimeFormatdirectly, so an invalidtimeZone(or an invalid/NaN Date) will throw and can crash callers. Since this package is explicitly hardened elsewhere against bad timezones (and is used on the companion against untrusted JSON), this helper should also degrade safely (e.g., fall back to UTC for invalid zones and returnNaNfor invalid dates).
*/
export function calendarDayIndex(
value: string | Date,
timeZone: string
): number {
const date = value instanceof Date ? value : new Date(value);
const parts = new Intl.DateTimeFormat("en-US", {
timeZone,
year: "numeric",
month: "2-digit",
day: "2-digit",
}).formatToParts(date);
const get = (t: string) => Number(parts.find((p) => p.type === t)?.value);
return Math.floor(
Date.UTC(get("year"), get("month") - 1, get("day")) / 86_400_000
);
- Files reviewed: 27/28 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Contributor
There was a problem hiding this comment.
🟡 Not ready to approve
OrgContext currently passes resolvedOptions().timeZone through without a fallback/guard, which can yield an invalid/undefined timezone flowing into resolved prefs in environments where Intl timezone data is missing or partial.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (1)
apps/companion/lib/org-context.tsx:160
Intl.DateTimeFormat().resolvedOptions().timeZonecan be missing/unsupported (there are other call sites in the companion that already fall back to "UTC"). Here we passtimeZonestraight through toresolveFormatPrefs, which can result inundefinedbeing treated as a valid timezone and then flowing into formatter prefs (or throwing in environments where Intl is partially available). Add a safe fallback before building theFormatHints.
const formatPrefs = useMemo<ResolvedFormatPrefs>(() => {
const { locale, timeZone } = Intl.DateTimeFormat().resolvedOptions();
- Files reviewed: 27/28 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
…tl data OrgContext passed `Intl.DateTimeFormat().resolvedOptions()` straight into the format-pref hints. resolveFormatPrefs already validates the zone downstream, but guard at the source too: coalesce an undefined/blank locale or timeZone to en-US / UTC and wrap the call in try/catch, so an environment with missing or partial Intl timezone data degrades cleanly instead of feeding an undefined hint through (or throwing out of render). (Copilot re-review, PR #2798.)
Contributor
There was a problem hiding this comment.
🟢 Ready to approve
The formatter extraction is consistent with existing workspace patterns, the mobile /me contract is guarded by a targeted unit test, and companion formatting changes are centralized via context/hooks with safe fallbacks.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
- Files reviewed: 27/28 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
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 & why
The companion (Expo/React Native) app formatted every date/time device-local,
ignoring the user's Shelf format preferences that the webapp respects (shipped in
#2654 / #2791). This makes the companion honor those same preferences — date
format, time format (12/24h), and timezone — by sharing one formatter with the
webapp so the two can never drift.
Rather than re-porting the formatter to native, the webapp's pure formatter is
extracted into a new workspace package
@shelf/datetimethat both apps consume.It's pure JS (uses only
Intl, which Hermes ships), so the companion side ships viaEAS OTA — no App Store submission.
The three commits
refactor(datetime)— new@shelf/datetimepackage (the webapp's formattercore moved verbatim; the two webapp-coupled type imports replaced by local
string-union enums mirroring the Prisma enums, and a dependency-free
FormatHintsin place of
ClientHint).apps/webapp/app/utils/date-format.tsbecomes a thinexport *re-export, so all ~54 webapp import sites are unchanged. Zero behaviorchange — the webapp's 57 formatter tests pass against the re-exported package.
Metro transpiles the TS source directly (verified via an export build); webapp
consumes it through
ssr.noExternal, same as@shelf/database.feat(mobile-api)— add the four preference columns (dateFormat,timeFormat,weekStart,timeZone) to therequireMobileAuthselect so/api/mobile/mereturns them. Additive and backward-compatible; a new testasserts the columns are both selected and returned (internal-only fields stay
stripped).
feat(companion)— resolve the raw prefs once inOrgContextwith adevice-hint fallback (
Intl.DateTimeFormat().resolvedOptions()), exposeuseDateFormatter()/useFormatPrefs()hooks, and swap 12 display surfaces(bookings, audits, assets, kits, notes, custom fields) onto the bound formatter.
Intended behavior changes
the device's. A user set to
YYYY_MM_DD/H24/Asia/Tokyosees exactly thateverywhere.
DATE) render with no timezone shift(the day is preserved).
/api/mobile/meloads (or against a pre-prefs server), formatting falls backto device-local rather than an arbitrary US/UTC default.
Deliberately untouched: device-timezone capture for API submission
(
getTimeZone/toLocalWire/timeZonepayloads), the relative bookingcountdown, and the native date-picker widget's internal value. Only visible display
strings change.
Verification
tscclean · 57 formatter tests green · 10 mobile-auth tests green · lint cleantscclean · ESLint 0 new issues ·react-doctor0 errorstoLocale*/ device-local formatters in companion display pathsRollout
/me) ships with a normal deploy.changes, no store submission.
Testing notes
On-device iOS smoke recommended before/after merge: with a user set to
YYYY_MM_DD/H24/Asia/Tokyo, eyeball a booking's from/to, an audit due date,an asset created/updated date, a custom-field DATE, and a picker label — plus a
booking near a UTC-midnight boundary to confirm the timezone flip.
Summary by CodeRabbit
New Features
Bug Fixes