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

Provide support to serialize dates in util-dynamodb #1534

Closed
trivikr opened this issue Sep 18, 2020 · 4 comments · Fixed by #1969
Closed

Provide support to serialize dates in util-dynamodb #1534

trivikr opened this issue Sep 18, 2020 · 4 comments · Fixed by #1969
Labels
feature-request New feature or enhancement. May require GitHub community feedback.

Comments

@trivikr
Copy link
Member

trivikr commented Sep 18, 2020

Is your feature request related to a problem? Please describe.
util-dynamodb currently throws error when date is passed.
Refs: #1531

Describe the solution you'd like
The problem is with JSON spec itself: there is no literal syntax for dates in JSON.
JSON spec: https://www.ietf.org/rfc/rfc4627.txt

Possible solution #1: Convert dates to ISO 8601 strings by default. Provide an option (say convertDateToNumber) which converts date to a string value. This option can be passed along with convertEmptyValues

Possible solution #2: Expose pre-marshall configuration allowing users to send their own convert functions, detailed in #1533

Describe alternatives you've considered
Converting dates to String/Number or a different format before passing it to the converter.

Additional context
This request was raised in PR #1531 (comment)

@trivikr
Copy link
Member Author

trivikr commented Sep 18, 2020

If we go ahead with using JSON.stringify() for serializing objects as suggested in #1535, then date will be stored as a String in ISO 8601 format.

We still need to discuss adding convertDateToNumber (or similar) option, which converts date to number (output of .getTime()) instead of string.

@russell-dot-js
Copy link
Contributor

russell-dot-js commented Sep 19, 2020

I like the idea of ISO 8601 by default, especially considering nobody wants the current default behavior of serializing to {}. That being said, it's only OK if we allow a way out of it.

convertDateToNumber, just like convertEmptyValues, seems like a trap to me, as it could be a slippery slope:

  • convertBufferToArray
  • convertPromiseToAwaited
  • convertArrayToSet
  • convertBigIntToString
  • convertGoToRust

Hence I prefer letting consumers inject their own serialization preferences - nothing is stopping them from re-using and composing their rules as they see fit

@trivikr
Copy link
Member Author

trivikr commented Oct 1, 2020

Update: we're exploring serialization of Date objects using premarshalling #1533 (comment)

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request New feature or enhancement. May require GitHub community feedback.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants