chore: handle conflict from firebase #227
Conversation
| function isFileReferencedInXcodeProject(project: any, fileName: string): boolean { | ||
| try { | ||
| // Get all file references | ||
| const fileReferences = project.pbxFileReferenceSection(); |
There was a problem hiding this comment.
I have considered this option initially but I was hesitant to do that route since I don't know how reliable it is to be parsing Xcode project file and if this might fail with Xcode upgrades or things like that? Specially since there are no Expo plugins or mods that allow us to access those details in a structured way.
Or maybe even this file looks different for complicated project structures, at the end of the day, the vast majority of iOS development depend on Xcode updating this file when files are added to the project and it's not done manually
There was a problem hiding this comment.
I have found the xdode package to be very reliable, the approach is actually the most common way to programmatically interact with Xcode projects in the React Native ecosystem. Most tools (including React Native itself in the CLI), fastlane, expo config use similar methods.
While Apple does change the Xcode project format in xcode upgrades, they generally maintain backward compatibility, especially for core features like file references.
And lets say there is a case with very complicated structure out of ordinary, and this check fails, the consequences would be we get a duplicated reference which we were going to get anyway? and if someone has this issue we can suggest the solution of not adding the googleservices file in our config?
There was a problem hiding this comment.
I've recently updated our Xcode project file for APN UIKit to the newest version (available with Xcode 16.x), and our existing project file parsing (in .github workflow/actions) failed.
I would advise testing this with both Xcode 15.x and 16.x project-file versions before merging.
Reference:
- our build failure on
PBXFileSystemSynchronizedRootGroup: https://github.com/customerio/customerio-ios/actions/runs/13946832456/job/39035926170?pr=872 - bug Mahmoud found describing this in more detail:
PBXGroupattempted to initialize an object with unknown ISAPBXFileSystemSynchronizedRootGroupCocoaPods/CocoaPods#12456
There was a problem hiding this comment.
I actually think we should re-think this change, because a similar logic was problematic with Expo warnings for version conflicts. I believe this is could even be more problematic.
Like @uros-mil mentions, seems like Xcode 16 added/changed this file format and we don't wanna keep testing and supporting multiple Xcode project files format
There was a problem hiding this comment.
I didn't analyze this in depth, but my logic is:
- We'll need a solution that is not error-prone
- If we need to look inside the project file (and there is no other way), let's check if anyone has found an approach that is working well with both newer and older versions of the format
| expect(existsInAnyLocation).toBe(true); | ||
| }); | ||
|
|
||
| test("GoogleService-Info.plist file is not duplicated", () => { |
There was a problem hiding this comment.
I think those tests will never fail since our test project doesn't have any Firebase SDKs setup for it, right?
There was a problem hiding this comment.
yes thats correct, created them so when we run these on the branch where you added react native firebase, we can verify it via unit test too.
also, might help even in future when we want to quickly test any other rn-firebase feature while making sure nothing else breaks. But it can be removed if deemed unnecessary.
mahmoud-elmorabea
left a comment
mahmoud-elmorabea
left a comment
There was a problem hiding this comment.
I actually think we should re-think this change, because a similar logic was problematic with Expo warnings for version conflicts. I believe this is could even be more problematic.
Like @uros-mil mentions, seems like Xcode 16 added/changed this file format and we don't wanna keep testing and supporting multiple Xcode project files format
|
Putting this back to draft, so when we plan to bring the expo improvements project again, we can restart the conversation. |
summary:
withGoogleServicesJsonFile.tsto detect and handleGoogleService-Info.plistfiles across multiple locationsproject referenceschanges:
This PR attempts to resolve build errors caused by both our plugin and React Native Firebase trying to add the same GoogleService-Info.plist file.