Skip to content

Commit 69e42fc

Browse files
committed
fix: Hermes wire-date parsing, formatter fallback ladder, honest week-start tests
1 parent 302a712 commit 69e42fc

20 files changed

Lines changed: 236 additions & 266 deletions

File tree

src/components/PerDiemEReceipt.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ function computeDefaultPerDiemExpenseRates(customUnit: TransactionCustomUnit, cu
4242
return subRateComments.join(', ');
4343
}
4444

45-
/** Last three comma parts of the merchant are the date range, the rest is the location. See `computePerDiemExpenseMerchant` for why that holds. */
45+
/** Last three comma parts are the date range, the rest is the location. `computePerDiemExpenseMerchant` pins that shape. */
4646
function getPerDiemDestination(merchant: string) {
4747
const merchantParts = merchant.split(', ');
4848
if (merchantParts.length < 3) {

src/components/TimePicker/TimePicker.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ function TimePicker({defaultValue = '', onSubmit, onInputChange = () => {}, shou
129129
const value = DateUtils.extractTime12Hour(defaultValue, showFullFormat);
130130
const canUseTouchScreen = canUseTouchScreenDeviceCapabilities();
131131

132-
// Parsed once on mount. An unparsable value seeds the same 12:00 PM placeholder an empty one gets, so it is not visually distinct from a deliberate noon.
132+
// Parsed once on mount. An unparsable value seeds the same 12:00 PM placeholder an empty one gets.
133133
const [initialTime] = useState(() => DateUtils.get12HourTimeObjectFromDate(value, showFullFormat) ?? EMPTY_TWELVE_HOUR_TIME);
134134
const [hours, setHours] = useState(initialTime.hour);
135135
const [minutes, setMinutes] = useState(initialTime.minute);

src/hooks/useNow.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,9 @@ import {getSnapshot, subscribe} from '@libs/NowStore';
22

33
import {useSyncExternalStore} from 'react';
44

5-
/** SSR is unsupported. Server module-load time would diverge from the client's fresh `new Date()` at hydration; throwing surfaces the problem loudly. */
6-
function getServerSnapshot(): Date {
7-
throw new Error('[NowStore] useNow is not SSR-safe; server and client snapshots would diverge on hydration.');
8-
}
9-
5+
/** No `getServerSnapshot`: React also calls it on the client during hydration, so a throwing one would not stay confined to SSR. */
106
function useNow(): Date {
11-
return useSyncExternalStore(subscribe, getSnapshot, getServerSnapshot);
7+
return useSyncExternalStore(subscribe, getSnapshot);
128
}
139

1410
export default useNow;

src/languages/IntlStore.ts

Lines changed: 66 additions & 49 deletions
Large diffs are not rendered by default.

src/languages/__mocks__/IntlStore.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ class IntlStore {
5050

5151
private static listeners = new Set<() => void>();
5252

53-
// One cached snapshot so repeated `useSyncExternalStore` reads return the same reference and never trigger an infinite render loop. Replaced (not mutated) on locale change so subscribers see a fresh identity.
53+
// One cached snapshot, so repeated `useSyncExternalStore` reads return the same reference. Replaced, never mutated.
5454
private static snapshot: {locale: Locale; loaded: boolean; hasAnyTranslations: boolean} = {
5555
locale: IntlStore.currentLocale,
5656
loaded: IntlStore.localeCache.has(IntlStore.currentLocale),
@@ -62,7 +62,7 @@ class IntlStore {
6262
}
6363

6464
static load(locale?: Locale): Promise<void> {
65-
// Real behaviour: mutate currentLocale, replace the snapshot, notify subscribers. Otherwise tests exercising a locale switch see no effect and coverage silently fails-open.
65+
// Real behaviour, otherwise a suite exercising a locale switch sees no effect and passes for the wrong reason.
6666
if (locale && IntlStore.localeCache.has(locale)) {
6767
IntlStore.currentLocale = locale;
6868
IntlStore.snapshot = {locale, loaded: true, hasAnyTranslations: true};

src/libs/DateUtils.ts

Lines changed: 46 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -58,12 +58,6 @@ type MachineDateFormat =
5858

5959
const TIMEZONE_UPDATE_THROTTLE_MINUTES = 5;
6060

61-
/**
62-
* Offset shapes `new Date()` accepts at the end of an ISO string: `±HHMM` and `±HH:MM`. V8 and Hermes reject bare `±HH`,
63-
* so this pattern requires the minutes component. Used by `isAlreadyZoned` to decide whether to append `Z`.
64-
*/
65-
const ISO_OFFSET_PATTERN = /[+-]\d{2}:?\d{2}$/;
66-
6761
type IntlFormatKey = keyof typeof CONST.DATE.INTL_FORMATS;
6862

6963
/** Narrows an arbitrary timezone string to one our backward-mapping table knows about. */
@@ -77,43 +71,39 @@ function isWeekDay(value: number): value is WeekDay {
7771
}
7872

7973
/**
80-
* LRU-bounded (Intl.DateTimeFormat holds 10-50 KB ICU state per entry). Returns `null` and caches that on any
81-
* construction failure (bad locale on old engines, missing IANA name) so subsequent calls short-circuit instead of
82-
* re-throwing every render. Retries with the backward-mapped IANA name on older iOS/macOS (e.g. `Europe/Kyiv`).
83-
* Key space is ~19 presets x the active locale x a few timezones, and all three args are primitives, so a small
84-
* shallow-equality cache avoids a deep comparison on every formatted cell in a long transaction list.
74+
* LRU-bounded (Intl.DateTimeFormat holds 10-50 KB ICU state per entry). Returns `null` and caches that when no candidate
75+
* constructs, so subsequent calls short-circuit instead of re-throwing every render. The key space is ~19 presets x the
76+
* active locale x however many timezones are on screen, which a long LHN of `ParticipantLocalTime` rows can push into the
77+
* hundreds, so the bound has to sit above that to cache rather than thrash. All three args are primitives, so shallow
78+
* equality avoids a deep comparison on every formatted cell in a long transaction list.
8579
*/
8680
const getIntlDateTimeFormat = memoize(
8781
(locale: Locale, formatKey: IntlFormatKey, timeZone?: string): Intl.DateTimeFormat | null => {
8882
const preset = CONST.DATE.INTL_FORMATS[formatKey];
89-
const options = timeZone ? {...preset, timeZone} : preset;
90-
try {
91-
return new Intl.DateTimeFormat(locale, options);
92-
} catch (error) {
93-
const backwardTimeZone = timeZone && isKnownTimezone(timeZone) ? timezoneNewToBackwardMap[timeZone] : undefined;
94-
if (backwardTimeZone && backwardTimeZone !== timeZone) {
95-
try {
96-
return new Intl.DateTimeFormat(locale, {...preset, timeZone: backwardTimeZone});
97-
} catch (retryError) {
98-
Log.warn('[DateUtils] Intl.DateTimeFormat retry with backward timezone also failed', {locale, formatKey, timeZone, backwardTimeZone, retryError});
99-
return null;
100-
}
101-
}
102-
// Hermes implements DateTimeFormat, so the realistic failure is one bad locale tag. Retry on the default locale so dates degrade to English rather than blank.
103-
if (locale !== CONST.LOCALES.DEFAULT) {
83+
const backwardTimeZone = timeZone && isKnownTimezone(timeZone) ? timezoneNewToBackwardMap[timeZone] : undefined;
84+
// Degrade one dimension at a time, because either can be the one the engine rejects. Timezone first (older
85+
// iOS/macOS know only the backward-mapped IANA name), then locale (Hermes implements DateTimeFormat, so the
86+
// realistic remaining failure is one bad tag). Dropping the timezone is never a candidate: it would render UTC
87+
// wall-clock as if it were local, which is worse than the empty string callers already handle.
88+
const timeZoneCandidates = backwardTimeZone && backwardTimeZone !== timeZone ? [timeZone, backwardTimeZone] : [timeZone];
89+
const localeCandidates: Locale[] = locale === CONST.LOCALES.DEFAULT ? [locale] : [locale, CONST.LOCALES.DEFAULT];
90+
for (const candidateLocale of localeCandidates) {
91+
for (const candidateTimeZone of timeZoneCandidates) {
10492
try {
105-
Log.warn('[DateUtils] Intl.DateTimeFormat construction failed; retrying on the default locale', {locale, formatKey, timeZone, error});
106-
return new Intl.DateTimeFormat(CONST.LOCALES.DEFAULT, options);
107-
} catch (defaultLocaleError) {
108-
Log.warn('[DateUtils] Intl.DateTimeFormat default-locale retry also failed', {locale, formatKey, timeZone, defaultLocaleError});
109-
return null;
93+
const formatter = new Intl.DateTimeFormat(candidateLocale, candidateTimeZone ? {...preset, timeZone: candidateTimeZone} : preset);
94+
if (candidateLocale !== locale || candidateTimeZone !== timeZone) {
95+
Log.warn('[DateUtils] Intl.DateTimeFormat constructed on a fallback candidate', {locale, formatKey, timeZone, candidateLocale, candidateTimeZone});
96+
}
97+
return formatter;
98+
} catch {
99+
// Next candidate.
110100
}
111101
}
112-
Log.warn('[DateUtils] Intl.DateTimeFormat construction failed', {locale, formatKey, timeZone, error});
113-
return null;
114102
}
103+
Log.warn('[DateUtils] Intl.DateTimeFormat construction failed for every candidate', {locale, formatKey, timeZone, backwardTimeZone});
104+
return null;
115105
},
116-
{maxSize: 64, equality: 'shallow'},
106+
{maxSize: 256, equality: 'shallow'},
117107
);
118108

119109
/**
@@ -179,21 +169,20 @@ const getWeekStartsOn = memoize(
179169
} catch {
180170
// Fall through to the static map below.
181171
}
182-
// Total over the Locale union, so this is exhaustive by construction rather than a partial lookup with a default.
183-
return WEEK_STARTS_ON_BY_LOCALE[locale];
172+
// Total over the `Locale` union, but the tag reaches here from an Onyx NVP, so a malformed persisted value would
173+
// otherwise return undefined and collapse the calendar via `WEEK_DAYS[NaN]` in `getWeekEndsOn`.
174+
return WEEK_STARTS_ON_BY_LOCALE[locale] ?? CONST.WEEK_STARTS_ON;
184175
},
185176
{maxSize: 16, equality: 'shallow'},
186177
);
187178

188-
/**
189-
* Get the day of the week that the week ends on for the given locale (derived from `getWeekStartsOn` so they stay in lockstep).
190-
*/
179+
/** Derived from `getWeekStartsOn` so the two stay in lockstep. */
191180
function getWeekEndsOn(locale: Locale): WeekDay {
192181
return WEEK_DAYS[(getWeekStartsOn(locale) + 6) % 7];
193182
}
194183

195184
/**
196-
* Returns a zoned Date for the given datetime. `string` parses as ISO (with legacy `Z`-suffix fallback);
185+
* Returns a zoned Date for the given datetime. Unzoned `string` values are the DB wire format and read as UTC;
197186
* `Date`/`number` passes through; `undefined` reads `Date.now()` — only safe outside render.
198187
* `locale` is unused; kept on the signature for compat with LocaleContextProvider's wrapper.
199188
*/
@@ -204,16 +193,12 @@ function getLocalDateFromDatetime(locale: Locale, currentSelectedTimezone: strin
204193
if (datetime instanceof Date || typeof datetime === 'number') {
205194
return toZonedSafe(datetime, currentSelectedTimezone);
206195
}
207-
let parsedDatetime: Date;
208-
// Skip the `Z` on already-zoned strings — appending produces `...ZZ` / `...+05:00Z` (Invalid Date). Runs every minute in useNow consumers.
209-
const isAlreadyZoned = datetime.endsWith('Z') || ISO_OFFSET_PATTERN.test(datetime);
210-
try {
211-
parsedDatetime = new Date(isAlreadyZoned ? datetime : `${datetime}Z`);
212-
parsedDatetime.toISOString();
213-
} catch (e) {
214-
parsedDatetime = new Date(datetime);
215-
}
216-
return toZonedSafe(parsedDatetime, currentSelectedTimezone);
196+
// `toDate` reads an unzoned value as UTC, honours an embedded offset when there is one, and parses the space-separated
197+
// wire shape on every engine. Appending `Z` to that shape instead relied on a V8 leniency Hermes lacks, which left
198+
// every chat timestamp showing the current time. It only understands ISO-like input, so non-ISO strings (a
199+
// `Date.prototype.toString()` value, which an engine is required to parse back) still need the engine's own parser.
200+
const isoParsed = toDate(datetime, {timeZone: 'UTC'});
201+
return toZonedSafe(Number.isNaN(isoParsed.getTime()) ? new Date(datetime) : isoParsed, currentSelectedTimezone);
217202
}
218203

219204
function toZonedSafe(date: Date | number, timeZone: string): Date {
@@ -290,7 +275,8 @@ const fallbackToSupportedTimezone = memoize((timezoneInput: SelectedTimezone): s
290275
* Jan 20, 2019 at 5:30 PM anything over 1 year ago
291276
*/
292277
function datetimeToCalendarTime(locale: Locale, datetime: string, currentSelectedTimezone: SelectedTimezone, includeTimeZone = false, isLowercase = false): string {
293-
// Capture the mapped tz once. Passing the unmapped tz into isToday/isYesterday/toZonedTime after formatting `date` with the mapped tz would let the "today" branch and the display disagree on the reference zone if a legacy alias with a divergent offset is ever added to `timezoneNewToBackwardMap`. The map's backward IANA values are valid tz identifiers that date-fns accepts, they just aren't in the tight `SelectedTimezone` union, so the cast is safe.
278+
// Map once and reuse, so the isToday/isYesterday branches and the rendered string cannot resolve against different zones.
279+
// The backward IANA values are real tz identifiers, just outside the tighter `SelectedTimezone` union.
294280
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
295281
const mappedTimezone = fallbackToSupportedTimezone(currentSelectedTimezone) as SelectedTimezone;
296282
const date = getLocalDateFromDatetime(locale, mappedTimezone, datetime);
@@ -496,7 +482,7 @@ function getCurrentTimezone(timezone: Timezone): Required<Timezone> {
496482
const FALLBACK_MONTH_NAMES = ['January', 'February', 'March', 'April', 'May', 'June', 'July', 'August', 'September', 'October', 'November', 'December'] as const;
497483

498484
function monthNamesIn(locale: Locale): string[] {
499-
// Fixed-year UTC mid-month dates. Month name is year-independent, so UTC + mid-month sidesteps any local-tz boundary that could shift a January-1 or December-31 endpoint into the neighbouring month on extreme timezones.
485+
// Mid-month in UTC, so no timezone can shift a month-edge date into the neighbouring month.
500486
const monthsArray = Array.from({length: 12}, (_, monthIndex) => new Date(Date.UTC(2000, monthIndex, 15)));
501487
return monthsArray.map((monthDate) => Str.UCFirst(formatIntl(locale, 'LONG_MONTH', monthDate)));
502488
}
@@ -1127,8 +1113,8 @@ function getCancellationDateTimezoneLabel(venueTimezone: string): string {
11271113
}
11281114

11291115
/**
1130-
* Captures the trailing offset so this helper can do the shift itself. Wider than `ISO_OFFSET_PATTERN` (it also matches a
1131-
* bare `±HH`) because the offset is stripped before parsing, so shapes `new Date` would reject still work here.
1116+
* Captures the trailing offset so this helper can do the shift itself. Matches a bare `±HH` too, because the offset is
1117+
* stripped before parsing, so shapes `new Date` would reject still work here.
11321118
*/
11331119
const CANCELLATION_OFFSET_PATTERN = /([+-])(\d{2}):?(\d{2})?$/;
11341120

@@ -1203,7 +1189,8 @@ function formatCountdownTimer(translateParam: LocaleContextProps['translate'], h
12031189
const WIRE_YEAR_PREFIX = /^(\d{4})/;
12041190

12051191
function doesDateBelongToAPastYear(date: string): boolean {
1206-
// Extract the year from the wire string directly, so a "2023-12-31" transaction viewed on Dec 31 evening (Jan 1 UTC) still compares 2023 vs 2023 and does not spuriously suffix ", 2023" onto today's row. Falls back to UTC parsing when the input is not a leading-year string.
1192+
// Read the year off the wire string, so a Dec 31 transaction viewed that evening (already Jan 1 in UTC) is not
1193+
// suffixed with a year on what is still today's row.
12071194
const yearMatch = date.match(WIRE_YEAR_PREFIX);
12081195
const transactionYear = yearMatch ? Number(yearMatch[1]) : toUTCDate(date).getUTCFullYear();
12091196
return transactionYear !== new Date().getFullYear();
@@ -1300,8 +1287,9 @@ function toLocalDate(date: Date | string): Date {
13001287
}
13011288

13021289
/**
1303-
* Like `toLocalDate` but anchors the value to UTC midnight, so downstream UTC-zone formatters render the intended
1304-
* calendar day for viewers east of UTC.
1290+
* Like `toLocalDate` but resolves the value in UTC, so downstream UTC-zone formatters render the intended calendar day
1291+
* for viewers east of UTC. A `Date` is re-read as the calendar fields it displays locally, which is what the date-only
1292+
* callers want and what an instant-valued caller would not: pass those a string.
13051293
*/
13061294
function toUTCDate(date: Date | string): Date {
13071295
if (typeof date !== 'string') {
@@ -1315,9 +1303,7 @@ function toUTCDate(date: Date | string): Date {
13151303
return toDate(date, {timeZone: 'UTC'});
13161304
}
13171305

1318-
/**
1319-
* Converts a date to a locale-aware long date string (e.g. "March 1, 2025" in English).
1320-
*/
1306+
/** @returns March 1, 2025 (en) / 1 de marzo de 2025 (es) */
13211307
function formatToReadableString(date: Date | string, locale: Locale): string {
13221308
return formatIntl(locale, 'LONG_DATE', toLocalDate(date));
13231309
}

src/libs/Localize/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ const memoizedGetTranslatedPhrase = memoize(getTranslatedPhrase, {
113113
*/
114114
function translate<TPath extends TranslationPaths>(locale: Locale, path: TPath, ...parameters: TranslationParameters<TPath>): string {
115115
if (!IntlStore.hasLocale(locale)) {
116-
// Requested locale not loaded yet. Fall back to the currently loaded locale so callers (OnyxDerived, etc.) get a real string instead of a raw dotted path they might persist into derived state.
116+
// Requested locale not loaded yet. Fall back to the loaded one, so callers never persist a raw dotted path.
117117
const currentLocale = IntlStore.getCurrentLocale();
118118
if (currentLocale !== locale && IntlStore.hasLocale(currentLocale)) {
119119
return translate(currentLocale, path, ...parameters);

src/libs/NowStore.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,13 @@ const MS_PER_MINUTE = 60_000;
88

99
const listeners = new Set<() => void>();
1010
let timeoutId: ReturnType<typeof setTimeout> | null = null;
11-
// Seeded at module load. Refreshed by `tick` and by `subscribe`. Never advanced from `getSnapshot` so the useSyncExternalStore purity contract holds (repeated reads in one render return the same reference).
11+
// Advanced by `tick` and `subscribe`, never by `getSnapshot`, which must stay pure for `useSyncExternalStore`.
1212
let snapshot: Date = new Date();
1313
let lastMinute = Math.floor(snapshot.getTime() / MS_PER_MINUTE);
1414

1515
function advanceIfStale(): boolean {
1616
const now = new Date();
17-
// Monotonic minute index (not `getMinutes()` 0-59) so sleep/wake gaps that land on the same minute-of-hour (10:30 → 11:30) still count as changes.
17+
// Monotonic index rather than `getMinutes()`, so a sleep/wake gap landing on the same minute-of-hour still counts.
1818
const currentMinute = Math.floor(now.getTime() / MS_PER_MINUTE);
1919
if (currentMinute === lastMinute) {
2020
return false;
@@ -24,7 +24,7 @@ function advanceIfStale(): boolean {
2424
return true;
2525
}
2626

27-
/** Schedule the next tick aligned to the upcoming minute boundary, with a small safety margin so drift does not accumulate. */
27+
/** Aligned to the next minute boundary, with a small margin so drift does not accumulate. */
2828
function scheduleNextTick() {
2929
const msUntilNextMinute = MS_PER_MINUTE - (Date.now() % MS_PER_MINUTE);
3030
timeoutId = setTimeout(tick, msUntilNextMinute + 10);
@@ -44,7 +44,7 @@ function tick() {
4444
}
4545

4646
function subscribe(listener: () => void): () => void {
47-
// Refresh (and notify existing siblings) BEFORE adding the new listener, so the fresh subscriber does not receive a redundant onStoreChange in addition to React's mount-time getSnapshot comparison.
47+
// Refresh before adding the listener, so the new subscriber is not notified on top of React's own mount-time check.
4848
if (advanceIfStale()) {
4949
for (const other of listeners) {
5050
other();

0 commit comments

Comments
 (0)