change before and after data should not be undefined for onUpdate

[sk-]

The change argument of the handler for onUpdate should probably be Change<QueryDocumentSnapshot> instead of Change<DocumentSnapshot>, so that change.after.data() and change.before.data() return a non undefined object.

See https://github.com/firebase/firebase-functions/blob/master/src/providers/firestore.ts#L205-L212

If that's not the case, please clarify when either of those can be actually undefined.

[REQUIRED] Version info

firebase-functions: 3.6.0

[REQUIRED] Expected behavior

change.before.data() and change.after.data() should have a type of DocumentData

[REQUIRED] Actual behavior

change.before.data() and change.after.data() have a type of DocumentData | undefined

[google-oss-bot]

I found a few problems with this issue:

• I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.
• This issue does not seem to follow the issue template. Make sure you provide all the required information.

[mbleigh]

They are undefined for onWrite events when the data doesn't exist in either the before or the after.

…

On Sat, Apr 11, 2020, 4:02 PM Google Open Source Bot < ***@***.***> wrote: I found a few problems with this issue: - I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight. - This issue does not seem to follow the issue template. Make sure you provide all the required information. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#659 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAAH7SW5UPO6KUI3OG3P7LRMDZJ3ANCNFSM4MGFVGJQ> .

[sk-]

@mbleigh i know about the restrictions for onWrite, but I'm talking about onUpdate.

[sk-]

@mbleigh the same is true for onCreate, and onDelete. snapshot should have a type of QueryDocumentSnapshot.

[sk-]

If anyone needs to augment firebase-functions's types, you could use the following:

import {firestore} from 'firebase-functions';

import {Change, CloudFunction, EventContext} from 'firebase-functions';
import * as firebase from 'firebase-admin';

declare module 'firebase-functions/lib/providers/firestore' {
  export type QueryDocumentSnapshot = firebase.firestore.QueryDocumentSnapshot;

  interface DocumentBuilder {
    /** Respond only to document updates. */
    onUpdate(handler: (change: Change<QueryDocumentSnapshot>, context: EventContext) => PromiseLike<any> | any): CloudFunction<Change<QueryDocumentSnapshot>>;
    /** Respond only to document creations. */
    onCreate(handler: (snapshot: QueryDocumentSnapshot, context: EventContext) => PromiseLike<any> | any): CloudFunction<QueryDocumentSnapshot>;
    /** Respond only to document deletions. */
    onDelete(handler: (snapshot: QueryDocumentSnapshot, context: EventContext) => PromiseLike<any> | any): CloudFunction<QueryDocumentSnapshot>;
  }
}

I'd be happy to add this to a PR.

[laurenzlong]

Hey @sk- this is a great suggestion! When we first started supporting Firestore, QueryDocumentSnapshot was not introduced. If you can make the PR that would be very appreciated, I'd be happy to review.

[sk-]

@laurenzlong great, I just sent #670