Fixed implementation of useAutoUpdateTimezone to ignore delegate and do the check on focus event#84738
Fixed implementation of useAutoUpdateTimezone to ignore delegate and do the check on focus event#84738shubham1206agra wants to merge 8 commits intoExpensify:mainfrom
Conversation
…do the check on focus event
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@ahmedGaber93 Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1a98aa6147
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
JmillsExpensify
left a comment
There was a problem hiding this comment.
No Produce review on this one
mountiny
left a comment
There was a problem hiding this comment.
Thanks for the fix — the root cause analysis is correct and moving away from the reactive useEffect on Onyx-synced timezone.selected does break the infinite loop.
A few concerns noted inline:
- No timezone check on initial mount — the most impactful one. The timezone won't be reconciled on first app load, only on subsequent focus events. This can be fixed without re-introducing the loop by adding a one-shot mount check.
- Equality check compares formatted vs. unformatted timezone values —
formatToSupportedTimezonecan remap deprecated names, so the guard may miss semantic equality. - Existing tests silently broken — 3 of 4 tests pass trivially because they never trigger the focus event and the hook no longer runs on mount.
See inline comments for details.
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-04-14.at.10.55.23.PM.movAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / SafariScreen.Recording.2026-04-14.at.10.27.27.PM.movScreen.Recording.2026-04-14.at.10.55.23.PM.mov |
|
@shubham1206agra I left two comments above: #84738 (comment) and #84738 (comment), please let me know what you think, or when this is ready again for review. |
|
@shubham1206agra any updates here? |
|
@shubham1206agra any updates here? Bumped on slack. |
|
@ahmedGaber93 PR is now ready for review. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f04842224
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@shubham1206agra I still encountered this issue.
Screen.Recording.2026-04-14.at.10.03.14.PM.mov |
|
Another bug with the same root cause like the above issue. New accounts show incorrect message time until the browser is refocused.
Screen.Recording.2026-04-15.at.12.13.55.AM.mov |
|
@MelvinBot Do you have any good fix that does not break React compiler? |
|
Yes — the issue is that const useAutoUpdateTimezone = () => {
const [isActingAsDelegate] = useOnyx(ONYXKEYS.ACCOUNT, {selector: isActingAsDelegateSelector});
const currentUserPersonalDetails = useCurrentUserPersonalDetails();
const timezone = currentUserPersonalDetails?.timezone ?? {};
const isAutomatic = timezone?.automatic;
const updateTimezone = useEffectEvent(() => {
if (isActingAsDelegate) {
return;
}
const currentTimezone = Intl.DateTimeFormat().resolvedOptions().timeZone as SelectedTimezone;
const hasValidCurrentTimezone = typeof currentTimezone === 'string' && currentTimezone.trim().length > 0;
const proposedTimezone = DateUtils.formatToSupportedTimezone({
automatic: true,
selected: currentTimezone,
});
if (hasValidCurrentTimezone && isAutomatic && timezone?.selected !== proposedTimezone.selected) {
updateAutomaticTimezone(proposedTimezone, currentUserPersonalDetails.accountID);
}
});
useAppFocusEvent(updateTimezone);
useEffect(() => {
updateTimezone();
}, [isAutomatic]);
};Why this works:
|
|
@ahmedGaber93 It should be fixed now |
Explanation of Change
Fixed Issues
$ #83765
Tests
Offline tests
Same as Tests
QA Steps
Same as Tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari