fixed SDK issue while uploading app in debug scheme

[arinjay]

Summary

Problem - Error when trying to publish to Apple Store in debug scheme

Error thread - #31507

Changelog

[iOS][Changed] - The diffs renames the required variable which was causing conflicts in names with Apple core SDK's

[facebook-github-bot]

Hi @arinjay!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

[pull-bot]

	Messages
📖	📋 Missing Test Plan - Can you add a Test Plan? To do so, add a "## Test Plan" section to your PR description. A Test Plan lets us know how these changes were tested.

Generated by 🚫 dangerJS against 6cd06c6

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
ios	-	universal	n/a	--

Base commit: ed46ea2
Branch: main

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	8,188,328	+415,607
android	hermes	armeabi-v7a	7,788,450	+610,516
android	hermes	x86	8,558,277	+476,782
android	hermes	x86_64	8,511,318	+449,549
android	jsc	arm64-v8a	9,857,663	+212,091
android	jsc	armeabi-v7a	8,842,331	+422,770
android	jsc	x86	9,823,706	+228,766
android	jsc	x86_64	10,420,886	+228,480

Base commit: ed46ea2
Branch: main

[facebook-github-bot]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

[facebook-github-bot]

@sshic has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[arinjay]

@lunaleaps can you help me in getting this MR merged

[kelset]

@cortinico & @fortmarek this PR addressed the fact that if you try to upload to Apple's store a debug build, there seems to be an overlap of methods' names with the ones from the core iOS SDK. By changing them, it seems to address the problem - so I think overall it'd be a good change to integrate.
My only concern is that we might have to consider it a breaking change? I don't think we need to merge this for 0.69, but the issue has been open since May 2021 so it'd be good to get this sorted at some point.

(cc @tido64, do you have any opinions on this?)

[fortmarek]

My only concern is that we might have to consider it a breaking change?

The properties changed are with _ prefix and thus should be considered private. The only change which could be imho breaking is renaming of handleNotification to handlePackageNotification. Are we sure this is something we consider part of the public API? If so, it should be a breaking change, indeed.

I don't think there's a particular reason not to merge this change for 0.69.

[cortinico]

Looping @cipolleschi in here.

My only concern is that we might have to consider it a breaking change? I don't think we need to merge this for 0.69, but the issue has been open since May 2021 so it'd be good to get this sorted at some point.

IMHO we're fine merging this on main + mark it as a breaking change in the README.

I don't think there's a particular reason not to merge this change for 0.69.

Given the limited use case ("trying to publish to Apple Store in debug scheme") this is not pick-worthy for .69 imho.

[aakashchoubey]

Given the limited use case ("trying to publish to Apple Store in debug scheme") this is not pick-worthy for .69 imho.

Just curious about the impact of this breaking change here. From what I understand, since iOS doesn't allow users to install apps as Android does with apks, Testflight is the only available option to upload an iOS build and test for non-ios developers.

Not supporting that is bad dev-exp right?

[cipolleschi]

Thanks @cortinico. I agree, it is not pick worthy.

I'm also not so sure about it being breaking: is the RCTPackagerClient class supposed to be accessed and used by clients or should it be an internal details? In other terms, is it a private protocol or not? If the answer is yes, it definitely isn't a breaking change.

@choubeyaakash77, more or less yes, although I don't remember if we can actually submit something in Debug mode nor whether it is a good practice. By the way, the PR has been open since May 2021, as @kelset commented, so it is a year that this is broken. Waiting 1 more release won't hurt more than how it is already doing.

What do you think?

[aakashchoubey]

Waiting 1 more release won't hurt more than how it is already doing.

@cipolleschi Yeah it makes sense. If it is not a breaking change, there should be no problem in including it in 0.69, but if there is a breaking change, then that's a call on the react-native team I guess.

submit something in Debug mode nor whether it is a good practice

Submitting on TestFlight is different from submitting on AppStore. I am not sure but I don't think submitting something in Debug mode is a bad practice. That's a limitation given to us by Apple.
There is a valid use case imo to float around an app of a different variant than Release, to some internal users. There are many reasons for doing that - an app for backend developers, adding specific options for variants that won't be available in a release app, etc. One can potentially achieve the same by maintaining different branches/tags but that brings an overhead of maintaining multiple codebases. And this is fairly common in Android to make things available by app's variant.

[cipolleschi]

Yeah, I see all the points and I agree with them.

What I was trying to say, failing, is that, if I remember well:

• submitting an app to the App Store is one thing, and IIRC it must be built for release.
• submitting an app for TestFlight should follow a different path and it can be sent in Debug mode.

So these warnings looks a little bit weird to me.

But again, it has a been a while since I had to submit something to Apple, and I was using an Enterprise profile to distribute the app internally in other context. So, I could remember wrong.

[cortinico]

@arinjay Don't you mind updating the changelog entry to mention this is a breaking change?

[arinjay]

@cortinico I can't see how it's a breaking change?

As per my understanding of the code, RCTPackagerClient is supposed to be an internal protocol used by SDK itself and not directly by the client.
So updating the method definition both at the protocol and implementation level can't break it on the client-side.

[arinjay]

@cipolleschi yes you’re right

1. submitting an app to the App Store (release mode) in one thing
2. submitting an app for TestFlight(debug mode) is separate.

Schemes are different on iOS apps.

The need comes from the very limitation Apple has in the first place i.e. not being able to install IPA directly onto any iOS device.
This generally hampers the development cycle and the majority of folks have to resolve to release apps in debug mode on a test flight as not all end users have machines with Xcode.

Sharing it with multiple end-users and then gathering bugs becomes very easier. The whole iOS development cycle becomes more efficient.

[Tak-As]

When will this PR be merged?
I am facing this issue and am waiting for a merge.
I appreciate your help with moving this issue ahead.

[cortinico]

When will this PR be merged?
I am facing this issue and am waiting for a merge.
I appreciate your help with moving this issue ahead.

Just to clarify, this won't be included in RN 0.69. The earliest this will be released in RN 0.70 as it's not considered a regression.

[react-native-bot]

This pull request was successfully merged by @arinjay in 086c13d.

^{When will my fix make it into a release? | Upcoming Releases}

[javache]

I don't believe this is correct to fix and will revert this. This code is internal only and therefore can use private API's. You should never upload debug scheme builds to the App Store.

[javache]

@cipolleschi and I look at an alternative way to address the issue in #31507. For now this breaks some development workflows, so we prefer to back this out.

[facebook-github-bot]

This pull request has been reverted by 6fcb878.

[arinjay]

@javache what's the alternate way of resolving this? Can you mention that

Also "You should never upload debug scheme builds to the App Store"
This is correct
but we didn't upload it to AppStore, it failed to upload on the testFlight (these 2 are different mediums to publish app), even without the above MR changes.

Can you mention the changes using which we can still upload the app on testflight in debug mode?

[javache]

You should never upload debug scheme builds to any distribution platform. They are inherently insecure as they allow any local packager instance to completely redefine your application's behaviour, while still providing access to privileged local storage (eg keychain).

[arinjay]

'You should never upload debug scheme builds to any distribution platform'
Why is that?
Apple made testfight for this purpose only. People to whom the app is made available are aware of the access they will be sharing when downloading the app from testflight. Apple made sure to list out all potential risks before setting up test flight apps.

Use case in which debug mode is required - An app running both on react native and native (swift) codebase. now in debug mode we need to have one API call debugger library or profiler library which we only want in debug mode only, and installed dependencies in debug mode only. Post putting it on test flight to share it with other user, which in real-time can track the profiler data on actual devices or detect which API was fired.

These cases are very much required in many day-to-day app developments.

[gabriellend]

@javache @arinjay Hi! Not sure this is still an ongoing conversation as the last reply was in June but I don't think it was implemented as I don't see it in the tags. I wanted to throw my two cents in as I can attest to the importance of being able to upload a debug scheme to TestFlight for internal testing before we submit a release version to the App Store. We've been doing it this way for years at my job (we have a staging environment and a production environment, so the debug scheme works off of staging and the release scheme works off production) and we now can't submit to TestFlight because of running into the selectors mentioned in this post. I've been in talks with Apple and they just got back to me directing me to manually change these myself so that the upload will be accepted to TestFlight. I want to voice my support for including this in the package. Thanks!

[rolfen]

@javache Whether you should upload debug scheme builds to the App Store or not is outside RN's scope.

This error message suggests an issue, either in the RN code or in Apple's validation.