Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nit: Allow date formatting to be configured by client? #284

Closed
cprince-foreflight opened this issue Dec 14, 2023 · 4 comments
Closed

Nit: Allow date formatting to be configured by client? #284

cprince-foreflight opened this issue Dec 14, 2023 · 4 comments
Assignees
Labels

Comments

@cprince-foreflight
Copy link
Contributor

Is your feature request related to a problem? Please describe.
This is more of a nit than anything else, but it would be nice. Our app currently does date formatting using the ISO8601DateFormatter() configured with formatter.formatOptions.insert(.withFractionalSeconds). In this way, we get a string date with a format like:
"2023-10-04T21:41:12.000Z"

The default date formatter in Segment currently formats dates slightly differently. For example:
"2023-12-13T23:26:37.091+0000"

Describe the solution you'd like
Could date formatting be configurable by the calling client? E.g., through supplying a DateEncodingStrategy for the JSONEncoder and DateDecodingStrategy for the JSONDecoder?

@bsneed
Copy link
Contributor

bsneed commented Dec 14, 2023

Hi @cprince-foreflight, yeah, definitely, it's a possibility! Though, we don't want to go that route probably as we need consistent dates across the board. Last I looked, we were getting the 000Z version you show but it has been a minute since I checked. Segment internally needs 8601 dates universally, so the options provided by JSONDecoder are more than would be desirable. If you have an idea and PR, I can probably get it merged pretty quick if it follows the 000Z format. Otherwise, it'll likely be post holidays.

@cprince-foreflight
Copy link
Contributor Author

cprince-foreflight commented Dec 14, 2023

Looking at this more this morning, I'm realizing that the date formatting difference I'm finding has to do with using the Codable interface versus the non-Codable interface in Segment 1.5.0.

Codable interface
Default date formatting:
"2023-12-13T23:26:37.091+0000"

non-Codable interface
Default date formatting:
"2023-12-14T16:03:14.300Z"

Looking for the underlying difference in the code now.

@cprince-foreflight
Copy link
Contributor Author

cprince-foreflight commented Dec 14, 2023

I see it now. Different date formatters are being used:

Codable
Code reference

extension DateFormatter {
    static let iso8601: DateFormatter = {
        let formatter = DateFormatter()
        formatter.dateFormat = "yyyy-MM-dd'T'HH:mm:ss.SSSZ"
        formatter.calendar = Calendar(identifier: .iso8601)
        formatter.timeZone = TimeZone(secondsFromGMT: 0)
        formatter.locale = Locale(identifier: "en_US_POSIX")
        return formatter
    }()
}

Non-Codable
Code reference

    enum SegmentISO8601DateFormatter {
    static let shared: ISO8601DateFormatter = {
        let formatter = ISO8601DateFormatter()
        formatter.formatOptions.update(with: .withFractionalSeconds)
        return formatter
    }()

Looking into a PR to fix now.

@bsneed
Copy link
Contributor

bsneed commented Dec 14, 2023

Thanks @cprince-foreflight ! Nice work, very clean! Will do a release once the gh actions finish up on main.

@bsneed bsneed closed this as completed Dec 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants