fix: skip native Date fast path when local timezone is overridden via setLocalTimeZone#9678
Conversation
|
Thank you for the PR! I think the code changes and tests look good, but it seems @sonsu-lee has also submitted a PR with similar changes. I'll leave it to you two to decide how yall want to proceed from here since these came in at roughly the same time haha. As an aside, I'd would like the queries tests from #9680 to also make it in just for completeness. |
|
Thanks for the review @LFDanLu! I've pushed updates addressing your feedback:
Regarding @sonsu-lee's PR #9680 — it looks like we both arrived at the same solution independently, which is great validation of the approach! I submitted this one ~30 minutes earlier, but happy to defer to whatever you and the team prefer. If there's anything from #9680 you'd like incorporated here that I'm still missing, just let me know. |
|
Thanks for the review @LFDanLu. Funny that @apoorvdarshan and I ended up with basically the same fix. #9678 was first and already has your feedback addressed, so I'll close this one. Nice work @apoorvdarshan! |
|
Thanks @sonsu-lee, appreciate the kind words and for closing yours in favor of this one! Great minds think alike 😄 @LFDanLu all feedback has been addressed and CI is green now — let me know if there's anything else needed. |
… setLocalTimeZone When setLocalTimeZone is called with a timezone different from the browser's, the fast paths in getTimeZoneOffset and toAbsolute incorrectly use native Date (which always reflects the browser timezone) instead of the Intl-based path. This causes incorrect offset calculations for the overridden timezone. Skip the native Date fast paths when setLocalTimeZone has been called, falling back to the correct Intl.DateTimeFormat-based computation. Fixes adobe#9669
…-0 vs 0 issue in UTC CI
…es tests - Fix alphabetical import sorting in conversion.ts - Export isLocalTimeZoneOverridden from public index - Add isLocalTimeZoneOverridden unit tests in queries.test.js as requested
4f342f2 to
59af4e2
Compare
LFDanLu
left a comment
There was a problem hiding this comment.
Looks good to me, thank you @apoorvdarshan and @sonsu-lee both! I'll see about getting another pair of eyes on this
Summary
setLocalTimeZonecauses incorrect timezone conversion when the overridden timezone differs from the browser's timezoneDatefast paths ingetTimeZoneOffsetandtoAbsolutewhensetLocalTimeZonehas been called, falling back to the correctIntl.DateTimeFormat-based computationlocalTimeZoneOverrideflag inqueries.tsthat tracks whethersetLocalTimeZonehas been called (and is cleared byresetLocalTimeZone)Test plan
conversion.test.jscovering:getTimeZoneOffsetreturns correct offset when local timezone is overriddentoAbsolutereturns correct result when local timezone is overriddenresetLocalTimeZonerestores the native Date fast path@internationalized/datecontinue to pass