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

chore: dependency updates, related fixes #4711

Merged
merged 6 commits into from
Dec 28, 2020
Merged

Conversation

mikehardy
Copy link
Collaborator

Description

The package.json in root and tests were going pretty stale

This brings them all up to date along with associated fixes to handle major version breaking changes in dependencies

There is still one dependency out of date - superstruct, I'm going to attempt to forward port that one as well but this is merge-able as is. If I forward port it successfully I'll remove this

Release Summary

each commit has it's own conventional commit message

Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
    • Yes
  • My change supports the following platforms;
    • Android
    • iOS
  • My change includes tests;
    • e2e tests added or updated in packages/\*\*/e2e
    • jest tests added or updated in packages/\*\*/__tests__
  • I have updated TypeScript types that are affected by my change.
  • This is a breaking change;
    • Yes
    • No

Test Plan


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

Note that the breaking change was in #4471 and was called out in release notes
This is just a remnant where the pod utilities will likely give you more notice about it
@vercel
Copy link

vercel bot commented Dec 24, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/invertase/react-native-firebase/oeh46ps6l
✅ Preview: https://react-native-firebase-git-dependency-updates.invertase.vercel.app

@mikehardy
Copy link
Collaborator Author

The only thing of concern here for review is the alteration in packages/**/lib/index.d.ts of 'Function' to specific signatures. I think I did those all correctly but they bear a second check by me then a review by someone else if they have time. That said people can ts-ignore temporarily if I got them wrong and things will still work while we patch them up after the fact if necessary

@codecov
Copy link

codecov bot commented Dec 24, 2020

Codecov Report

Merging #4711 (4e02ddd) into master (36fcfd0) will increase coverage by 3.67%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #4711      +/-   ##
==========================================
+ Coverage   85.43%   89.09%   +3.67%     
==========================================
  Files         109      109              
  Lines        3712     3712              
  Branches      347      347              
==========================================
+ Hits         3171     3307     +136     
+ Misses        475      363     -112     
+ Partials       66       42      -24     

any step that uses network resources should be in a retry to reduce
flakiness
@mikehardy
Copy link
Collaborator Author

I have successfully test-integrated the patch-package test set in my work app as a cross-check, everything still working

one in particular was enabled and failing CI, the rest should work
now that I correctly handle callback delay and local firestore persistence
by using random collections
@mikehardy
Copy link
Collaborator Author

Test integration with my work app (which uses all of this except admob/ml/database) works well in local emulators, real devices and my developer teammates devices'. Even the typing changes are intended to be non-functional and those can be fixed post-hoc if needed, going to put this in and move forward

@mikehardy
Copy link
Collaborator Author

I wasn't able to get superstruct forward-port working, I have a local commit that can either work in unit tests or in e2e tests but not both at the same time due to the way tests configures the metro packager I think. Will post it for consideration / help shortly

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

Successfully merging this pull request may close these issues.

1 participant