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

RTL Layout Direction Not Updating after app reload with New React Native Architecture #45661

Closed
serhii-yalla opened this issue Jul 25, 2024 · 47 comments
Labels
Issue: Author Provided Repro This issue can be reproduced in Snack or an attached project. Needs: Author Feedback Resolution: Fixed A PR that fixes this issue has been merged. Type: New Architecture Issues and PRs related to new architecture (Fabric/Turbo Modules)

Comments

@serhii-yalla
Copy link

serhii-yalla commented Jul 25, 2024

Description

When using I18nManager to switch the layout direction between RTL (Right-to-Left) and LTR (Left-to-Right), the changes do not reflect after reload on the UI(doesn't force styles), but apply for animations. This issue appears specifically with the new React Native architecture. The styles and layout directions are only applied correctly after the app is fully restarted.

Steps to reproduce

  1. Set up a React Native project with the new architecture enabled.
  2. Use I18nManager.forceRTL(true/false) to change the layout direction and use react-native-restart. to restart the app.
  3. Observe that the UI does not update immediately(styles not forced to be like text-align: right).

React Native Version

0.74.0

Affected Platforms

Runtime - iOS, Build - MacOS

Areas

Fabric - The New Renderer, TurboModule - The New Native Module System, JSI - Javascript Interface, Bridgeless - The New Initialization Flow, Codegen

Output of npx react-native info

System:
  OS: macOS 14.5
  CPU: (10) arm64 Apple M1 Max
  Memory: 272.36 MB / 32.00 GB
  Shell:
    version: "5.9"
    path: /bin/zsh
Binaries:
  Node:
    version: 20.10.0
    path: ~/.nvm/versions/node/v20.10.0/bin/node
  Yarn:
    version: 4.3.1
    path: ~/.nvm/versions/node/v20.10.0/bin/yarn
  npm:
    version: 10.2.3
    path: ~/.nvm/versions/node/v20.10.0/bin/npm
  Watchman:
    version: 2024.07.15.00
    path: /opt/homebrew/bin/watchman
Managers:
  CocoaPods:
    version: 1.15.2
    path: /usr/local/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: Not Found
IDEs:
  Android Studio: 2024.1 AI-241.18034.62.2411.12071903
  Xcode:
    version: 15.4/15F31d
    path: /usr/bin/xcodebuild
Languages:
  Java:
    version: 17.0.11
    path: /usr/bin/javac
  Ruby:
    version: 2.6.10
    path: /usr/bin/ruby
npmPackages:
  "@react-native-community/cli": Not Found
  react:
    installed: 18.2.0
    wanted: 18.2.0
  react-native:
    installed: 0.74.0
    wanted: 0.74.0
  react-native-macos: Not Found
npmGlobalPackages:
  "*react-native*": Not Found
Android:
  hermesEnabled: true
  newArchEnabled: true
iOS:
  hermesEnabled: true
  newArchEnabled: true

info React Native v0.74.3 is now available (your project is running on v0.74.0).
info Changelog: https://github.com/facebook/react-native/releases/tag/v0.74.3
info Diff: https://react-native-community.github.io/upgrade-helper/?from=0.74.0
info For more info, check out "https://reactnative.dev/docs/upgrading?os=macos".



### Stacktrace or Logs

```text
No crush or failure

Reproducer

Screenshots and Videos

No response

@serhii-yalla serhii-yalla added Needs: Triage 🔍 Type: New Architecture Issues and PRs related to new architecture (Fabric/Turbo Modules) labels Jul 25, 2024
@react-native-bot
Copy link
Collaborator

⚠️ Missing Reproducible Example
ℹ️ We could not detect a reproducible example in your issue report. Please provide either:
  • If your bug is UI related: a Snack
  • If your bug is build/update related: use our Reproducer Template. A reproducer needs to be in a GitHub repository under your username.

@react-native-bot react-native-bot added Needs: Repro This issue could be improved with a clear list of steps to reproduce the issue. Newer Patch Available labels Jul 25, 2024
@react-native-bot
Copy link
Collaborator

⚠️ Newer Version of React Native is Available!
ℹ️ You are on a supported minor version, but it looks like there's a newer patch available - 0.74.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
Copy link
Collaborator

⚠️ Missing Reproducible Example
ℹ️ We could not detect a reproducible example in your issue report. Please provide either:

@react-native-bot
Copy link
Collaborator

⚠️ Newer Version of React Native is 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.

@cortinico
Copy link
Contributor

@serhii-yalla is the same behavior happening also on Old Arch?

@serhii-yalla
Copy link
Author

@serhii-yalla is the same behavior happening also on Old Arch?

No. On the old arch reload forces styles to change. So it works as expected

@github-actions github-actions bot added Needs: Attention Issues where the author has responded to feedback. and removed Needs: Author Feedback labels Jul 25, 2024
@cortinico
Copy link
Contributor

@serhii-yalla great thanks for the info.
Could you add a couple of videos of OldArch/NewArch? It would make it easier for me to find an owner for this one.

@serhii-yalla
Copy link
Author

@serhii-yalla great thanks for the info. Could you add a couple of videos of OldArch/NewArch? It would make it easier for me to find an owner for this one.

@cortinico Here it is.

WithoutNewArch.1.mp4
WithNewArch.1.mp4

@cortinico
Copy link
Contributor

Great thank you!
If we could also have a small reproducer using this template: https://github.com/react-native-community/reproducer-react-native

it would be gold

@serhii-yalla
Copy link
Author

@cortinico Here is reproducer for you
https://github.com/serhii-yalla/RTL-Layout-Direction-Not-Updating-after-app-reload-with-New-React-Native-Architecture

@cortinico cortinico added Issue: Author Provided Repro This issue can be reproduced in Snack or an attached project. and removed Needs: Repro This issue could be improved with a clear list of steps to reproduce the issue. Newer Patch Available Needs: Attention Issues where the author has responded to feedback. labels Jul 26, 2024
@numandev1
Copy link
Contributor

getting the same issue, in the new architecture, the Layout direction is not working

@numandev1
Copy link
Contributor

maybe it is related to #39803

@asgarPeerbits
Copy link

I am also facing the same issue in version 0.76.1. However, it works perfectly on Android; the issue only occurs on iOS.

@mohamedadlyy
Copy link

ihave the same issue on ios

@a-eid
Copy link

a-eid commented Nov 15, 2024

weirdly enough if you close and relaunch the app it works, but not with reloading.

@mrusmanali
Copy link

I guess we have to leave an undeclared variable after the language change to let tha app crash... and hope that user will open it again.

@hkellou-vitalcare
Copy link

I guess we have to leave an undeclared variable after the language change to let tha app crash... and hope that user will open it again.

I would advice against it, that will make your crash metrics go down hill and might trigger an alert in your google play console.

@AsherSheikh
Copy link

Having facing the same issue only on iOS, android work perfectly in react native version 0.76.3... any update for iOS ??

@cortinico
Copy link
Contributor

Another reproducer: https://github.com/bhojaniasgar/RTLIssue

@ahmamedhat
Copy link

Facing the same issue here, I use:

"react-native": "0.76.3",
"expo": "~52.0.11"

@armata99
Copy link

armata99 commented Dec 9, 2024

I'm experiencing same issue on IOS with react-native: 0.76.3 and newArch enabled. Android is fine.

@mktufan21
Copy link

mktufan21 commented Dec 13, 2024

Disable newArch until it is fixed

It will solve the problem

add this inside your PodFile

use_react_native!(:path => config[:reactNativePath], :new_arch_enabled => false)
$ReactNativePath = "../node_modules/react-native"
pod 'React-Codegen', :path => $ReactNativePath, :inhibit_warnings => true if false

@asgarPeerbits
Copy link

@mktufan21 After making this false react-native-mmkv is not working in Version 3.x..x and when downgrading MMKV 2.x.x then android not working

is there any way to use MMKV version 3.x.x in android and 2.x.x in iOS till this issue solves in new Architecture

@a-eid
Copy link

a-eid commented Dec 13, 2024

@asgarPeerbits You have to use the new arch to be able to use rn mmkv v3.

@asgarPeerbits
Copy link

@a-eid Yes its enable in Android side

@a-eid
Copy link

a-eid commented Dec 14, 2024

@asgarPeerbits ah I didn't get your question correctly, I don't think there is a proper way to use mmkv v2 on ios & v3 on android. just downgrade back to mmkv v2 until this is fixed.

@Saad962404
Copy link

Saad962404 commented Dec 15, 2024

I'm also experiencing same with new arch. Android seems to be working fine.
"react-native": "0.76.1",

@blakef
Copy link
Contributor

blakef commented Dec 15, 2024

Locking as we’re aware this has never worked on iOS with the new architecture.

As always PRs are welcome, however it's quite a complex issue as it involves how Yoga handles LTR and how we keep things in sync with the New Architecture. You'll need to know these quite well to implement a fix.

@cipolleschi
Copy link
Contributor

This should be fixed by #49455

@facebook facebook unlocked this conversation Feb 25, 2025
@cortinico cortinico added the Resolution: Fixed A PR that fixes this issue has been merged. label Feb 25, 2025
hoxyq pushed a commit to hoxyq/react-native that referenced this issue Feb 25, 2025
Summary:
This fixes an issue in Fabric where changing the layout direction and then reloading the JS bundle did not honor the layout direction until the app was restarted on iOS. This now calls  `_updateLayoutContext` whenever RCTSurfaceView is recreated which happens on bundle reload. This is not an issue on the old architecture because the layout direction is determined within the [SurfaceViews](https://github.com/facebook/react-native/blob/acdddef48eb60b002c954d7d2447cb9c2883c8b3/packages/react-native/React/Views/RCTRootShadowView.m#L18) which were recreated on bundle reload.

## Related Issues:
- react-native-community/discussions-and-proposals#847
- facebook#49451
- facebook#48311
- facebook#45661

## How can we take this further?
If we want to make it so that it doesn't require an entire bundle reload for RTL to take effect I believe these are the steps that would need to be taken:
- Make it so [RCTI18nManager](https://github.com/facebook/react-native/blob/acdddef48eb60b002c954d7d2447cb9c2883c8b3/packages/react-native/React/CoreModules/RCTI18nManager.mm#L52) exports isRTL as a method instead of consts
- Send Notification Center notif when RTL is forced on or off
- Listen for that notification RCTSurfaceView and call _updateLayoutContext similar to UIContentSizeCategoryDidChangeNotification.

## Changelog:

[iOS] [FIXED] - Layout direction changes are now honored on bundle reload.

<!-- Help reviewers and the release process by writing your own changelog entry.

Pick one each for the category and type tags:

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests

Pull Request resolved: facebook#49455

Test Plan:
On the new architecture change force the layout direction and reload the bundle:
```
import React, { useCallback } from "react";
import { Button, I18nManager, StyleSheet, Text, View } from "react-native";
import RNRestart from "react-native-restart";

export default function Explore() {
    const onApplyRTL = useCallback(() => {
        if (!I18nManager.isRTL) {
            I18nManager.forceRTL(true);
            RNRestart.restart();
        }
    }, []);

    const onApplyLTR = useCallback(() => {
        if (I18nManager.isRTL) {
            I18nManager.forceRTL(false);
            RNRestart.restart();
        }
    }, []);

    return (
        <View style={styles.area}>
            <Text>Test Block</Text>
            <View style={styles.testBlock}>
                <Text>Leading</Text>
                <Text>Trailing</Text>
            </View>
            <Button title={"Apply RTL"} onPress={onApplyRTL} />
            <Button title={"Apply LTR"} onPress={onApplyLTR} />
        </View>
    );
}

const styles = StyleSheet.create({
    area: {
        marginVertical: 50,
        paddingHorizontal: 24,
    },
    testBlock: {
        paddingVertical: 10,
        flexDirection: "row",
        justifyContent: "space-between",
    },
});

```

https://github.com/user-attachments/assets/0eab0d79-de3f-4eeb-abd0-439ba4fe25c0

Reviewed By: cortinico, cipolleschi

Differential Revision: D69797645

Pulled By: NickGerleman

fbshipit-source-id: 97499621f3dd735d466f5119e0f2a0eccf1c3c05
@Linuhusainnk
Copy link

Will this fix be released in 0.76 or 0.77

@a-eid
Copy link

a-eid commented Mar 4, 2025

@Linuhusainnk I think it'll be released to both 0.76 & 0.77 cc @cortinico

@ahmamedhat
Copy link

@a-eid @Linuhusainnk Guys any idea how to make a patch with this until its released? i have tried adding this line
[self _updateLayoutContext];
in the same exact place changed in the PR, deleted ios directory and rebuilt the app again but sadly it didn’t work
i am using rtl manager and forcing to and from RTL and then call RNRestart the app restarts but nothing happens had to close and reopen the app, is there’s something im missing here?!

@a-eid
Copy link

a-eid commented Mar 4, 2025

@ahmamedhat I am not sure TBH, could you share your patch file and some details about your setup ?

@Linuhusainnk
Copy link

Hi @a-eid

will it take long time, we are waiting for long or should we go for making patch for now?

@cipolleschi
Copy link
Contributor

If you want for the patch to be released, you need to open pick requests for each version using this template.

We release new patches of older version periodically, when there is a critical commit or when there are enough commits to ship to justify the release.

react-native-bot pushed a commit that referenced this issue Mar 10, 2025
Summary:
This fixes an issue in Fabric where changing the layout direction and then reloading the JS bundle did not honor the layout direction until the app was restarted on iOS. This now calls  `_updateLayoutContext` whenever RCTSurfaceView is recreated which happens on bundle reload. This is not an issue on the old architecture because the layout direction is determined within the [SurfaceViews](https://github.com/facebook/react-native/blob/acdddef48eb60b002c954d7d2447cb9c2883c8b3/packages/react-native/React/Views/RCTRootShadowView.m#L18) which were recreated on bundle reload.

## Related Issues:
- react-native-community/discussions-and-proposals#847
- #49451
- #48311
- #45661

## How can we take this further?
If we want to make it so that it doesn't require an entire bundle reload for RTL to take effect I believe these are the steps that would need to be taken:
- Make it so [RCTI18nManager](https://github.com/facebook/react-native/blob/acdddef48eb60b002c954d7d2447cb9c2883c8b3/packages/react-native/React/CoreModules/RCTI18nManager.mm#L52) exports isRTL as a method instead of consts
- Send Notification Center notif when RTL is forced on or off
- Listen for that notification RCTSurfaceView and call _updateLayoutContext similar to UIContentSizeCategoryDidChangeNotification.

## Changelog:

[iOS] [FIXED] - Layout direction changes are now honored on bundle reload.

<!-- Help reviewers and the release process by writing your own changelog entry.

Pick one each for the category and type tags:

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests

Pull Request resolved: #49455

Test Plan:
On the new architecture change force the layout direction and reload the bundle:
```
import React, { useCallback } from "react";
import { Button, I18nManager, StyleSheet, Text, View } from "react-native";
import RNRestart from "react-native-restart";

export default function Explore() {
    const onApplyRTL = useCallback(() => {
        if (!I18nManager.isRTL) {
            I18nManager.forceRTL(true);
            RNRestart.restart();
        }
    }, []);

    const onApplyLTR = useCallback(() => {
        if (I18nManager.isRTL) {
            I18nManager.forceRTL(false);
            RNRestart.restart();
        }
    }, []);

    return (
        <View style={styles.area}>
            <Text>Test Block</Text>
            <View style={styles.testBlock}>
                <Text>Leading</Text>
                <Text>Trailing</Text>
            </View>
            <Button title={"Apply RTL"} onPress={onApplyRTL} />
            <Button title={"Apply LTR"} onPress={onApplyLTR} />
        </View>
    );
}

const styles = StyleSheet.create({
    area: {
        marginVertical: 50,
        paddingHorizontal: 24,
    },
    testBlock: {
        paddingVertical: 10,
        flexDirection: "row",
        justifyContent: "space-between",
    },
});

```

https://github.com/user-attachments/assets/0eab0d79-de3f-4eeb-abd0-439ba4fe25c0

Reviewed By: cortinico, cipolleschi

Differential Revision: D69797645

Pulled By: NickGerleman

fbshipit-source-id: 97499621f3dd735d466f5119e0f2a0eccf1c3c05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue: Author Provided Repro This issue can be reproduced in Snack or an attached project. Needs: Author Feedback Resolution: Fixed A PR that fixes this issue has been merged. Type: New Architecture Issues and PRs related to new architecture (Fabric/Turbo Modules)
Projects
None yet
Development

No branches or pull requests