Skip to content
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

fix(database): allow null values for equalTo, etc. #715

Merged
merged 2 commits into from
Dec 12, 2016

Conversation

cartant
Copy link
Contributor

@cartant cartant commented Dec 10, 2016

Checklist

After following the steps in the contribution guide, the generated test-root.umd.js bundle was empty - apart from the boilerplate. This could have something to do with Windows being my OS. Hopefully, Travis will run the tests for me.

Description

This PR includes a refactor to rename a method that was considered to be misleading and a fix to allow the equalTo, startAt and endAt options for list queries to be null - as the current AngularFire2 behaviour differs from that of the underlying SDK (see the repro in the associated issue).

I've renamed the method to isNil - rather than the suggested isEmpty - as isNil is the name that's used in lodash (for an identical method) and using isEmpty would make the implementation of isEmptyObject too confusing:

export function isEmptyObject(obj: Object): boolean {
  // If the variable is considered empty, the object isn't? Huh?
  // What does empty mean?
  if (isEmpty(obj)) { return false; }
  return Object.keys(obj).length === 0 && JSON.stringify(obj) === JSON.stringify({});
}

isPresent is a misleading name (see angular#704) as it considers options
specified as null to not be present.
Values passed for equalTo, startAt and endAt should be allowed to be
null.

Closes angular#704.
@katowulf katowulf merged commit 70a3e94 into angular:master Dec 12, 2016
@katowulf
Copy link
Contributor

🚀🚀🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants