Skip to content

feat(app-check): add Swift AppDelegate support for Expo SDK53+ #8521

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

Merged
merged 2 commits into from
May 12, 2025

Conversation

jeremyplt
Copy link
Contributor

Description

Adding support for Swift AppDelegate for the new version of Expo SDK (53) for Firebase AppCheck.

Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
    • Yes
  • My change supports the following platforms;
    • Android
    • iOS
    • Other (macOS, web)
  • 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

Unit Tests:
Test Suites: 39 passed, 39 total
Tests: 1 skipped, 789 passed, 790 total
Snapshots: 31 passed, 31 total
Time: 3.848 s

E2E Tests:
PASS e2e/firebase.test.js (556.894 s)
Jet Tests
✓ runs all tests (527266 ms)

Test Suites: 1 passed, 1 total
Tests: 1 passed, 1 total
Snapshots: 0 total
Time: 556.965 s
Ran all test suites.


🔥

Copy link

vercel bot commented May 8, 2025

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 May 12, 2025 8:04pm

@CLAassistant
Copy link

CLAassistant commented May 8, 2025

CLA assistant check
All committers have signed the CLA.

@ShantIssa
Copy link

Hi, thanks for working on this!

Just wanted to check — is this ready to be used as-is from the branch, or are there still known issues or missing pieces that you're planning to address before merge? Also, is there a target version or timeline for when this might be included in a release?

Appreciate the work — looking forward to this being available!

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.

This looks great to me, thank you so much for posting the PR
will get this merged and kick out a release, if anyone else sees this and there's a problem with it post-release (perhaps some interaction with the app config plugin? you never know) I will happily take any follow-ons and release them as quickly as I can.

(maintener note to myself: CI is running and should go green on this but we don't normally take Podfile.lock updates from PRs - if there is a hang-up on iOS CI will strip the Podfile.lock commit via rebase, re-push, then merge...)

@mikehardy mikehardy force-pushed the fix/app-check-swift-expo branch from 017ede7 to 14079e8 Compare May 12, 2025 20:00
@mikehardy
Copy link
Collaborator

Had a trivial lint issue, I fixed that and since I was re-pushing anyway, stripped the lockfile update (I do those when updating the test app normally, just my personal preference). Should all go green now but if not will shepherd it through CI with whatever is needed. I'm about to do a release and I want this in there :-). Cheers

@mikehardy mikehardy merged commit 8c631e1 into invertase:main May 12, 2025
16 checks passed
@mikehardy mikehardy removed the Workflow: Pending Merge Waiting on CI or similar label May 12, 2025
@rorogeno
Copy link

Really cool thank you 👍

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

Successfully merging this pull request may close these issues.

5 participants