Skip to content

feat!: convert date to iso instead of number #21

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

Merged
merged 1 commit into from
Apr 25, 2025
Merged

Conversation

rgomezcasas
Copy link
Member

Since it's a more standard approach

Since it's a more standard approach
@rgomezcasas rgomezcasas self-assigned this Feb 12, 2025
@TheAlbertDev
Copy link

Since it's a more standard approach

Hi @rgomezcasas ,
A question from complete ignorance: why isn't Date used directly as a primitive instead of number/string? And in cases where number/string is used as a primitive, how are value objects that use Date instantiated, since they require a Date in the constructor? (In the toPrimitives/fromPrimitives methods, I could do new Date(number), but I don't see the benefit of adding this extra code .)

On the other hand, I saw there was a breaking change to switch dates to numbers, and now this PR proposes another breaking change to switch to strings. Will you be merging it soon? Just asking to know whether I should wait or move forward while keeping an eye on it.

Merci por vuestro trabajo!

@rgomezcasas
Copy link
Member Author

rgomezcasas commented Apr 25, 2025

Thanks, @TheAlbertDev!

Yep, merging it today. 🙌

The reason is that Date is an object, not a scalar value, so you can't have it serialized in JSON or Avro. The only way is to transform it to a Unix timestamp (number) or some string standard (like the ISO one).

We started with the numeric approach, but we realized that we prefer the ISO one since it's easier for humans to read.

@rgomezcasas rgomezcasas merged commit 7447dd2 into main Apr 25, 2025
2 checks passed
@rgomezcasas rgomezcasas deleted the date-string-iso branch April 25, 2025 09:44
@github-actions github-actions bot mentioned this pull request Apr 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants