Skip to content
This repository has been archived by the owner on Apr 4, 2023. It is now read-only.

fix snapshot creation logic and data callback return type #662

Merged

Conversation

vcooley
Copy link
Contributor

@vcooley vcooley commented Mar 28, 2018

Addresses #661

Note that this brings the plugin API in line with the web API which could cause breaking changes for those using checks like DocumentSnapshot.data === null. If you want to use checks like that, you should use DocumentSnapshot.data() === undefined now.

@EddyVerbruggen
Copy link
Owner

Awesome! I’ll review this shortly.

@vcooley
Copy link
Contributor Author

vcooley commented Mar 28, 2018

I'm actually looking a little deeper. Looks like I'd need to update in multiple places in order to ensure that the data method returns undefined. What do you think about actually changing the DocumentSnapshot constructor to modify the data method if exists === false?

i.e. this:

export class DocumentSnapshot implements firestore.DocumentSnapshot {
  constructor(public id: string, public exists: boolean, public data: () => firestore.DocumentData) {
    if (!exists) {
     this.data = () => undefined; 
   }
  }
}

@EddyVerbruggen
Copy link
Owner

That's a great suggestion! The goal is to be as much Web-API-compatible as possible, so I love this change. I'll merge and add your suggestion as well. THANKS 👍

@EddyVerbruggen EddyVerbruggen merged commit 5685b63 into EddyVerbruggen:master Mar 31, 2018
@EddyVerbruggen EddyVerbruggen added this to the 5.2.0 milestone Mar 31, 2018
@EddyVerbruggen EddyVerbruggen self-assigned this Mar 31, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants