facebook / react-native

A framework for building native applications using React
https://reactnative.dev
MIT License
119.45k stars 24.37k forks source link

Transaction merging can cause `CREATE` mutations to be effectively dropped #47960

Open j-piasecki opened 2 hours ago

j-piasecki commented 2 hours ago

Description

Related to https://github.com/facebook/react-native/issues/38743

Transaction merging introduced in https://github.com/facebook/react-native/pull/44188 may cause CREATE mutations to be effectively dropped.

When two transactions, where the first one contains a DELETE mutation for a view with a view tag X and the other contains a CREATE mutation for a view with a view tag X, get merged the result is a transaction containing both mutations. When that transaction is executed, the DELETE operations are run after CREATE operations, so the view will not exist after the transaction finishes which results in the host tree diverging from the shadow tree.

Inside FabricUIManagerBinding pendingTransactions_ field tracks transactions to be executed but only one per surface: https://github.com/facebook/react-native/blob/a6b2355b8d559aba2f1f13394c91f563f9b9e1a4/packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricUIManagerBinding.h#L164-L166

Inside schedulerDidFinishTransaction transactions are merged when a transaction for a given surface is already pending: https://github.com/facebook/react-native/blob/a6b2355b8d559aba2f1f13394c91f563f9b9e1a4/packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricUIManagerBinding.cpp#L546-L550

If instead of merging transactions into one, multiple ones could be queued for a single surface and executed in order, the issue would be fixed. I can open a PR with that change but before that, I'd like to know whether that could have any unwanted side effects, as I'm lacking context on why merging was chosen there.

It also seems like this may be a known problem: https://github.com/facebook/react-native/blob/69356b7f2144283fa6be7e9998e25a0681e159f0/packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricMountingManager.cpp#L828-L829

Steps to reproduce

  1. Open the application (the code may be pasted to RNTester)
  2. Click the Click this first button
  3. Click the Click this second button

React Native Version

0.76.2

Affected Platforms

Runtime - Android

Areas

Fabric - The New Renderer

Output of npx react-native info

```text System: OS: macOS 14.7.1 CPU: (12) arm64 Apple M3 Pro Memory: 96.86 MB / 18.00 GB Shell: version: "5.9" path: /bin/zsh Binaries: Node: version: 20.18.0 path: ~/.nvm/versions/node/v20.18.0/bin/node Yarn: version: 1.22.19 path: /opt/homebrew/bin/yarn npm: version: 10.8.2 path: ~/.nvm/versions/node/v20.18.0/bin/npm Watchman: version: 2024.09.09.00 path: /opt/homebrew/bin/watchman Managers: CocoaPods: version: 1.15.2 path: /Users/jakubpiasecki/.rvm/gems/ruby-2.7.5/bin/pod SDKs: iOS SDK: Platforms: - DriverKit 23.5 - iOS 17.5 - macOS 14.5 - tvOS 17.5 - visionOS 1.2 - watchOS 10.5 Android SDK: API Levels: - "24" - "26" - "28" - "29" - "30" - "31" - "32" - "33" - "34" - "35" Build Tools: - 26.0.3 - 28.0.3 - 29.0.2 - 29.0.3 - 30.0.2 - 30.0.3 - 31.0.0 - 32.0.0 - 32.1.0 - 33.0.0 - 33.0.1 - 34.0.0 - 35.0.0 - 35.0.0 System Images: - android-28 | Google ARM64-V8a Play ARM 64 v8a - android-33 | Google APIs ARM 64 v8a - android-34 | Google Play ARM 64 v8a - android-35 | Google Play ARM 64 v8a Android NDK: Not Found IDEs: Android Studio: 2024.2 AI-242.23339.11.2421.12550806 Xcode: version: 15.4/15F31d path: /usr/bin/xcodebuild Languages: Java: version: 17.0.2 path: /usr/bin/javac Ruby: version: 2.7.5 path: /Users/jakubpiasecki/.rvm/rubies/ruby-2.7.5/bin/ruby npmPackages: "@react-native-community/cli": Not Found react: Not Found react-native: Not Found react-native-macos: Not Found npmGlobalPackages: "*react-native*": Not Found Android: hermesEnabled: false newArchEnabled: false iOS: hermesEnabled: true newArchEnabled: true ```

Stacktrace or Logs

Exception in native call
com.facebook.react.bridge.RetryableMountingLayerException: Unable to find viewState for tag 4. Surface stopped: false
    at com.facebook.react.fabric.mounting.SurfaceMountingManager.getViewState(SurfaceMountingManager.java:1103)
    at com.facebook.react.fabric.mounting.SurfaceMountingManager.updateProps(SurfaceMountingManager.java:687)
    at com.facebook.react.fabric.mounting.mountitems.IntBufferBatchMountItem.execute(IntBufferBatchMountItem.java:157)
    at com.facebook.react.fabric.mounting.MountItemDispatcher.executeOrEnqueue(MountItemDispatcher.java:370)
    at com.facebook.react.fabric.mounting.MountItemDispatcher.dispatchMountItems(MountItemDispatcher.java:265)
    at com.facebook.react.fabric.mounting.MountItemDispatcher.tryDispatchMountItems(MountItemDispatcher.java:122)
    at com.facebook.react.fabric.FabricUIManager$DispatchUIFrameCallback.doFrameGuarded(FabricUIManager.java:1392)
    at com.facebook.react.fabric.GuardedFrameCallback.doFrame(GuardedFrameCallback.kt:22)
    at com.facebook.react.modules.core.ReactChoreographer.frameCallback$lambda$1(ReactChoreographer.kt:59)
    at com.facebook.react.modules.core.ReactChoreographer.$r8$lambda$nSkFhrr5T7rop_XKwzlLov4NLLw(Unknown Source:0)
    at com.facebook.react.modules.core.ReactChoreographer$$ExternalSyntheticLambda0.doFrame(D8$$SyntheticClass:0)
    at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1404)
    at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1415)
    at android.view.Choreographer.doCallbacks(Choreographer.java:1015)
    at android.view.Choreographer.doFrame(Choreographer.java:941)
    at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:1389)
    at android.os.Handler.handleCallback(Handler.java:959)
    at android.os.Handler.dispatchMessage(Handler.java:100)
    at android.os.Looper.loopOnce(Looper.java:232)
    at android.os.Looper.loop(Looper.java:317)
    at android.app.ActivityThread.main(ActivityThread.java:8699)
    at java.lang.reflect.Method.invoke(Native Method)
    at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:580)
    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:886)

Reproducer

https://snack.expo.dev/@jpiasecki/unable_to_find_viewstate_repro

Screenshots and Videos

https://github.com/user-attachments/assets/7df6bc47-a741-48cc-85e3-398239415b11

react-native-bot commented 2 hours ago

[!TIP] Newer version available: You are on a supported minor version, but it looks like there's a newer patch available - 0.76.3. Please upgrade to the highest patch for your minor or latest and verify if the issue persists (alternatively, create a new project and repro the issue in it). If it does not repro, please let us know so we can close out this issue. This helps us ensure we are looking at issues that still exist in the most recent releases.

react-native-bot commented 2 hours ago

[!TIP] Newer version available: You are on a supported minor version, but it looks like there's a newer patch available - undefined. Please upgrade to the highest patch for your minor or latest and verify if the issue persists (alternatively, create a new project and repro the issue in it). If it does not repro, please let us know so we can close out this issue. This helps us ensure we are looking at issues that still exist in the most recent releases.