RN 77 Upgrade #8018

kochavi-daniel · 2025-04-30T11:48:51Z

No description provided.

…dCommandCompletionWithPushedComponentId

…nt lifecycle methods for new architecture

…ctView

# Conflicts: # package-lock.json # scripts/start.js

* Implemented turbo modules for android * Implemented missing method * Complete Android Turbo module implementation * Complete Android Turbo module implementation * Undo some unneeded fixes * Fixed build on ios

(cherry picked from commit c8a2fad)

This reverts commit 497ebd6.

* WIP * Update package-lock.json * Fixed ios build * Fixed unit tests * Fixed ios build * Fixed ios build * Revert "Fixed ios build" This reverts commit 3dbd98a. * Fixed ios build * fix tests for ios * Fixed eslint * DO NOT RELEASE THIS COMMIT! * Revert "DO NOT RELEASE THIS COMMIT!" This reverts commit b0e840e. * Fixed build for ios 77

The problem was that when setting a titleView of a navigationItem the normal life cycle flow of a view will not be called so the RNNReactView componentWillAppear and DidAppear need to be implicitly called. This was somehow removed from the TopBarTitlePresenter.mm unit but is present on the current Master branch's file. This PR restores those two lines and also actives the Unit test for that module.

Copilot

Pull Request Overview

This PR upgrades the React Native Navigation module in preparation for RN 77, introducing a new TurboModule and updating module registration, test timing, native build settings, and linting configurations.

Add NavigationTurboModule implementation for TurboModules.
Update NavigationPackage to register the TurboModule and adjust module info.
Refactor end-to-end tests and adjust podspec and ESLint configurations.

Reviewed Changes

Copilot reviewed 359 out of 365 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
lib/android/app/src/main/java/com/reactnativenavigation/react/NavigationTurboModule.kt	New TurboModule implementation with lifecycle handling and navigation command support.
lib/android/app/src/main/java/com/reactnativenavigation/react/NavigationPackage.kt	Updated registration to include the TurboModule and new module info provider.
index.e2e.js	Minor import and LogBox setup adjustments.
e2e/StaticLifecycleEvents.test.js	Removed an outdated expectation for static lifecycle events.
e2e/Stack.test.js	Improved test stability with waitFor timeouts.
e2e/Modals.test.js	Updated modal test assertions to use waitFor with timeouts.
ReactNativeNavigation.podspec	Upgraded C++ standard and adjusted dependencies and compiler flags.
.eslintrc.js	Updated ESLint extend configuration for React Native.

Files not reviewed (6)

autolink/postlink/snapshots/appDelegateLinker.test.js.snap: Language not supported
lib/android/app/build.gradle: Language not supported
lib/android/build.gradle: Language not supported
lib/ios/AnimatedImageView.mm: Language not supported
lib/ios/AnimatedReactView.mm: Language not supported
lib/ios/AnimatedTextView.mm: Language not supported

Comments suppressed due to low confidence (1)

lib/android/app/src/main/java/com/reactnativenavigation/react/NavigationPackage.kt:30

[nitpick] Consider using the fully qualified class name (e.g. NavigationTurboModule::class.java.name) for the className field in ReactModuleInfo for improved clarity and consistency.

className = NavigationTurboModule.NAME,

Copilot · 2025-05-04T10:57:29Z

lib/android/app/src/main/java/com/reactnativenavigation/react/NavigationTurboModule.kt

+    ) {
+        handle {
+            val _children = ArrayList<ViewController<*>>()
+            for (i in 0..<children.size()) {


The '0..<children.size()' syntax is not a standard Kotlin range operator; consider replacing it with '0 until children.size()' to ensure correct iteration.

Suggested change

for (i in 0..<children.size()) {

for (i in 0 until children.size()) {

markdevocht · 2025-05-06T07:36:04Z

lib/ios/AnimatedTextView.mm

@@ -24,7 +45,6 @@ - (void)layoutSubviews {

 - (void)reset {
    [super reset];
-    _fromTextContainer.size = _fromSize;


Is it not necessary to reset the _fromTextContainer size?

markdevocht · 2025-05-06T07:40:07Z

lib/ios/RNNCommandsHandler.mm

@@ -89,7 +90,7 @@ - (void)setRoot:(NSDictionary *)layout
    [_layoutManager addPendingViewController:vc];

    RNNNavigationOptions *optionsWithDefault = vc.resolveOptionsWithDefault;
-    vc.waitForRender = [optionsWithDefault.animations.setRoot.waitForRender withDefault:NO];
+    vc.waitForRender = [optionsWithDefault.animations.setRoot.waitForRender withDefault: [RNNUtils getDefaultWaitForRender]];


In all the parts where the [... withDefault: NO] is replaced with [RNNUtils getDefaultWaitForRender], what is the reason for this?

markdevocht · 2025-05-06T07:47:27Z

lib/ios/RNNComponentViewController.mm

@@ -77,10 +77,6 @@ - (void)render {
    [self renderReactViewIfNeeded];
 }

- (void)destroyReactView {


Can you explain to me why there is no need to invalidate the reactView?

markdevocht · 2025-05-06T07:48:28Z

lib/ios/RNNCustomTitleView.mm

@@ -53,6 +53,11 @@ - (NSString *)alignment {
    return _alignment ? _alignment : @"center";
 }

+#ifdef RCT_NEW_ARCH_ENABLED
+- (void)surface:(RCTSurface *)surface didChangeIntrinsicSize:(CGSize)intrinsicSize {
+	// TODO: FILL WITH DATA


This part is still a TODO

markdevocht · 2025-05-06T07:51:03Z

lib/ios/RNNUtils.mm

@@ -2,6 +2,14 @@

 @implementation RNNUtils

+ (BOOL)getDefaultWaitForRender {
+#ifdef RCT_NEW_ARCH_ENABLED
+	return YES;


Why do we need to return YES for NEW_ARCH?

markdevocht

@kochavi-daniel , I left some comments to get to know the code better and the reasoning behind it.

gosha212 · 2025-05-04T10:53:46Z

e2e/StaticLifecycleEvents.test.js

@@ -10,7 +10,6 @@ describe('static lifecycle events', () => {
    await elementById(TestIDs.SHOW_STATIC_EVENTS_SCREEN).tap();
    await elementById(TestIDs.STATIC_EVENTS_OVERLAY_BTN).tap();
    await expect(elementByLabel('Static Lifecycle Events Overlay')).toBeVisible();
-    await expect(elementByLabel('componentWillAppear | EventsOverlay | Component')).toBeVisible();


Why did you remove it?

gosha212 · 2025-05-04T11:01:15Z

playground/src/screens/StackCommandsScreen.tsx

      })
+      .then((popId) => new Promise<string>((resolve) => setTimeout(() => resolve(popId), 500)))


Why do we have a timeout here? Is it a problem with detox?

gosha212 · 2025-05-04T11:02:16Z

playground/src/app.ts

-    Navigation.dismissAllModals();
-    setRoot();
-  });
+  Navigation.dismissAllModals();


This doesn't feel right? Does this callback work?

e2e/Modals.test.js

gosha212 · 2025-05-05T13:33:49Z

lib/ios/RNNBridgeEventEmitter.mm

Do we need this class? Can we remove it?

gosha212 · 2025-05-06T11:09:06Z

lib/ios/TransitionOptions.mm

@@ -54,7 +55,7 @@ - (BOOL)hasValue {
 }

 - (BOOL)shouldWaitForRender {
-    return [self.waitForRender withDefault:NO] || self.hasAnimation;
+	return [self.waitForRender withDefault:[RNNUtils getDefaultWaitForRender]] || self.hasAnimation;


Why did it changed for no to yes?

gosha212 · 2025-05-06T11:13:56Z

lib/ios/RNNUIBarButtonItem.mm

+	self.widthConstraint.constant = intrinsicSize.width;
+	self.heightConstraint.constant = intrinsicSize.height;
+	[surface setSize:intrinsicSize];
+	//[rootView setNeedsUpdateConstraints];


Do we need it? what it does?

gosha212 · 2025-05-06T11:14:59Z

lib/ios/ReactNativeNavigation.h


 + (NSArray<id<RCTBridgeModule>> *)extraModulesForBridge:(RCTBridge *)bridge;

+ (RCTBridge *)getBridge;


We don't need it anymore

gosha212 · 2025-05-06T11:16:25Z

lib/ios/ReactNativeNavigation.h


-+ (void)bootstrapWithDelegate:(id<RCTBridgeDelegate>)bridgeDelegate


We still need something similar for host

kochavi-daniel and others added 30 commits February 25, 2025 14:38

Merge branch 'master' into daniel/rn77-ios-new-arch

3ceabfe

auto renaming

5d717d0

changes

4928798

native fixes - passes build with manual change in RCTAppSetupUtils.h

23510df

all works still with change in RCTAppSetupUtils

3b4187d

loads the menu - playground works

54b6c73

fixed event receivers

d68c0c0

hermes falg

0c99bf7

fix side menu pres test

4a6ac23

test ot .mm

18f7953

comment out test that fail linkage

4f6b723

fix testSetRoot_withAnimation and testPush_shouldResolvePromiseAndSen…

2ad25c5

…dCommandCompletionWithPushedComponentId

fix linking by a hack

1027229

remove invalidation

f6d5ff7

fix Stub in Turbo Module

afa721c

Fix StaticLifecycleEvents tests

b1e2391

Delay will appear and did appear when component not already mounted

73eedbc

Update RNNReactView to use RCTSurfacePresenterStub and adjust compone…

5c00a8d

…nt lifecycle methods for new architecture

Better implementation

3e2223a

Fix buttons

4cfc980

Fix button order test

f4be349

Another buttons fix

54bef68

Refactor component lifecycle methods in RNNReactButtonView and RNNRea…

986a9b9

…ctView

fix snapshot c++ error

d7286c6

fix snapshot refactor mm

41bbb15

fix snapshot c code

e1f2a87

view controller factory

b412677

moved turbo module spec to async

429450c

codegen type

e733ae0

cleanup native sender

b206437

kochavi-daniel and others added 17 commits April 2, 2025 09:37

adjust interface

3ffb4fc

fix remaining e2e

92a182b

another test bites the dust

56152f3

Disable failing E2E, failing snapshot and Turbo Modules on Android

129a413

comment ios test out

52d5732

wrong test

fca1e8d

Fixed android screenshot

bb8cae9

Update package.json version to 8.1.0-alpha01 [buildkite skip]

a7e5cf2

Update package.json version to 8.1.0-alpha [buildkite skip]

f5bd327

Merge branch 'feat/rn77-android-newarch' into daniel/rn77-ios-new-arch

9930d6e

# Conflicts: # package-lock.json # scripts/start.js

Merge branch 'daniel/rn77-ios-new-arch' into daniel/rn77-preview

84913d7

Android Turbo Modules (#7999)

27f2658

* Implemented turbo modules for android * Implemented missing method * Complete Android Turbo module implementation * Complete Android Turbo module implementation * Undo some unneeded fixes * Fixed build on ios

WIP

497ebd6

(cherry picked from commit c8a2fad)

Revert "WIP"

11b1e59

This reverts commit 497ebd6.

Fixed android turbo modules generation and autolinking (#8006)

3120643

gosha212 requested review from markdevocht and gosha212 May 4, 2025 07:50

markdevocht self-assigned this May 4, 2025

gosha212 added the type: accepted/enhancement label May 4, 2025

gosha212 requested a review from Copilot May 4, 2025 10:56

Copilot AI reviewed May 4, 2025

View reviewed changes

markdevocht reviewed May 6, 2025

View reviewed changes

gosha212 reviewed May 7, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RN 77 Upgrade #8018

RN 77 Upgrade #8018

kochavi-daniel commented Apr 30, 2025

Copilot AI left a comment

Copilot AI May 4, 2025

markdevocht May 6, 2025

markdevocht May 6, 2025

markdevocht May 6, 2025

markdevocht May 6, 2025

markdevocht May 6, 2025

markdevocht left a comment

gosha212 May 4, 2025

gosha212 May 4, 2025

gosha212 May 4, 2025

gosha212 May 5, 2025

gosha212 May 6, 2025

gosha212 May 6, 2025

gosha212 May 6, 2025

gosha212 May 6, 2025

	for (i in 0..<children.size()) {
	for (i in 0 until children.size()) {

		})
		.then((popId) => new Promise<string>((resolve) => setTimeout(() => resolve(popId), 500)))


		+ (NSArray<id<RCTBridgeModule>> )extraModulesForBridge:(RCTBridge )bridge;

		+ (RCTBridge *)getBridge;


		+ (void)bootstrapWithDelegate:(id<RCTBridgeDelegate>)bridgeDelegate

RN 77 Upgrade #8018

Are you sure you want to change the base?

RN 77 Upgrade #8018

Conversation

kochavi-daniel commented Apr 30, 2025

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Copilot AI May 4, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markdevocht left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment