rtdb(types): DataSnapshot#forEach - action parameter has null key

[rhodgkins]

[REQUIRED] Describe your environment

• Operating System version: N/A
• Browser version: N/A
• Firebase SDK version: 9.8
• Firebase Product: database

[REQUIRED] Describe the problem

When using DataSnapshot#forEach the parameter available to the action closure is a DataSnapshot which is correct, but the key property of this value has the type string | null, but as its a child of the iterating snapshot it can't be the root node so its key can never be null.

Steps to reproduce:

const node = await getDatabase().ref().get()
node.forEach(child => {
   const key = child.key
   // key is possibly null so have to cast etc.
})

Relevant Code:

https://github.com/firebase/firebase-js-sdk/blob/efe2000fc499e2c85c4e5e0fef6741ff3bad2eb0/packages/database-types/index.d.ts#L25

https://github.com/firebase/firebase-js-sdk/blob/efe2000fc499e2c85c4e5e0fef6741ff3bad2eb0/packages/firebase/compat/index.d.ts#L5821-L5823

https://github.com/firebase/firebase-js-sdk/blob/efe2000fc499e2c85c4e5e0fef6741ff3bad2eb0/packages/database-compat/src/api/Reference.ts#L167

Fix

Happy to do a PR for this - I just remember it being a bit complicated last time I did this. Is the info in that issue still correct - the function signature in the above 3 places just needs to be changed?

Would changing the function signature to forEach(action: (a: DataSnapshot & { key: string }) => boolean | void): boolean; be OK for a fix?

[maneesht]

That syntax doesn't quite match v9, but your suggestions should work. For v9, you still need to update https://github.com/firebase/firebase-js-sdk/blob/d87d3a8b8cd68e757be6628a72538bfd303e78d1/packages/database/src/api/Reference_impl.ts#L397

[maneesht]

If you could create a PR addressing this, that would be great

[rhodgkins]

That syntax doesn't quite match v9

@maneesht Can you suggest some syntax that would match please!

If you could create a PR addressing this, that would be great

Will do!

[maneesht]

If you're looking at the root, then:

import { getDatabase } from 'firebase/database';
const rootSnapshot = await get(ref(getDatabase()));
rootSnapshot.forEach(node => {
  console.log(node.key);
});

Out of curiosity, are you using v9-compat? (The non-modular version of firebase database)

[rhodgkins]

Oh I see the syntax of the example. Yes I'm using firebase-admin.

[hsubox76]

Those 3 places you pointed out are the correct places to change, and although you are not using the v9 API, the compat API actually wraps the v9 API so you may have to change the line @maneesht indicated as well, because the compat forEach calls the v9 forEach here https://github.com/firebase/firebase-js-sdk/blob/efe2000fc499e2c85c4e5e0fef6741ff3bad2eb0/packages/database-compat/src/api/Reference.ts#L171 and the signatures may have to match.

[rhodgkins]

Great - thanks both.

[maneesht]

@rhodgkins thank you for your contribution. However, we will have to rollback this change as it is causing a compilation error with AngularFire. We will add this back on the next major release. Until then, I will reopen this for tracking purposes.

[maneesht]

Closing this as it was part of the 10.0 release: https://firebase.google.com/support/release-notes/js#realtime-database