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

Replace deprecated method for reloading bundle on iOS #2784

Merged
merged 1 commit into from
Dec 12, 2024

Conversation

esbenvb
Copy link
Contributor

@esbenvb esbenvb commented Nov 26, 2024

The code is currently using a deprecated method to reload the bundle. This PR will switch to the recommended method of using RCTTriggerReloadCommandListeners.

When using New Architecture, the iOS app does not restart properly using the deprecated method. (tested in RN 0.76.3, RN CodePush 9.0.0).

It either hangs or crashes. However, the new bundle is installed and loaded properly when the app is killed and restarted.

Switching to the one in this PR, makes the iOS app restart properly and use the new bundle, when New Architecture is enabled, and also works when it's disabled.

@esbenvb esbenvb requested a review from a team as a code owner November 26, 2024 21:59
@DordeDimitrijev
Copy link
Contributor

Hello @esbenvb can you provide more details about these changes. How do these change affect RNCP behavior and why do you need them?

@esbenvb
Copy link
Contributor Author

esbenvb commented Nov 29, 2024

Hello @esbenvb can you provide more details about these changes. How do these change affect RNCP behavior and why do you need them?

I've added a description now, thanks for the reminder

@DordeDimitrijev
Copy link
Contributor

Thank you for updating the description. Since this contribution involves replacement of deprecated method and not actual implementation of changes for new architecture, could you please update the title and description accordingly? Currently it seems we support NA, but that is not the case.

@esbenvb esbenvb changed the title Fix app reloading with New Architecture on iOS Replace deprecated method for reloading bundle on iOS Dec 9, 2024
@esbenvb
Copy link
Contributor Author

esbenvb commented Dec 9, 2024

Could you please update the title and description accordingly? Currently it seems we support NA, but that is not the case.

Done. If possible, I'd like to assist with other feedback regarding NA support, which would make much sense now that it's enabled default, and CodePush can live on as a standalone product.

@DordeDimitrijev
Copy link
Contributor

DordeDimitrijev commented Dec 10, 2024

@esbenvb Do you have a similar solution for android in mind, also could you test the changes with older versions of react native?

@esbenvb
Copy link
Contributor Author

esbenvb commented Dec 11, 2024

@esbenvb Do you have a similar solution for android in mind, also could you test the changes with older versions of react native?

Unfortunately, no. Tried to look into the Android code as well but I have more experience debugging iOS code, and didn't find a solution for reloading the bundle properly on Android with NA.

Regarding older versions, the RCTTriggerReloadCommandListeners call that I'm using here has been around for quite some time, also in older RN versions that's not supported by the v8.0.0 and v9.0.0 versions of react-native-code-push.

@DordeDimitrijev
Copy link
Contributor

Tested and verified the changes. Set up demo app with these changes with and without NA to test out reload of bundle.

@DordeDimitrijev DordeDimitrijev merged commit a8e64fc into microsoft:master Dec 12, 2024
1 check passed
@esbenvb esbenvb deleted the patch-1 branch December 12, 2024 13:36
@DordeDimitrijev DordeDimitrijev mentioned this pull request Dec 19, 2024
@bardliu
Copy link

bardliu commented Jan 12, 2025

@esbenvb Do you have a similar solution for android in mind, also could you test the changes with older versions of react native?

Unfortunately, no. Tried to look into the Android code as well but I have more experience debugging iOS code, and didn't find a solution for reloading the bundle properly on Android with NA.

Regarding older versions, the RCTTriggerReloadCommandListeners call that I'm using here has been around for quite some time, also in older RN versions that's not supported by the v8.0.0 and v9.0.0 versions of react-native-code-push.

https://github.com/facebook/react-native/blob/main/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactDelegate.java#L250

hi~ maybe can use this away

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.

5 participants