Skip to content

DataSnapshot forEach method should be able to return void #555

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Francescu opened this issue Mar 7, 2018 · 5 comments
Closed

DataSnapshot forEach method should be able to return void #555

Francescu opened this issue Mar 7, 2018 · 5 comments
Assignees

Comments

@Francescu
Copy link

Code from the doc doesn't use the return boolean from the forEach function when it doesn't want to interrupt the enumeration.

Example

var query = firebase.database().ref("users").orderByKey();
query.once("value")
  .then(function(snapshot) {
    snapshot.forEach(function(childSnapshot) {
      // key will be "ada" the first time and "alan" the second time
      var key = childSnapshot.key;
      // childData will be the actual contents of the child
      var childData = childSnapshot.val();
  });
});

Problem

This code is not valid with the current Typescript definitions.

export interface DataSnapshot {
  forEach(action: (a: DataSnapshot) => boolean): boolean;
}

Proposal

I'm new to Firebase so I'd be careful there. But it seems that adding to the closure return type |void would fix the issue. I can provide a PR, if you're OK with this solution.

export interface DataSnapshot {
  forEach(action: (a: DataSnapshot) => boolean|void): boolean;
}
@google-oss-bot
Copy link
Contributor

Hmmm this issue does not seem to follow the issue template. Make sure you provide all the required information.

@google-oss-bot
Copy link
Contributor

Hey there! I couldn't figure out what this issue is about, so I've labeled it for a human to triage. Hang tight.

@schmidt-sebastian
Copy link
Contributor

Thanks for sending this over @Francescu. We should indeed update the types. If you want to take a stab at this, I would be more than welcome to review your PR.

Take a look here: https://github.com/firebase/firebase-js-sdk/blob/master/packages/database-types/index.d.ts

@jspri
Copy link
Contributor

jspri commented May 18, 2018

@jshcrowthe
Copy link
Contributor

Closing this as it seems things have been taken care of. Thanks!

villesau pushed a commit to flow-typed/flow-typed that referenced this issue Oct 12, 2018
sprmn added a commit to sprmn/firebase-admin-node that referenced this issue Nov 15, 2018
hiranya911 pushed a commit to firebase/firebase-admin-node that referenced this issue Nov 16, 2018
@firebase firebase locked and limited conversation to collaborators Oct 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants