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

Proposal: Always convert DateTime to UTC during serialization #1371

Open
Rexios80 opened this issue Nov 14, 2023 · 4 comments · May be fixed by #1472
Open

Proposal: Always convert DateTime to UTC during serialization #1371

Rexios80 opened this issue Nov 14, 2023 · 4 comments · May be fixed by #1472
Labels
Next breaking change release Fix that will be a breaking change – good for next breaking change release Type: enhancement

Comments

@Rexios80
Copy link

Since dates should pretty much always be stored in UTC would it be reasonable to automatically convert DateTime objects to UTC during serialization? Could this at least be an option in the generator? I know I could write my own DateTime converter but then I have to remember to use it every time I create a new DateTime field.

I see #811, but that was abandoned. I think making this a generator setting, maybe off by default to prevent issues with existing codebases, would be ideal.

@Rexios80
Copy link
Author

Rexios80 commented Jan 7, 2025

I made a custom_lint rule to enforce creating DateTime objects using the DateTime.timestamp() instead of DateTime.now() constructor, but I feel like this should still be an option

@kevmoo
Copy link
Collaborator

kevmoo commented Jan 7, 2025

I get your pitch here, certainly. The only issue is backwards compat at this point. Not sure how to move forward gracefully here. Suggestions welcome.

@Rexios80
Copy link
Author

Rexios80 commented Jan 7, 2025

Would it be acceptable to implement a breaking change to address this issue? Currently, all projects are in one of the following states:

  • Developers are aware that their timestamps are stored as localized strings (I was unaware of this initially)
  • Developers are inadvertently storing their timestamps as localized strings, resulting in an invalid database (this happened to me)
  • Developers are manually verifying that their timestamps are stored in UTC (I am currently doing this)

Considering that DateTime.timestamp() was introduced relatively recently, I believe this is a common pitfall to encounter when working with JSON serialization.

@kevmoo kevmoo added Next breaking change release Fix that will be a breaking change – good for next breaking change release and removed State: needs info labels Jan 7, 2025
@kevmoo
Copy link
Collaborator

kevmoo commented Jan 7, 2025

I'll mark this to consider for the next breaking change. Thanks! PRs welcome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Next breaking change release Fix that will be a breaking change – good for next breaking change release Type: enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants