Skip to content

disable event loop#44169

Closed
sammy-SC wants to merge 1 commit into
facebook:mainfrom
sammy-SC:export-D56355863
Closed

disable event loop#44169
sammy-SC wants to merge 1 commit into
facebook:mainfrom
sammy-SC:export-D56355863

Conversation

@sammy-SC

Copy link
Copy Markdown
Contributor

Summary:

Changelog:

[General][Changed] Disable new event loop behavior when bridgeless (new architecture) is enabled.

What is the problem

With event loop, specifically with batchRenderingUpdatesInEventLoop, prop change is not delivered to Android mounting layer if the prop change was initiated from state update inside of useLayoutEffect, componentDidMount or componentDidUpdate. Note this has to be a prop change affecting mounting layer directly, not something consumed by Yoga, e.g. background colour or border colour.

This affects android only.

Minimal repro :

import React, {useLayoutEffect, useState} from 'react';
import {Button, SafeAreaView, View} from 'react-native';
function Foo() {
  const [bgColor, setBgColor] = React.useState('red');
  useLayoutEffect(() => {
    console.log('useLayoutEffect');
    setBgColor('blue');
  }, []);
  return (
    <View
      style={{
        backgroundColor: bgColor,
        width: '100%',
        height: '100%',
      }}
    />
  );
}
function RNTesterApp() {
  const [show, setShow] = useState(false);
  return (
    <SafeAreaView>
      <Button title="Toggle" onPress={() => setShow(!show)} />
      {show && <Foo />}
    </SafeAreaView>
  );
}
export default RNTesterApp;

The underlaying problem

The problem is in batched rendering updates and how props are delivered to Android mounting layer.

Here is a step by step what happens in the repro above:

  1. React issues asks Fabric to create new shadow node A with background colour red.
  2. Fabric asks Android to allocate a view for shadow node A with background colour red.
  3. React commits tree T1 and calls layout effects. Meanwhile Fabric waits, without trying to mount the tree T1, to prevent painting state that is about to be updated and prevent flickering.
  4. React clones node A, changing the background colour to blue and commits the new tree T2.
  5. Fabric, will now go ahead and mount the latest tree T2. While creating mount instructions, it will drop prop updates because it believes prop updates where delivered already as part of step 2.

At first this might appear as a problem with view preallocation. But the underlaying trouble is that on Android, we currently have no way of knowing how to combine changesets from React into single folly::dynamic.

Differential Revision: D56355863

Summary:
## Changelog: 
[General][Changed] Disable new event loop behavior when bridgeless (new architecture) is enabled.

# What is the problem
With event loop, specifically with `batchRenderingUpdatesInEventLoop`, prop change is not delivered to Android mounting layer if the prop change was initiated from state update inside of `useLayoutEffect`, `componentDidMount` or `componentDidUpdate`.  Note this has to be a prop change affecting mounting layer directly, not something consumed by Yoga, e.g. background colour or border colour.

This affects android only.

Minimal repro :
```
import React, {useLayoutEffect, useState} from 'react';
import {Button, SafeAreaView, View} from 'react-native';
function Foo() {
  const [bgColor, setBgColor] = React.useState('red');
  useLayoutEffect(() => {
    console.log('useLayoutEffect');
    setBgColor('blue');
  }, []);
  return (
    <View
      style={{
        backgroundColor: bgColor,
        width: '100%',
        height: '100%',
      }}
    />
  );
}
function RNTesterApp() {
  const [show, setShow] = useState(false);
  return (
    <SafeAreaView>
      <Button title="Toggle" onPress={() => setShow(!show)} />
      {show && <Foo />}
    </SafeAreaView>
  );
}
export default RNTesterApp;
```

# The underlaying problem
The problem is in batched rendering updates and how props are delivered to Android mounting layer.

Here is a step by step what happens in the repro above:
1. React issues asks Fabric to create new shadow node A with background colour **red**.
2. Fabric asks Android to allocate a view for shadow node A with background colour **red**.
3. React commits tree **T1** and calls layout effects. Meanwhile Fabric waits, without trying to mount the tree **T1**, to prevent painting state that is about to be updated and prevent flickering.
4. React clones node A, changing the background colour to **blue** and commits the new tree **T2**.
5. Fabric, will now go ahead and mount the latest tree **T2**. While creating mount instructions, it will drop prop updates because it believes prop updates where delivered already as part of step 2. 


At first this might appear as a problem with view preallocation. But the underlaying trouble is that on Android, we currently have no way of knowing how to combine changesets from React into single folly::dynamic.

Differential Revision: D56355863
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 19, 2024
@facebook-github-bot

Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D56355863

@analysis-bot

Copy link
Copy Markdown
Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 19,410,749 +305
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 22,784,269 +142
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: c0ea79d
Branch: main

@facebook-github-bot

Copy link
Copy Markdown
Contributor

This pull request has been merged in cf75d03.

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Apr 19, 2024
@github-actions

Copy link
Copy Markdown

This pull request was successfully merged by @sammy-SC in cf75d03.

When will my fix make it into a release? | How to file a pick request?

@sammy-SC sammy-SC deleted the export-D56355863 branch April 22, 2024 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged This PR has been merged. p: Facebook Partner: Facebook Partner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants