firebase / firebase-js-sdk

Firebase Javascript SDK
https://firebase.google.com/docs/web/setup
Other
4.85k stars 893 forks source link

Date handling in functions serializer #683

Closed abrgr closed 3 years ago

abrgr commented 6 years ago

[REQUIRED] Describe your environment

[REQUIRED] Describe the problem

Javascript dates passed as arguments to firebase functions are converted to empty objects. The code in https://github.com/firebase/firebase-js-sdk/blob/master/packages/functions/src/serializer.ts does not account for dates as distinct from objects. I believe the more idiomatic method would either be to serialize dates as iso8601 and expect that functions convert to a javascript date themselves or to make use of the '@type' properties used in decode (though, obviously, this would require some backend changes). Also, despite the comment to the contrary, the decode method does not appear to handle dates.

Steps to reproduce:

Invoke a function via the callable interface.


const func = functions.httpsCallable('someFunc');
func({
  date: new Date() // this will appear as { date: {} } in someFunc
}).then(({ returnedDate }) => {
  // notice that returnedDate is a string
});
google-oss-bot commented 6 years ago

Hey there! I couldn't figure out what this issue is about, so I've labeled it for a human to triage. Hang tight.

milesingrams commented 5 years ago

Dealing with the same issue. Ive had to implement my own serialize/deserialize shim for dates for all my cloud functions. Interesting that the serializer.ts file specifically says its built to handle dates but has no functionality for dates at all. Either the documentation should change to reflect the current state or the function should include date handling. Furthermore the cloud functions should also handle serializing and deserializing firebase classes such as Timestamps and GeoPoints as it probably is not uncommon to be sending those documents in and out of cloud functions.

mohshraim commented 5 years ago

any chance for this??

bklimt commented 5 years ago

Hi. We hear you. We are still looking into the best way to support this. Thank you for your feedback and your patience.

milesingrams commented 5 years ago

Glad you’re looking into it! I know serialization/deserialization comes with a lot of caveats. Perhaps you can use a reserved property that specifies the instance type of a complex object. For example: { _instanceType: “Date”, milliseconds: 1234567890 } { _instanceType: “firebase.firestore.Timestamp”, seconds: 1234567890, nanoseconds: 1234567890 } Of course this would break if someone already has a property with the key _instanceType. Another option is to send a separate object alongside the data object that has a map of paths with the types to deserialize into. The benefit being that there are no reserved properties.

And finally you can just leave it up to the user and provide a way to extend the serialize/deserialize functions arbitrarily which would allow the user to easily use any class type they please.

I’m sure you’ve already dug into these options but thought I might as well write em out.

Thanks!

mmtftr commented 3 years ago

This is still an issue, and I've just experienced it. This should be a pretty standard thing to support, what is its progress?