react-native icon indicating copy to clipboard operation
react-native copied to clipboard

Interop layer doesnt work for react-native-charts-wrapper

Open MaxToyberman opened this issue 1 year ago • 7 comments

Description

I am using react-native-charts-wrapper, and after enabling the new architecture (Fabric) on iOS, the charts no longer render. On Android, I have started migrating to Fabric (work in progress), and it compiles and runs successfully.

However, on iOS, the library and native code are written in Swift. As I understand, Fabric currently has limited compatibility with Swift, which may be causing the issue.

Steps to reproduce

  1. yarn install
  2. cd ios && bundle install
  3. bundle exec pod install - Graph is visible
  4. bundle install && RCT_NEW_ARCH_ENABLED=1 arch -x86_64 bundle exec pod install - Graph is invisible

React Native Version

0.75.4

Affected Platforms

Runtime - iOS

Areas

Fabric - The New Renderer

Output of npx react-native info

System:
  OS: macOS 14.7
  CPU: (10) arm64 Apple M1 Pro
  Memory: 215.09 MB / 32.00 GB
  Shell:
    version: "5.9"
    path: /bin/zsh
Binaries:
  Node:
    version: 20.11.1
    path: /usr/local/bin/node
  Yarn:
    version: 3.6.4
    path: ~/.npm-global/bin/yarn
  npm:
    version: 10.2.4
    path: /usr/local/bin/npm
  Watchman:
    version: 2024.09.16.00
    path: /opt/homebrew/bin/watchman
Managers:
  CocoaPods:
    version: 1.15.2
    path: /Users/maximtoyberman/.rvm/gems/ruby-3.2.2/bin/pod
SDKs:
  iOS SDK:
    Platforms:
      - DriverKit 24.0
      - iOS 18.0
      - macOS 15.0
      - tvOS 18.0
      - visionOS 2.0
      - watchOS 11.0
  Android SDK: Not Found
IDEs:
  Android Studio: 2024.1 AI-241.18034.62.2412.12266719
  Xcode:
    version: 16.0/16A242d
    path: /usr/bin/xcodebuild
Languages:
  Java:
    version: 17.0.9
    path: /usr/bin/javac
  Ruby:
    version: 3.2.2
    path: /Users/maximtoyberman/.rvm/rubies/ruby-3.2.2/bin/ruby
npmPackages:
  "@react-native-community/cli": Not Found
  react:
    installed: 18.3.1
    wanted: 18.3.1
  react-native:
    installed: 0.75.4
    wanted: 0.75.4
  react-native-macos: Not Found
npmGlobalPackages:
  "*react-native*": Not Found
Android:
  hermesEnabled: true
  newArchEnabled: false
iOS:
  hermesEnabled: true
  newArchEnabled: true

Stacktrace or Logs

Bridgeless mode is enabled
Unbalanced calls start/end for tag 20
Unbalanced calls start/end for tag 19
Running "ReproducerApp" with {"rootTag":1,"initialProps":{"concurrentRoot":true},"fabric":true}

Reproducer

https://github.com/MaxToyberman/charts-wrapper-repro

Screenshots and Videos

No response

MaxToyberman avatar Oct 14 '24 10:10 MaxToyberman

@MaxToyberman thanks for reporting this bug. A couple of questions:

On Android, I have started migrating to Fabric (work in progress), and it compiles and runs successfully.

  1. Is the library react-native-charts-wrapper also working for Android? Or are you referring to your app in general?
  2. Have you opened an issue against the library maintainer?
  3. Do you have a stacktrace from the iOS log of what you're experiencing?

cortinico avatar Oct 14 '24 17:10 cortinico

Hi @cortinico,

  1. On Android, the library works fine when the newArch flag is disabled, but when it's enabled, no views are rendered. (same for iOS)

  2. I noticed there are open issues regarding the migration to the new architecture, but it doesn't seem like it's moving forward. That's why I wanted to contribute. However, the iOS code is written entirely in Swift, which seems to pose a problem when migrating to Fabric. Is that correct?

  3. I’ve included a stack trace in the issue. The app launches, but no views are visible.

Maybe there is an option to debug the interop layer ?

MaxToyberman avatar Oct 14 '24 17:10 MaxToyberman

  1. On Android, the library works fine when the newArch flag is disabled, but when it's enabled, no views are rendered. (same for iOS)

Ok so it's broken for both Android & iOS. This was not clear in your original issue.

However, the iOS code is written entirely in Swift, which seems to pose a problem when migrating to Fabric. Is that correct?

@cipolleschi can provide more insights on this

Maybe there is an option to debug the interop layer ?

We're aware of scenarios where the Interop Layer doesn't work. For example if your view managers provide a custom shadow node implementation. That doesn't seem to be the case here (at least for Android) and would need a bit more of investigation.

cortinico avatar Oct 15 '24 10:10 cortinico

Hi @cortinico sorry for the confustion.

The issue is only with iOS, will edit my original issue now

MaxToyberman avatar Oct 21 '24 06:10 MaxToyberman

Hi @cortinico,

It seems I've managed to fix the issue, but I'd appreciate any insights you might have on the root cause. You can check out the change here:

react-native-charts-wrapper PR #1004

I noticed that when adjusting the frame size in the new architecture, the view becomes visible again.

another issue that i see is that the viewRegistry is empty, i found this ticket

MaxToyberman avatar Oct 21 '24 12:10 MaxToyberman

It seems I've managed to fix the issue, but I'd appreciate any insights you might have on the root cause. You can check out the change here:

@cipolleschi can help here as this is iOS specific

cortinico avatar Oct 21 '24 17:10 cortinico

Hi @cipolleschi I have an issue with addUIBlock which looks like it was fixed here

MaxToyberman avatar Oct 22 '24 11:10 MaxToyberman

Just to understand: is this issue fixed with the fix you mentioned here?

The fix has been there for a long time, now. Versions 74 to 76 all have the fix included.

Could you clarify what root cause you'd like to have an explanation for?

cipolleschi avatar Oct 28 '24 14:10 cipolleschi

Hi @cipolleschi

If you see the changes here

you will notice 2 changes:

  1. This will make the graphs visible: (it is invisible on iOS new arch)
    override func layoutSubviews() {
        super.layoutSubviews()
        _chart.frame = self.bounds // Adjust the chart's frame to fill the entire component's bounds
    }
  1. I noticed that viewRegistry.count is 0 in new architecture, and than i found The fix but looks like it doesnt work, this is not a real fix but for now i am doing this
      if (viewRegistry?.count == 0) {
        return
      }

I hope it clarifies the issue.

MaxToyberman avatar Oct 28 '24 15:10 MaxToyberman

for 2, why do you need to count them? the View registry is not supposed to be used like that. It is supposed to be used by specifically access the view you know the tag of. In the old architecture, the registry might have contained views that you are not interested into.

In the New Architecture, we would like to get rid of the viewRegistry. You need to rely on the UIManager, instead and that's what the interop layer is doing, forwarding the call to the UIManager.

The right check for you is not

if (viewRegistry?.count == 0) {
  return
}

but more

guard 
  let view: RNBarLineChartViewBase = viewRegistry[reactTag] as? RNBarLineChartViewBase,
  let barLineChart = view.chart as? BarLineChartViewBase {
  return
}
barLineChart.moveViewToX(xValue.doubleValue);

This will also let you avoid those ! which are a bad practice in swift.

cipolleschi avatar Oct 28 '24 15:10 cipolleschi

@cipolleschi Thanks,

i dont need to count them but the app crashes because viewRegistry.count is 0... i am not the author of the library but i thought that the fix that was introduced by you should solve it when using the interop layer

MaxToyberman avatar Oct 28 '24 16:10 MaxToyberman