Repository navigation
fix(swift): keep the written wall-clock time in formatDate and accept timestamps without an offset - #3380
Conversation
FormatDateFunction formatted with a DateFormatter that had no time zone, so the result depended on the host: 2026-09-04T23:30:00-05:00 formatted as 2026-09-05 on a Warsaw device. It also returned "" for a timestamp without an offset, and rolled 2026-02-30T10:00:00Z over into March. Parse the timestamp into its instant and the offset it was written with, and format in that offset, as web_core, Dart and Python v1.0 do. A timestamp without an offset, or a date, is read as UTC. Fields that do not exist return "". ISO still emits the UTC instant, and numeric epochs now format in UTC. The tests now assert literal strings instead of host-formatted ones, and functions.yaml gains three shared cases. The offset case is pinned to v1.0 because Python's v0.9 catalog converts to UTC first. Fixes a2ui-project#3379
There was a problem hiding this comment.
Code Review
This pull request refactors the FormatDateFunction in Swift to parse and format dates using a new Timestamp struct, ensuring that timestamps are formatted in their written offset rather than depending on the host time zone. It also adds corresponding conformance and unit tests. The review feedback suggests improving robustness by ensuring parsed Double values are finite before initializing Timestamp, and setting the Calendar locale to en_US_POSIX to guarantee deterministic calendar calculations independent of host regional settings.
|
Hi @karolkulesza, thank you for raising the PR. We have checked the PR and confirmed that it fixes #3379. Before merging, please address the 2 bot review comments in |
- Return "" for a numeric epoch that is not finite (NaN, Infinity, also as the strings "NaN", "inf", "-Infinity") or lies outside the +-100,000,000 days a JavaScript Date can hold, checked before the Timestamp is built. - Give the calendar that validates ISO 8601 fields the en_US_POSIX locale, so it never depends on the host's regional settings. - Accept a lowercase t between date and time, as RFC 3339 allows and as web_core and Python already do. A lowercase z was already accepted.
|
@Varun-S10 Updated in 1e138a9:
I kept the lowercase |
|
Hi @karolkulesza, thank you for addressing all the review comments, adding the finite epoch and locale checks, and supporting lowercase t in timestamps. We have verified the updated changes and new test cases, and everything works now. I will escalate this PR to the team for review. Thank you for your patience and understanding. |
Parse ISO 8601 timestamps with a Swift Regex literal and wholeMatch(of:), so a trailing newline no longer matches, and read the fields through named captures. Truncate the fraction to whole milliseconds, as web_core, Python and Dart do, so 23:59:59.9999 no longer rounds into the next day. Treat epoch numbers whose magnitude exceeds 10^10 as milliseconds, so pre-1970 millisecond timestamps are read correctly. Move the JSONValue dispatch into a failable Timestamp initializer so evaluate unwraps its inputs in one guard.
|
Ah, it looks like one of the new tests in the function conformance tests fails on Python. If you want to update the Python renderer to pass, we'll need a Python reviewer to take a look at this PR. Otherwise, you could remove the broken test from this PR and open an issue to add the test and get Python working. Up to you whether you want to take that task on leave it for someone else :) |
|
I thought I left a top-level comment about this, but it seems like I did not: We'll need someone a bit more familiar with Python to sign off on this one as well. Perhaps @josemontesp or @gspencergoog? |
|
@pinieb thanks for flagging it. The Python failure wasn't in Python's Locally, This PR changes no Python code, only the shared YAML case and Swift. Python would only need a closer look for the v0.9 behaviour I mentioned in #3379, and that's why the case is pinned to v1.0. Happy to have a Python reviewer confirm anyway. |
pinieb
left a comment
There was a problem hiding this comment.
Looks good, thank you! 🎆
Description
Swift's
formatDateformatted in the host time zone, so the same payload rendered differentlyfrom device to device.
2026-09-04T23:30:00-05:00withyyyy-MM-dd HH:mmgave2026-09-05 06:30on a Warsaw device. web_core, Dart and Python v1.0 give2026-09-04 23:30.A timestamp without an offset returned
"", and2026-02-30T10:00:00Zwas rolled over intoMarch.
Changes:
FormatDateFunctionparses an ISO 8601 timestamp into its instant and written offset (Z,+hh,+hhmm,+hh:mm;Tor space; optional seconds and fraction), and formats thepattern in that offset. Without an offset, the timestamp or date is read as UTC. Fields that
do not exist give
"".ISOstill emits the UTC instant. Numeric epochs now format in UTCinstead of the host zone.
FormatDateFunctionTestsasserts literal strings, so it no longer derives expectations fromthe host time zone. It no longer uses
@testable import, and it adds cases for each offsetform, no offset, date-only and impossible fields.
conformance/core/functions.yamladds three cases. The offset case is pinned toprotocolVersion: "v1.0"because Python's v0.9 catalog converts to UTC before formatting(
2026-09-05 04:30). That case passes in the Dart and Python v1.0 runners; for web_core I checkedparseTimestampon the three inputs but did not run its yarn suite.Verification:
swift test: 582 tests. The only failures are the 2 known [Swift] BasicCatalog formatNumber, formatCurrency, and pluralize use Locale.current instead of defaulting to en-US #3144 locale issues inevaluateFunctionConformanceon an en_PL host.Europe/Warsaw and Asia/Tokyo. Without the fix, 11 of the 17 FormatDate tests and both
unversioned YAML cases fail on a Warsaw host.
dart test test/conformancepasses, and Pythontests/test_conformance.pypasses (567 passed,17 skipped), both including the new cases.
Fixes #3379
Pre-launch Checklist
One time:
For this PR: