-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
[π] Config Plugin breaks on Expo SDK 44 #5936
Comments
It looks like this regex isn't matching: https://github.com/invertase/react-native-firebase/blob/main/packages/app/plugin/src/ios/appDelegate.ts#L7 |
Oh darn - this happened with Expo 34 as well. It appears there is no stable interface here yet? Additionally I'm not sure if Expo allows patch-package to do it's thing but if you do propose anything, I made a CI action that generates a patch-package set for the monorepo packages, which helps get you over the hurdle of quick feedback, although I suppose tweaking a regex in node_modules isn't that hard of a case |
A patch would be fine here for me. Unfortunately I have no idea where to start β I donβt know what an app delegate looks like or anything like that. Iβm spoiled by expo and only know JS haha. |
They hoped not to change AppDelegate template since SDK 43 and use the plain React Native template, but I found out they actually did it again π I'm sorry. Afaik it's related to some "app delegate extensions/wrappers / react delegates" (the name changed a few times), but I'm not into that deep into the topic yet. Here's some info I found I'll open a PR later, but now a quick patch for diff --git a/node_modules/@react-native-firebase/app/plugin/build/ios/appDelegate.js b/node_modules/@react-native-firebase/app/plugin/build/ios/appDelegate.js
index be0f265..1cabc5b 100644
--- a/node_modules/@react-native-firebase/app/plugin/build/ios/appDelegate.js
+++ b/node_modules/@react-native-firebase/app/plugin/build/ios/appDelegate.js
@@ -9,7 +9,7 @@ const generateCode_1 = require("@expo/config-plugins/build/utils/generateCode");
const fs_1 = __importDefault(require("fs"));
const methodInvocationBlock = `[FIRApp configure];`;
// https://regex101.com/r/Imm3E8/1
-const methodInvocationLineMatcher = /(?:(self\.|_)(\w+)\s?=\s?\[\[UMModuleRegistryAdapter alloc\])|(?:RCTBridge\s?\*\s?(\w+)\s?=\s?\[\[RCTBridge alloc\])/g;
+const methodInvocationLineMatcher = /(?:(self\.|_)(\w+)\s?=\s?\[\[UMModuleRegistryAdapter alloc\])|(?:RCTBridge\s?\*\s?(\w+)\s?=\s?\[(\[RCTBridge alloc\]|self\.reactDelegate))/g;
function modifyObjcAppDelegate(contents) {
// Add import
if (!contents.includes('@import Firebase;')) { I put a patch-package.zip for the diff above, but feel free to test patch generated from my fix PR: https://github.com/invertase/react-native-firebase/actions/runs/1582563137 - I'll have a feedback if the fix is working properly The Regex101 proof: https://regex101.com/r/mPgaq6/1 |
Cool thanks for the quick PR, Iβll try this! |
I'm going to issue it as a release in just a moment, so it'll be out on npmjs.com - I have a different bugfix I just merged that I want to get out there |
Hey guys, following up here. I am having a slight issue with React Native Firebase with Expo SDK 44. When I call However, when I build the app on Expo's hosted EAS service, calling this function results in the app crashing. To summarize: if I do @barthap does anything come to mind for why this might be? Do you expect it to be on the Expo side, or perhaps on the plugin side? I tried running Thank you so much! |
I doubt that it's related to the config plugin in any way. It looks more like an EAS issue. Do you have any logs from that crash? |
I was thinking the same thing. Crash === crash trace needed for diagnosis |
Yeah my instinct says the same. Sorry for the silly question, but what's the easiest way to get shareable crash logs? Is it logcat? (I'm not used to native debugging; I usually use the Console app.) Thank you both! |
Plug device in to computer, Console.app should show the device and from there you will have the ability to see device logs I believe Also of note but unrelated, react-native 0.68 is releasing shortly and the react-native template has moved to |
I found the cause of the crash, which led me to #2346: I think I incorrectly followed the Firebase captcha instructions. I'm not sure why this only crashed on the EAS build though. Captcha worked fine on my simulator from local builds. The Firebase captcha instructions require editing fields inside of XCode, using a field from the Thank you so much for the lead on checking out the crash logs, and for responding quick in general. The moral support helps. |
Related: #5787 |
@nandorojo if this (the captcha info.plist expo command to define the url scheme) could use a pointer in the docs a PR would be awesome π π |
I am trying a solution via Expo config plugin. If it works, I think we should add it to the My attempted solutionimport { withInfoPlist } from '@expo/config-plugins'
import plist from 'plist'
import fs from 'fs'
const googleServicesPlist = fs.readFileSync(
path.resolve(__dirname, googleServicesFile),
'utf8'
)
const googleServicesJson = plist.parse(googleServicesPlist)
const { REVERSED_CLIENT_ID } = googleServicesJson
export default {
plugins: [
[
withInfoPlist,
(config) => {
if (!config.modResults) {
config.modResults = {}
}
if (!config.modResults.CFBundleURLTypes) {
config.modResults.CFBundleURLTypes = []
}
const hasReverseClientId = config.modResults.CFBundleURLTypes.some(
(urlType) => urlType.CFBundleURLSchemes.includes(REVERSED_CLIENT_ID)
)
if (!hasReverseClientId) {
config.modResults.CFBundleURLTypes.push({
CFBundleURLSchemes: [REVERSED_CLIENT_ID]
})
}
return config
}
]
]
} If this works, we should add it to Possible APIs1. Add an option in the config pluginexport default {
plugins: [['@react-native-firebase/app', { enableCaptcha: true }]]
} 2. Create a separate
|
A lot of people use app without auth, auth config / functionality should not be included in app |
Nitpick regarding your plugin code - you might want to read the location of your Google services file from |
Definitely β this is just in my own app's code to see if it works for now |
That fixed it! Should I open a PR for an |
Sure - I wish Expo support were frictionless (that is: required no maintenance at all or config plugins or anything) but until then, this can help people. Thanks! |
Yeah I hear you. I personally like that it lets me encapsulate all the native config steps using JS APIs. Opened a PR at #6167 |
Issue
If you upgrade to Expo SDK 44, and run
expo prebuild -p ios --clean
, you get this error:Click to view error
I'm omitting most other fields in this issue, since they don't apply to Expo projects. But let me know if you need any more info.
It was working before upgrading to SDK 44. It seems like the regex doesn't match anymore. I'm not sure, since I don't exactly know what to look for in native files.
Project Files
Javascript
Click To Expand
package.json
:firebase.json
for react-native-firebase v6:# N/A
iOS
Click To Expand
ios/Podfile
:# N/A
AppDelegate.m
:// N/A
Environment
Click To Expand
react-native info
output:react-native-firebase
version you're using that has this issue:13.1.0
Firebase
module(s) you're using that has the issue:basic installation
TypeScript
?4.4.4
React Native Firebase
andInvertase
on Twitter for updates on the library.The text was updated successfully, but these errors were encountered: