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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #1788 NSNull vs nil issue #1790

Merged
merged 2 commits into from Dec 24, 2018

Conversation

Projects
None yet
3 participants
@noahtallen
Copy link
Contributor

noahtallen commented Dec 24, 2018

Summary

There was a minor error in the iOS logs which showed that newer versions of the iOS Firebase SDK were returning NSNull instead of nil. (See #1788 for info.) This small PR adds a check for NSNull on top of nil, and the log error does not show up any more.

Checklist

  • (n/a) Supports Android
  • Supports iOS
  • e2e tests added or updated in /tests/e2e/*
  • (n/a) Updated the documentation in the docs repo
  • (n/a) Flow types updated
  • (n/a) Typescript types updated

Test Plan

Tested by:

  1. Updated the line of code in react-native-firebase, and copied that change to an existing project I have which utilizes react-native-firebase
  2. Observed the native iOS logs, and the error message no longer showed up. Previously, a couple hundred lines of this error would display.

Release Plan

[IOS][BUGFIX] [Firestore] - Fixed iOS log error "Unsupported value sent to buildTypeMap - class type is NSNull" which occurs when using iOS Firebase SDK version 5.15.


Think react-native-firebase is great? Please consider supporting the project with any of the below:

@CLAassistant

This comment has been minimized.

Copy link

CLAassistant commented Dec 24, 2018

CLA assistant check
All committers have signed the CLA.

@codecov

This comment has been minimized.

Copy link

codecov bot commented Dec 24, 2018

Codecov Report

Merging #1790 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1790   +/-   ##
=======================================
  Coverage   76.16%   76.16%           
=======================================
  Files          72       72           
  Lines        1838     1838           
=======================================
  Hits         1400     1400           
  Misses        438      438
3 similar comments
@codecov

This comment has been minimized.

Copy link

codecov bot commented Dec 24, 2018

Codecov Report

Merging #1790 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1790   +/-   ##
=======================================
  Coverage   76.16%   76.16%           
=======================================
  Files          72       72           
  Lines        1838     1838           
=======================================
  Hits         1400     1400           
  Misses        438      438
@codecov

This comment has been minimized.

Copy link

codecov bot commented Dec 24, 2018

Codecov Report

Merging #1790 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1790   +/-   ##
=======================================
  Coverage   76.16%   76.16%           
=======================================
  Files          72       72           
  Lines        1838     1838           
=======================================
  Hits         1400     1400           
  Misses        438      438
@codecov

This comment has been minimized.

Copy link

codecov bot commented Dec 24, 2018

Codecov Report

Merging #1790 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1790   +/-   ##
=======================================
  Coverage   76.16%   76.16%           
=======================================
  Files          72       72           
  Lines        1838     1838           
=======================================
  Hits         1400     1400           
  Misses        438      438

@Salakar Salakar merged commit 094b1b1 into invertase:master Dec 24, 2018

9 checks passed

WIP ready for review
Details
ci/circleci: analyse Your tests passed on CircleCI!
Details
ci/circleci: android-build-debug Your tests passed on CircleCI!
Details
ci/circleci: android-build-release Your tests passed on CircleCI!
Details
ci/circleci: checkout-code Your tests passed on CircleCI!
Details
ci/circleci: ios-test-e2e Your tests passed on CircleCI!
Details
codecov/patch Coverage not affected when comparing 5c1e838...13660eb
Details
codecov/project 76.16% remains the same compared to 5c1e838
Details
license/cla Contributor License Agreement is signed.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can鈥檛 perform that action at this time.