feat: add Fabric support #456

WoLewicki · 2022-11-17T15:16:10Z

PR adding Fabric support to the library. Android code regarding handling state on Fabric has been mainly taken from react-native-screens repo (Screen component).

As for the JS code, I have changed the implementation to functional style components, based mainly on Switch: https://github.com/facebook/react-native/blob/5dd2f2e4b7669397f8bfa9b3845afee7e4e47626/Libraries/Components/Switch/Switch.js.
It was done like this since Fabric introduces some changes in how the render/layout phases work, leading to jumping layout when native commands (substitute for setNativeProps on Fabric) were called in the event callback (see attached video).
Now the native event changes the state of component, leading to a rerender and call of useLayoutEffect, where there is a chance of reverting the native change.

Simulator.Screen.Recording.-.iPhone.14.Pro.-.2022-11-24.at.15.37.34.mp4

Coauthored by @j-piasecki.

RNCPicker.podspec

ios/RNCPicker.h

ios/RNCPicker.mm

ios/RNCPickerComponentView.h

Co-authored-by: Tomek Zawadzki <tomekzawadzki98@gmail.com>

j-piasecki · 2022-11-21T10:11:53Z

android/src/main/jni/CMakeLists.txt

+)
+
+file(GLOB LIB_CUSTOM_SRCS CONFIGURE_DEPENDS *.cpp)
+# file(GLOB LIB_CUSTOM_SRCS CONFIGURE_DEPENDS *.cpp ${LIB_COMMON_DIR}/react/renderer/components/${LIB_LITERAL}/*.cpp)


Suggested change

# file(GLOB LIB_CUSTOM_SRCS CONFIGURE_DEPENDS *.cpp ${LIB_COMMON_DIR}/react/renderer/components/${LIB_LITERAL}/*.cpp)

j-piasecki · 2022-11-21T10:12:02Z

android/src/main/jni/CMakeLists.txt

+  ${LIB_TARGET_NAME}
+  PUBLIC
+  .
+  # ${LIB_COMMON_DIR}


Suggested change

# ${LIB_COMMON_DIR}

j-piasecki · 2022-11-21T10:13:08Z

android/src/main/java/com/reactnativecommunity/picker/ReactPickerManager.java

+  public void focus(ReactPicker root) {
+    root.performClick();
+  }
+
+  public void blur(ReactPicker root) {
+    root.clearFocus();
+  }
+
+  public void setNativeSelected(ReactPicker picker, int selected) {
+    picker.setStagedSelection(selected);
+  }


We could use those methods in the receiveCommands and setSelected prop to avoid duplication.

WoLewicki · 2022-11-21T10:36:48Z

android/src/main/java/com/reactnativecommunity/picker/ReactPickerManager.java

@@ -149,9 +149,15 @@ protected void onAfterUpdateTransaction(ReactPicker view) {
  protected void addEventEmitters(
      final ThemedReactContext reactContext,
      final ReactPicker picker) {
+    EventDispatcher eventDispatcher =


Why is it done like this now? Cause it is a null on Fabric? If so, what about those listeners?

It didn't work on Fabric the way it was before, so I checked how it's done in the core components: https://github.com/facebook/react-native/blob/84874704766c0bc2dde88720b04b3ba4a695cf30/ReactAndroid/src/main/java/com/facebook/react/views/drawer/ReactDrawerLayoutManager.java#L60-L66.

WoLewicki · 2022-11-21T10:40:52Z

android/src/main/jni/CMakeLists.txt

@@ -0,0 +1,73 @@
+cmake_minimum_required(VERSION 3.13)


Is it taken from the react-native-screens repo?

WoLewicki · 2022-11-21T10:48:17Z

react-native.config.js

+let supportsCodegenConfig = true;
+// try {
+//   const rnCliAndroidVersion = require('@react-native-community/cli-platform-android/package.json')
+//     .version;
+//   const [major] = rnCliAndroidVersion.split('.');
+//   supportsCodegenConfig = major >= 9;
+// } catch (e) {
+//   // ignore
+// }


Should it be left like that?

It was this way because the commented part was picking the rncli version from the root package instead of the fabric example.

I made a workaround for it in 1f41b69, I added a flag that allows to enable the custom config and made a patch to the cli so that it runs with the added flag.

js/PickerAndroid.android.js

Co-authored-by: Jesse Katsumata <niconico.clarinet@gmail.com>

Naturalclar · 2022-12-05T00:43:10Z

js/PickerAndroid.android.js

+    nativeSelectedIndex,
+    props.children,
+    FABRIC_ENABLED,
+  ]);


fixing lint issue

Suggested change

]);

props.mode

]);

Naturalclar · 2022-12-05T00:50:27Z

js/AndroidDialogPickerNativeComponent.js

@@ -10,27 +10,63 @@

 'use strict';

-import {requireNativeComponent} from 'react-native';
+import codegenNativeComponent from 'react-native/Libraries/Utilities/codegenNativeComponent';


Adding missing flow types

Suggested change

import codegenNativeComponent from 'react-native/Libraries/Utilities/codegenNativeComponent';

import * as React from 'react';

import type {Int32} from 'react-native/Libraries/Types/CodegenTypes';

import type {BubblingEventHandler} from 'react-native/Libraries/Types/CodegenTypes';

import type {ColorValue} from 'react-native/Libraries/StyleSheet/StyleSheet';

import type {ViewProps} from 'react-native/Libraries/Components/View/ViewPropTypes';

import type {ComponentType} from 'react';

import codegenNativeComponent from 'react-native/Libraries/Utilities/codegenNativeComponent';

kevin-williams · 2023-01-26T18:31:08Z

Any update to this PR?

I have a release I want to get to prod for our company because fabric makes the app so much faster. This is one of my last TODOs on the upgrade. If the PR is soon I'll wait.

Thanks!

Korin92 · 2023-02-27T11:14:05Z

Doesn't support Fabric yet?

gtomitsuka · 2023-03-16T16:28:37Z

Hey, are there any remaining milestones on the JS or iOS side? Happy to support!

We could push this to a feature branch for easier beta testing & further work by more contributors

efstathiosntonas · 2023-08-05T07:16:51Z

Hey folks, are there any updates on this? Thanks

WoLewicki · 2023-08-07T10:27:25Z

I think it's up to @Naturalclar to decide what to do with it.

dmk3141618 · 2023-10-04T02:15:35Z

Any updates? Nowadays most of RN codes are based on the Fabric new architecture.
Can I only use this on the old architecture?

It seems I can not use only this package on the old architecture because I can only select one architecture with compile level option.

aos : gradle.properties > newArchEnabled=true
ios : RCT_NEW_ARCH_ENABLED=1 npx pod-install ios

Louis-C7 · 2023-10-11T02:30:14Z

Any update?

j-piasecki · 2023-11-27T15:47:39Z

@Naturalclar Are there any plans to merge this PR?

WoLewicki · 2023-11-27T15:49:16Z

It should be ready @Naturalclar 🚀 Would be really nice to merge it and release so people can check if everything works fine.

# [2.6.0](v2.5.1...v2.6.0) (2023-11-28) ### Features * add Fabric support ([#456](#456)) ([498b4e3](498b4e3))

Naturalclar · 2023-11-28T00:56:11Z

🎉 This PR is included in version 2.6.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

flochtililoch · 2024-03-15T16:32:32Z

js/PickerIOS.ios.js

-        value: child.props.value,
-        label: child.props.label,
+      return {
+        value: String(child.props.value),


this change caused a regression in my app, as the expected type returned by the onValueChange handler used to be an integer (if the item's value was initially an int).

WoLewicki added 2 commits November 16, 2022 18:50

feat: add FabricExample app

c2b01a3

feat: make project compile on ios

e9f8ce1

WoLewicki changed the title ~~feat: add FabricExample app~~ feat: add Fabric support Nov 17, 2022

WoLewicki added 2 commits November 17, 2022 16:43

feat: add all ios props

73bcf6f

fix: some quirks and remove setNativeProps

70a5761

tomekzaw reviewed Nov 17, 2022

View reviewed changes

RNCPicker.podspec Outdated Show resolved Hide resolved

RNCPicker.podspec Outdated Show resolved Hide resolved

ios/RNCPicker.h Outdated Show resolved Hide resolved

ios/RNCPicker.mm Outdated Show resolved Hide resolved

ios/RNCPickerComponentView.h Outdated Show resolved Hide resolved

feat: some work on Android

f63b694

roryabraham mentioned this pull request Nov 19, 2022

[HOLD for payment 2024-04-25] [Tracking] [Performance] Enable the new Fabric architecture Expensify/App#8503

Closed

38 tasks

j-piasecki and others added 8 commits November 21, 2022 10:45

Add c++ files and make them compile

6f5373b

Finish up the dialog picker

b2cff6c

Add dropdown picker shadow node

fb9d3e5

Clean up component descriptors

78c54ad

Make paper and fabric work at the same time

fb19d41

Make events work

bd63493

Use native commands instead of setNativeProps

b73e9b6

Apply suggestions from code review

0cfd37a

Co-authored-by: Tomek Zawadzki <tomekzawadzki98@gmail.com>

WoLewicki marked this pull request as ready for review November 21, 2022 10:01

WoLewicki requested a review from Naturalclar as a code owner November 21, 2022 10:01

j-piasecki reviewed Nov 21, 2022

View reviewed changes

fix: change flag as suggested

2ea07ab

WoLewicki commented Nov 21, 2022

View reviewed changes

Remove comments in CMake

d495eed

WoLewicki commented Nov 21, 2022

View reviewed changes

j-piasecki and others added 4 commits November 22, 2022 12:54

Add comments

fb33ad0

Clean up config

1f41b69

Add comments

f7706d0

fix: change setting state to command on ios

5e2fa0e

Naturalclar reviewed Nov 23, 2022

View reviewed changes

js/PickerAndroid.android.js Outdated Show resolved Hide resolved

Update js/PickerAndroid.android.js

b13a7db

Co-authored-by: Jesse Katsumata <niconico.clarinet@gmail.com>

j-piasecki added 3 commits November 30, 2022 11:52

Update measuring logic for dialog picker

f8f021f

Update measurement for dropdown picker

963ef63

Use measurement from onMeasure on fabric

d1540e9

Naturalclar reviewed Dec 5, 2022

View reviewed changes

fix: fix all eslint errors and most of flow checks

2d42ec5

valeriobelli mentioned this pull request Dec 24, 2022

Unimplemented component: <RNCPicker> for new architecture #466

Open

Support RN 0.71 when using the new arch

6fa0978

Sergiohcp mentioned this pull request Jul 6, 2023

feat: fabric winecombr/picker#1

Merged

j-piasecki added 2 commits November 27, 2023 11:02

Update to 0.73.0-rc.5

54b2cbf

Merge branch 'master' into @wolewicki/add-fabric

624af21

Naturalclar merged commit 498b4e3 into react-native-picker:master Nov 28, 2023
8 of 12 checks passed

github-actions bot pushed a commit that referenced this pull request Nov 28, 2023

chore(release): 2.6.0 [skip ci]

16d693e

# [2.6.0](v2.5.1...v2.6.0) (2023-11-28) ### Features * add Fabric support ([#456](#456)) ([498b4e3](498b4e3))

Naturalclar added the released label Nov 28, 2023

flochtililoch reviewed Mar 15, 2024

View reviewed changes

swrobel mentioned this pull request Mar 26, 2024

Picker 2.6+ transforms numeric values to strings #538

Open

WoLewicki mentioned this pull request Nov 25, 2024

fix: use dynamics to cover string and number #600

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add Fabric support #456

feat: add Fabric support #456

WoLewicki commented Nov 17, 2022 •

edited

Loading

j-piasecki Nov 21, 2022

j-piasecki Nov 21, 2022

j-piasecki Nov 21, 2022

WoLewicki Nov 21, 2022

j-piasecki Nov 22, 2022

WoLewicki Nov 21, 2022

j-piasecki Nov 22, 2022

WoLewicki Nov 21, 2022

j-piasecki Nov 22, 2022

Naturalclar Dec 5, 2022

Naturalclar Dec 5, 2022

kevin-williams commented Jan 26, 2023

Korin92 commented Feb 27, 2023

gtomitsuka commented Mar 16, 2023 •

edited

Loading

efstathiosntonas commented Aug 5, 2023

WoLewicki commented Aug 7, 2023

dmk3141618 commented Oct 4, 2023

Louis-C7 commented Oct 11, 2023

j-piasecki commented Nov 27, 2023

WoLewicki commented Nov 27, 2023

Naturalclar commented Nov 28, 2023

flochtililoch Mar 15, 2024

-import codegenNativeComponent from 'react-native/Libraries/Utilities/codegenNativeComponent';
+import * as React from 'react';
+import type {Int32} from 'react-native/Libraries/Types/CodegenTypes';
+import type {BubblingEventHandler} from 'react-native/Libraries/Types/CodegenTypes';
+import type {ColorValue} from 'react-native/Libraries/StyleSheet/StyleSheet';
+import type {ViewProps} from 'react-native/Libraries/Components/View/ViewPropTypes';
+import type {ComponentType} from 'react';
+import codegenNativeComponent from 'react-native/Libraries/Utilities/codegenNativeComponent';

feat: add Fabric support #456

feat: add Fabric support #456

Conversation

WoLewicki commented Nov 17, 2022 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kevin-williams commented Jan 26, 2023

Korin92 commented Feb 27, 2023

gtomitsuka commented Mar 16, 2023 • edited Loading

efstathiosntonas commented Aug 5, 2023

WoLewicki commented Aug 7, 2023

dmk3141618 commented Oct 4, 2023

Louis-C7 commented Oct 11, 2023

j-piasecki commented Nov 27, 2023

WoLewicki commented Nov 27, 2023

Naturalclar commented Nov 28, 2023

Choose a reason for hiding this comment

WoLewicki commented Nov 17, 2022 •

edited

Loading

gtomitsuka commented Mar 16, 2023 •

edited

Loading