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

feat(android, emulator): add firebase.json config element to bypass localhost remap #5852

Merged

Conversation

stchristian
Copy link
Contributor

Description

I came accross a problem with emulators and the way they are handling localhost addresses, described here.

I would love to see this work with firebase emulators, when running and testing apps on real devices.

I started a discussion here

Related issues

Release Summary

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:

🔥

@CLAassistant
Copy link

CLAassistant commented Nov 15, 2021

CLA assistant check
All committers have signed the CLA.

@vercel
Copy link

vercel bot commented Nov 15, 2021

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

react-native-firebase – ./

🔍 Inspect: https://vercel.com/invertase/react-native-firebase/4RdaowmCRKdFh6vMS6xQRpVDY7S4
✅ Preview: https://react-native-firebase-git-fork-stchristian-byp-9ba69a-invertase.vercel.app

react-native-firebase-next – ./website_modular

🔍 Inspect: https://vercel.com/invertase/react-native-firebase-next/5QKerrqomXoxqoAeFUjqUaS6z912
✅ Preview: Canceled

@vercel vercel bot temporarily deployed to Preview – react-native-firebase-next November 15, 2021 21:57 Inactive
@stchristian stchristian changed the title feat(emulator) add the choice bypass url remap feat(emulator): add the choice bypass url remap Nov 15, 2021
Copy link
Collaborator

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - thank you! The CI runs will produce a patch-package set that you may use immediately if you like, until this is released

We'll need the CLA signed though for sure - follow the details link there for more information if you haven't already signed it

Thanks again - I do like the toggle, I think the dev experience is better with the remapping but it's nice to not have it be mandatory

@mikehardy mikehardy changed the title feat(emulator): add the choice bypass url remap feat(android, emulator): add firebase.json config element to bypass localhost remap Nov 15, 2021
@codecov
Copy link

codecov bot commented Nov 15, 2021

Codecov Report

Merging #5852 (5d52622) into main (514e6bd) will decrease coverage by 19.41%.
The diff coverage is 66.67%.

❗ Current head 5d52622 differs from pull request most recent head ca230a3. Consider uploading reports for the commit ca230a3 to get more accurate results

@@              Coverage Diff              @@
##               main    #5852       +/-   ##
=============================================
- Coverage     72.41%   53.01%   -19.40%     
- Complexity        0      620      +620     
=============================================
  Files           109      208       +99     
  Lines          4534    10124     +5590     
  Branches       1003     1595      +592     
=============================================
+ Hits           3283     5366     +2083     
- Misses         1173     4503     +3330     
- Partials         78      255      +177     

@mikehardy
Copy link
Collaborator

Hey @stchristian - a gentle nudge to sign the CLA, if you follow the details link from the related comment it should walk you through it. Thanks!

@stchristian
Copy link
Contributor Author

Something went wrong with the tests:ios:test-cover. Should I care about that? @mikehardy

@mikehardy
Copy link
Collaborator

No, we're currently battling a nasty bit of flakiness from wix/Detox#3000 upstream, I've been re-starting it periodically, it will pass eventually. I have no doubt it's unrelated to this and will go green

@stchristian
Copy link
Contributor Author

Also, where can I see the patch package that has been producer?

@mikehardy
Copy link
Collaborator

They're linked off the action - should be able to see them here https://github.com/invertase/react-native-firebase/actions/runs/1464262461

@mikehardy mikehardy merged commit ddf3f5f into invertase:main Nov 16, 2021
@mikehardy
Copy link
Collaborator

boom! Thanks again

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.

3 participants