firebase.firestore.DocumentSnapshot.exists v8 -> v9 breaking change

[johnnyoshika]

[REQUIRED] Describe your environment

• Operating System version: Windows 10
• Browser version: Chrome
• Firebase SDK version: 9.6.5
• Firebase Product: firestore

[REQUIRED] Describe the problem

Steps to reproduce:

There is a major breaking change in firebase.firestore.DocumentSnapshot.exists between Web SDK v8 and v9 and I don't see it being called out anywhere. In v8, DocumentSnapshot.exists is a property while in v9, it's a method.

v8: https://firebase.google.com/docs/reference/js/v8/firebase.firestore.DocumentSnapshot#exists
v9: https://firebase.google.com/docs/reference/js/firestore_.documentsnapshot.md#documentsnapshotexists

Is this intentional? This can inadvertently cause serious bugs, as a falsy snap.exists in v8 now becomes an always truthy snap.exists in v9. For example, in v8:

    firestore
      .doc(docPath)
      .onSnapshot(
        snap => {
          if (snap.exists)
             console.log('do something')
       }
   )

becomes this in v9:

onSnapshot(
      docRef,
      snap => {
          if (snap.exists) // this is now always true in v9
             console.log('do something')
     }
)

The correct condition in v9 is snap.exists(), but this required change is not called out anywhere in the upgrade guides

• https://firebase.google.com/docs/web/modular-upgrade
• https://firebase.googleblog.com/2021/08/the-new-firebase-js-sdk-now-ga.html

Are there other breaking changes like this that we should be aware of?

Note also that the Node.js client's DocumentSnapshot.exists is a property, so for some reason v9 has created an inconsistency between libraries.

The documentation here needs to be updated, as exists() is no longer a property even though the comment seems to imply that it is.

Relevant Code:

See above

[dconeybe]

Hello @johnnyoshika. I'm sorry that you ran into this subtle bug during the v8 to v9 upgrade. That is definitely something that we overlooked and acknowledge that it could be an easy source of bugs. Unfortunately, we cannot change exists back to a property because that would be yet another breaking change.

Also, one of the benefits of it being a method is that it allows for TypeScript type guards. For example, in this code block

if (docSnap.exists()) {
   var data : DocumentData = docSnap.data() // Not nullable anymore
}

the TypeScript compiler deduces that the return value of docSnap.data() cannot be null, removing the need to explicitly check for non-nullness in your code.

I'm looking into calling this out specifically in the migration guide, as you suggest. I'll update this issue with the outcome of that investigation.

[johnnyoshika]

Thanks @dconeybe! The TypeScript type guard is really neat. I didn't realize that.

I don't think there's anything wrong with breaking changes, especially in major version bumps. I was just surprised to see it on DocumentSnapshot.exists. Amending the migration guide is a good idea. Thanks!

[dconeybe]

The v8 -> v9 migration guide has been updated. The changes are live here: https://firebase.google.com/docs/web/modular-upgrade#exists-update. Googlers can see the changes in cl/426428626. Thank you for reporting this issue. Other Firestore users will definitely benefit from this documentation update.

[johnnyoshika]

Excellent, thank you @dconeybe!