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(app-check): Add support for Expo config plugin #7662

Merged
merged 4 commits into from
Mar 21, 2024

Conversation

mickeywu
Copy link
Contributor

@mickeywu mickeywu commented Mar 1, 2024

Description

App Check needs additional setup in AppDelegate to work which is difficult in a managed Expo workflow.

Created a Expo config plugin similar to other react-native-firebase packages such as the one found in Dynamic Links. The pull request code is also modeled after the Dynamic Links config plugin.

Release Summary

Added support for Expo config plugin.

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

Tests have passed. Verified in iOS builds that the new config plugin will add lines as described in the App Check setup step.


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

🔥

Copy link

vercel bot commented Mar 1, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-native-firebase ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 21, 2024 2:55am

@CLAassistant
Copy link

CLAassistant commented Mar 1, 2024

CLA assistant check
All committers have signed the CLA.

@mickeywu mickeywu changed the title (feat app-check) add support for Expo config plugin feat(app-check): Add support for Expo config plugin Mar 1, 2024
@mickeywu
Copy link
Contributor Author

Hi @mikehardy, was wondering if you need anything else to get this PR approved? It might look like a lot of changes but it essentially is a copy of the Expo plugin setup found in Dynamic Links but with a few strings changed.

Much appreciated for all your hard work on this project 🙏

@mikehardy mikehardy added the Workflow: Needs Review Pending feedback or review from a maintainer. label Mar 21, 2024
mikehardy
mikehardy previously approved these changes Mar 21, 2024
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.

Sorry - whole family came down with good ol' COVID except me but I played the part of nurse. Everyone's fine :-), but I'm swamped with backlog now, that's for sure

This looks great, triggering CI after I commit the one change for Expo branch name

@mikehardy mikehardy added Workflow: Pending Merge Waiting on CI or similar and removed Workflow: Needs Review Pending feedback or review from a maintainer. labels Mar 21, 2024
@mikehardy mikehardy force-pushed the feat/app-check-config-plugin branch from bd3cf07 to e5874c0 Compare March 21, 2024 02:49
@mikehardy
Copy link
Collaborator

There was a trivial error with lint and a failure to include the expo plugin as a dev dependency - I've resolved those and pushed back out the branch. If CI passes I'll merge this, thank you!

Copy link

codecov bot commented Mar 21, 2024

Codecov Report

Merging #7662 (e5874c0) into main (fae5966) will decrease coverage by 34.69%.
The diff coverage is 79.42%.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #7662       +/-   ##
===========================================
- Coverage   68.05%   33.36%   -34.69%     
===========================================
  Files         149      252      +103     
  Lines        5943    12497     +6554     
  Branches     1250     1951      +701     
===========================================
+ Hits         4044     4168      +124     
- Misses       1802     8240     +6438     
+ Partials       97       89        -8     

@mikehardy mikehardy removed the Workflow: Pending Merge Waiting on CI or similar label Mar 21, 2024
@mikehardy mikehardy merged commit c6e813e into invertase:main Mar 21, 2024
15 of 17 checks passed
@mickeywu
Copy link
Contributor Author

Thanks for the approval and merge @mikehardy, hope your family is feeling better!

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