WithFieldValue mangles Class Types

[rgant]

Please make sure you have searched for information in the following guides.

• [x] Search the issues already opened: https://github.com/GoogleCloudPlatform/google-cloud-node/issues
• [x] Search StackOverflow: http://stackoverflow.com/questions/tagged/google-cloud-platform+node.js
• [x] Check our Troubleshooting guide: https://github.com/googleapis/google-cloud-node/blob/main/docs/troubleshooting.md
• [x] Check our FAQ: https://github.com/googleapis/google-cloud-node/blob/main/docs/faq.md
• [x] Check our libraries HOW-TO: https://github.com/googleapis/gax-nodejs/blob/main/client-libraries.md
• [x] Check out our authentication guide: https://github.com/googleapis/google-auth-library-nodejs
• [x] Check out handwritten samples for many of our APIs: https://github.com/GoogleCloudPlatform/nodejs-docs-samples

A screenshot that you have tested with "Try this API".

Attempting to use WithFieldValue<Date> mangles to Date type such that tsc no longer recognizes it as a Date. I assume this happens for most other non-primitive values in Firestore documents.

   63:37  error   Argument of type 'FieldValue | WithFieldValue<Date>' is not assignable to parameter of type 'Date | FieldValue'. ​typescript(2345)
                    Type '{ toString: FieldValue | WithFieldValue<() => string>; toDateString: FieldValue | WithFieldValue<() => string>; toTimeString: FieldValue | WithFieldValue<...>; ... 41 more ...; [Symbol.toPrimitive]: FieldValue | WithFieldValue<...>; }' is not assignable to type 'Date | FieldValue'.
                      Type '{ toString: FieldValue | WithFieldValue<() => string>; toDateString: FieldValue | WithFieldValue<() => string>; toTimeString: FieldValue | WithFieldValue<...>; ... 41 more ...; [Symbol.toPrimitive]: FieldValue | WithFieldValue<...>; }' is not assignable to type 'Date'.
                        Types of property 'toString' are incompatible.
                          Type 'FieldValue | WithFieldValue<() => string>' is not assignable to type '() => string'.
                            Type 'FieldValue' is not assignable to type '() => string'.
                              Type 'FieldValue' provides no match for the signature '(): string'.

Link to repro. A link to a public Github Repository or gist with a minimal reproduction.

https://github.com/rgant/brainfry

https://gist.github.com/rgant/adf3984ebd43c278b716c372412f7109/193be3d9e79f8beafc452a2c814af61883ea9958

A step-by-step description of how to reproduce the issue, based on the linked reproduction.

This all happens when running linters and type checkers in my editor.

A clear and concise description of what the bug is, and what you expected to happen.

Because WithFieldValue<Post> climbs inside of the Date property this results problems with typing as seen on line 63

I can fix this (with some ugly unnecessary code my throwing a TypeError if the parameter isn't a Date nor a FieldValue.

A clear and concise description WHY you expect this behavior, i.e., was it a recent change, there is documentation that points to this behavior, etc. **

I feel like the documentation update is better, but doesn't actually cover a real world case. Maybe I'm just strange but I wouldn't use a number of milliseconds in a front end application where I expect to display a date and time of a post. I would use a JavaScript Date (or Temporal).

[github-actions[bot]]

Issue was opened with an invalid reproduction link. Please make sure the repository is a valid, publicly-accessible github repository, and make sure the url is complete (example: https://github.com/googleapis/google-cloud-node)

[rgant]

If interested I can open a PR to update the documentation with my fixes. However, it really does feel like WithFieldValue (and PartialWithFieldValue) are not correct and should be fixed.

[rgant]

Issue was opened with an invalid reproduction link. Please make sure the repository is a valid, publicly-accessible github repository, and make sure the url is complete (example: https://github.com/googleapis/google-cloud-node)

Link should be good now. Hard to manage multiple revisions in a gist.

[milaGGL]

Hi @rgant, thank you for raising this issue. As you noticed, T is passed down only if it is a primitive data type, otherwise, it needs to wrap it with WithFieldValue as in your fix.

[rgant]

But then TypeScript doesn't understand that date instanceof Date is a type guard for WithFieldValue<Date> which since it is just a type doesn't have a way to type guard that I know of.

If WithFieldValue cannot be changed to not mangle standard Objects, then should the documentation be updated with a more real world example that documents this typical experience?

[MarkDuckworth]

seems like the bot incorrectly closed this issue

[github-actions[bot]]

Issue was opened with an invalid reproduction link. Please make sure the repository is a valid, publicly-accessible github repository, and make sure the url is complete (example: https://github.com/googleapis/google-cloud-node)

[github-actions[bot]]

Issue was opened with an invalid reproduction link. Please make sure the repository is a valid, publicly-accessible github repository, and make sure the url is complete (example: https://github.com/googleapis/google-cloud-node)

[rgant]

I tried adding another link to a repo to make the bot happy hopefully.

[MarkDuckworth]

@rgant, This is an interesting issue. If I understand, the general request is that the type WithFieldValue<T> will stop traversing into certain object types. In the example below you (or another user) might want it to traverse into Author, but not Date.

type Post = {
  title: string;
  author: Author;
  lastUpdated: Date;
}

type Author = {
  name: string;
}

type T = WithFieldValue<Post>;

I'm not aware of how WithFieldValue<T> could be that selective.

The runtime check d instanceof Date may your best solution for now.

[github-actions[bot]]

Issue was opened with an invalid reproduction link. Please make sure the repository is a valid, publicly-accessible github repository, and make sure the url is complete (example: https://github.com/googleapis/google-cloud-node)

[rgant]

I think there are relatively few types that can both appear in a Firestore document and payload without erring.

Could we add Date | Timestamp to the Primitive type? https://github.com/googleapis/nodejs-firestore/blob/c6c85b66a25b56bd23c19285302a740b0ca85d25/types/firestore.d.ts#L79

Or add some sort of "Classy" type like declare type Class<T = any> = new (...args: any[]) => T;

[MarkDuckworth]

Updated the description to stop the auto-close for "invalid link"

[MarkDuckworth]

I think there are relatively few types that can both appear in a Firestore document and payload without erring

I'm not sure if I understand. The type(s) passed to the converter (e.g. Post, Date, Author...) may or may not be converted to some other object that is written to the DB. Limiting the solution to Date and Timestamp, doesn't seem like a general solution. Interesting idea too about the "Classy" type, but I think that would make it ignore Author and Class, if they were defined as a class and not a type.

I may be able to tweak WithFieldValue<T> to ignore any functions, which would address the issue you're experiencing with Date, and also be a general improvement, I think.

[rgant]

It is my belief that WithFieldValue exists to allow you do do things like serverTimestamp() and arrayUnion(). Since post.author = FieldValue.delete() is a valid construct I don't have a problem that WithFieldValue applies to it.

What I think is wrong is post.lastUpdated.toJson = FieldValue.delete() since that is never a correct. So it sounds like ignoring functions would solve part of the problem.