[Feat] Firestore SSR to/from JSON for data types #8928
Conversation
|
Vertex AI Mock Responses Check
|
Size Report 1Affected ProductsNo changes between base commit (41b82e9) and merge commit (d39e94c).Test Logs |
Size Analysis Report 1Affected ProductsNo changes between base commit (41b82e9) and merge commit (d39e94c).Test Logs |
DellaBitta
changed the title
| @@ -91,4 +91,40 @@ export class Bytes { | |||
| isEqual(other: Bytes): boolean { | |||
| return this._byteString.isEqual(other._byteString); | |||
| } | |||
|
|
|||
| /** Returns a JSON-serializable representation of this `Bytes` instance. */ | |||
There was a problem hiding this comment.
These descriptions look good, but do we also need an @returns ... tag?
| error = `json missing required field: ${key}`; | ||
| } | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| const value = (json as any)[key]; |
There was a problem hiding this comment.
instead of const value = (json as any)[key];
You could just do
if (!(key in json)) {
error = `json missing required field: ${key}`;
} else {
json[key] ...
}
There was a problem hiding this comment.
Unfortunately, this still produces a compilation error:
Element implicitly has an 'any' type because expression of type 'string' can't be used to index type '{}'.
No index signature with a parameter of type 'string' was found on type '{}'.ts(7053)
Gemini suggested other ways to fix this but they seemed even more compilcated.
| if (typeof value !== 'string') { | ||
| error = `json field 'type' must be a string.`; | ||
| break; | ||
| } else if (value !== 'firestore/geopoint/1.0') { |
There was a problem hiding this comment.
I'm wondering if we should have this json have a version number independent of the SDK version.
If they are independent, that means JSON can be deserialized and reserialized across SDK versions... Maybe that's okay.
But if they are the same, that means the same SDK must be used for serialization and deserialization, which would also free us up to make changes more easily
There was a problem hiding this comment.
We spoke about this offline.
|
Accidentally pushed these changes directly into the Feature PR. Closing this for now. |
Discussion
Add support for toJSON and fromJSON for Firestore data types. This is part of the Firestore SSR data serialization feature, and this PR points to that feature branch, not main.
Testing
Added unit tests for all of the new methods.
API Changes
Added
toJSON()andfromJSON()methods to the following classes:BytesDocumentReferenceGeoPointTimestampVectorValue