googleapis / googleapis/google-cloud-node
Make it more easily possible to write objects with custom prototypes
- Dominant language
- TypeScript
- Stars
- 3.2k
- Forks
- 712
- Avg merge
- 2d 3h
- Merged PRs (30d)
- 99
Description
## Problem
When you work with classes and objects quite a bit you will see these errors appear over and over:
`Error: Value for argument "data" is not a valid Firestore document. Couldn't serialize object of type "ArticleIdentifier" (found in field "identifier"). Firestore doesn't support JavaScript objects with custom prototypes (i.e. objects that were created via the "new" operator).`
Especially if you have complex classes and models parsing/converting the model, again and again, is cumbersome and something I'd like to avoid. These classes are helpful to abstract complexity away from Firestore, etc. it's often more abstract/portable to replace the `Timestamp` class with a native Date object.
## Solution
It would be a lot cooler if I could simply do something like this:
```ts
ref.doc('articles/abc').set({
identifier: new ArticleIdentifier('abc'),
})
```
And this would work because I can in the class I'd have a method like this that is called by Firestore and would return the corresponding datatype for serializing the document to Firestore:
```ts
toProto(firestore) {
return firestore.doc(`articles/${id}`);
}
```
This solution is very much inspired by what is possible to do with converting to strings `${identifier}` and to JSON `JSON.stringify(identifier)` where I could simply add these two methods...
```ts
toString(): string {
return this.path;
}
toJSON(): string {
return this.path;
}
```
...and it would automatically convert the property to the correct representation.
There are two things that I want to point out about the implementation, the `toProto` (or whatever it will be called `toFirestore`?!) should get the used firestore app for writing to the database, this will make it a lot easier to work within a multi-app setup.
Second, this is only intended as a convenience feature for writing, but not reading, I know that those use-cases are almost impossible to achieve out-of-the-box, and I anyways do not intend to use that.
## Alternatives considered
### .withConverters
One way to approach that (probably the intended way right now) is to simply use the `.withConverters` feature, however, that does not really solve the issue, but only is a more out-of-the-box way to write converters, but we'd still need to manually convert the whole document.
### Own serializer
The second solution would be to introduce this function yourself and loop through the whole document data like the [`encodeValue` function](https://github.com/googleapis/nodejs-firestore/blob/a6ba5ccf8b21dbe117e5d8ad5fc1e26d16ed0d1d/dev/src/serializer.ts#L103) and detect all objects that have this function `toProto` and serialize it a Firestore accepted value (⚠️ DO NOT USE `toProto` here rather use `toFirestore` or something else as this may otherwise lead to unexpected results as internally Firestore uses `toProto` and it would convert a DocumentReference, GeoPoint, ... to something unexpected)
### Hack around the validations 🌝
⚠️ I would NOT use this approach this was only an experiment and may lead to unexpected side-effects due to changes in the Firestore SDK. ⚠️
1. Extend from a native Firestore class: `export class Identifier extends GeoPoint`
2. Implement the `toProto` method in the class and let it return a JSON object in the format of the [REST APIs Value](https://cloud.google.com/firestore/docs/reference/rest/Shared.Types/ArrayValue#Value).
Example:
DO NOT USE ME IN PRODUCTION
```
// !!!! NEVER USE THIS IN PRODUCTION !!!!
export class Identifier extends GeoPoint {
_segments;
constructor(path) {
super(0, 0);
this._segments = path.split('/');
}
toProto() {
const components = [
'projects',
app().options.projectId,
'databases',
'(default)',
'documents',
...this._segments,
];
return {referenceValue: components.join('/')};
}
}
// !!!! NEVER USE THIS IN PRODUCTION !!!!
```
## Additional context
This feature seems fairly straightforward to implement:
Instead of the [default converter](https://github.com/googleapis/nodejs-firestore/blob/ce9abfd8cd65d0d2be4dd7f6a56fb679a100296a/dev/src/types.ts#L120) that would simply return the data as is, the converter could simply look out for those kinds of objects that implement this function and call it.
Then there would still be the same safety feature for validation, etc. in place as the conversion happens as the first operation in all write operations!
Happy to hear your thoughts, especially also on the API design (I know this is always a huge part for implementing such a feature)!
Contributor guide
Assessment
This issue has not been assessed yet.