diff --git a/docs/docs/concepts/hybrid-views.md b/docs/docs/concepts/hybrid-views.md index 63e7a2eba9..5a14fd1f14 100644 --- a/docs/docs/concepts/hybrid-views.md +++ b/docs/docs/concepts/hybrid-views.md @@ -70,16 +70,16 @@ To access the actual underlying object, you can use the `hybridRef`: function App() { return ( { + hybridRef={(ref) => { console.log(ref.name) // <-- HybridCamera const image = ref.takePhoto() - })} + }} /> ) } ``` -> Note: If you're wondering about the `callback(...)` syntax, see ["Callbacks have to be wrapped"](../guides/view-components#callbacks-have-to-be-wrapped). +> Note: On react-native 0.78 - 0.80, this has to be wrapped in `callback(...)`, see ["Callbacks"](../guides/view-components#callbacks). ## Full Guides diff --git a/docs/docs/guides/view-components.md b/docs/docs/guides/view-components.md index b44f195d8e..fb97452e14 100644 --- a/docs/docs/guides/view-components.md +++ b/docs/docs/guides/view-components.md @@ -13,6 +13,7 @@ The key difference to a Fabric view is that it uses Nitro for prop parsing, whic :::note Nitro Views require **react-native 0.78.0** or higher, and require the new architecture. +On **react-native 0.81.0** or higher, callbacks can be passed to Nitro Views directly (see ["Callbacks"](#callbacks)). ::: ## Create a Nitro View @@ -235,12 +236,9 @@ To batch prop changes, you can override `beforeUpdate()` and `afterUpdate()` in -### Callbacks have to be wrapped +### Callbacks -Whereas Nitro allows passing JS functions to native code directly, React Native core doesn't allow that. Instead, functions are wrapped in an event listener registry, and a simple boolean is passed to the native side. -Unfortunately React Native's renderer does not yet allow changing this behaviour, so functions cannot be passed directly to Nitro Views. As a workaround, Nitro requires you to wrap each function in an object, which bypasses React Native's conversion. - -To simplify this, Nitro exposes the `callback(...)` method: +Nitro passes JS functions to native code directly, so callbacks are just regular props: ```tsx export interface CameraProps extends HybridViewProps { @@ -249,15 +247,25 @@ export interface CameraProps extends HybridViewProps { export type CameraView = HybridView function App() { - // diff-remove return console.log(i)} /> - // diff-add +} +``` + +#### react-native 0.78 - 0.80: `callback(...)` + +Historically, React Native core did not allow this. Instead, functions were wrapped in an event listener registry, and a simple boolean was passed to the native side. +Since [react-native 0.81](https://github.com/facebook/react-native/pull/48777) a View Config can opt out of that conversion, which Nitro does - so functions arrive on the native side unchanged. + +On **react-native 0.78 - 0.80** you still need to wrap every function in an object to bypass React Native's conversion. Nitro exposes the `callback(...)` method for this: + +```tsx +function App() { return console.log(i))} /> } ``` -:::info -We are working on a fix here: [facebook/react #32119](https://github.com/facebook/react/pull/32119) +:::warning +`callback(...)` is deprecated. On react-native 0.81 and above it is a no-op, so upgrade to react-native 0.81 or newer and remove all `callback(...)` calls. ::: ### Recycling @@ -325,14 +333,14 @@ To call the function, you would need to get a reference to the `HybridObject` fi function App() { return ( { + hybridRef={(ref) => { const image = ref.takePhoto() - })} + }} /> ) } ``` -> Note: If you're wondering about the `callback(...)` syntax, see ["Callbacks have to be wrapped"](#callbacks-have-to-be-wrapped). +> Note: On react-native 0.78 - 0.80, this has to be wrapped in `callback(...)`, see ["Callbacks"](#callbacks). The `ref` from within `hybridRef`'s callback is pointing to the `HybridObject` directly - you can also pass this around freely. diff --git a/example/__tests__/nitro.views.harness.tsx b/example/__tests__/nitro.views.harness.tsx index 120bbef9e2..36c2d1f69b 100644 --- a/example/__tests__/nitro.views.harness.tsx +++ b/example/__tests__/nitro.views.harness.tsx @@ -177,11 +177,11 @@ describe('TestView', () => { viewRef.resolve(view))} + hybridRef={(view) => viewRef.resolve(view)} isBlue={true} hasBeenCalled={false} colorScheme="dark" - someCallback={callback(onSomeCallback)} + someCallback={onSomeCallback} onLayout={({ nativeEvent }) => layout.resolve(nativeEvent.layout)} />, { timeout: RENDER_TIMEOUT } @@ -209,6 +209,31 @@ describe('TestView', () => { expect(onSomeCallback).toHaveBeenCalledTimes(1) }) + it('still accepts callbacks wrapped in the deprecated `callback(...)`', async () => { + const viewRef = deferred() + const callbackFinished = deferred() + const onSomeCallback = fn(() => callbackFinished.resolve(undefined)) + + await render( + viewRef.resolve(view))} + isBlue={true} + hasBeenCalled={false} + colorScheme="dark" + someCallback={callback(onSomeCallback)} + />, + { timeout: RENDER_TIMEOUT } + ) + + const mountedView = await viewRef.promise + mountedView.someMethod() + await callbackFinished.promise + expect(mountedView.hasBeenCalled).toBe(true) + expect(onSomeCallback).toHaveBeenCalledTimes(1) + }) + it('updates every prop, changes pixels, and resizes the same native view', async () => { const initialRef = deferred() const initialCallback = fn() @@ -216,11 +241,11 @@ describe('TestView', () => { initialRef.resolve(view))} + hybridRef={(view) => initialRef.resolve(view)} isBlue={true} hasBeenCalled={false} colorScheme="dark" - someCallback={callback(initialCallback)} + someCallback={initialCallback} />, { timeout: RENDER_TIMEOUT } ) @@ -233,10 +258,7 @@ describe('TestView', () => { const updatedRef = deferred() const updatedCallbackFinished = deferred() const updatedCallback = fn(() => updatedCallbackFinished.resolve(undefined)) - const updatedHybridRef = callback((view: TestViewRef) => - updatedRef.resolve(view) - ) - const updatedSomeCallback = callback(updatedCallback) + const updatedHybridRef = (view: TestViewRef) => updatedRef.resolve(view) await renderResult.rerender( { isBlue={false} hasBeenCalled={true} colorScheme="light" - someCallback={updatedSomeCallback} + someCallback={updatedCallback} /> ) @@ -277,7 +299,7 @@ describe('TestView', () => { isBlue={false} hasBeenCalled={true} colorScheme="light" - someCallback={updatedSomeCallback} + someCallback={updatedCallback} onLayout={({ nativeEvent }) => resizedLayout.resolve(nativeEvent.layout) } @@ -296,12 +318,45 @@ describe('TestView', () => { expectRed(resizedCapture.pixelCoverage) }) - it('preserves an omitted native default while applying another prop', async () => { + it('keeps updating props after an optional callback prop is removed', async () => { const viewRef = deferred() - const stableHybridRef = callback((view: TestViewRef) => - viewRef.resolve(view) + const stableSomeCallback = fn() + const renderResult = await render( + viewRef.resolve(view)} + isBlue={true} + hasBeenCalled={false} + colorScheme="dark" + someCallback={stableSomeCallback} + />, + { timeout: RENDER_TIMEOUT } + ) + + const view = await viewRef.promise + expect(view.isBlue).toBe(true) + + // `hybridRef` is now `null` in the props payload - it must not throw. + await renderResult.rerender( + ) - const stableSomeCallback = callback(fn()) + + expect(view.isBlue).toBe(false) + expect(view.colorScheme).toBe('light') + }) + + it('preserves an omitted native default while applying another prop', async () => { + const viewRef = deferred() + const stableHybridRef = (view: TestViewRef) => viewRef.resolve(view) + const stableSomeCallback = fn() const renderResult = await render( { it('only calls native setters for changed Nitro props', async () => { const viewRef = deferred() - const stableHybridRef = callback((view: TestViewRef) => - viewRef.resolve(view) - ) - const stableSomeCallback = callback(fn()) + const stableHybridRef = (view: TestViewRef) => viewRef.resolve(view) + const stableSomeCallback = fn() const renderResult = await render( { firstRef.resolve(view))} + hybridRef={(view) => firstRef.resolve(view)} isBlue={true} hasBeenCalled={false} colorScheme="dark" - someCallback={callback(fn())} + someCallback={fn(() => {})} />, { timeout: RENDER_TIMEOUT } ) @@ -453,11 +506,11 @@ describe('TestView', () => { secondRef.resolve(view))} + hybridRef={(view) => secondRef.resolve(view)} isBlue={false} hasBeenCalled={true} colorScheme="light" - someCallback={callback(fn())} + someCallback={fn(() => {})} />, { timeout: RENDER_TIMEOUT } ) @@ -478,12 +531,10 @@ describe('multiple RecyclableTestViews', () => { it('keeps instances isolated while one is updated and recycled', async () => { const firstRef = deferred() const secondRef = deferred() - const firstHybridRef = callback((view: RecyclableTestViewRef) => + const firstHybridRef = (view: RecyclableTestViewRef) => firstRef.resolve(view) - ) - const secondHybridRef = callback((view: RecyclableTestViewRef) => + const secondHybridRef = (view: RecyclableTestViewRef) => secondRef.resolve(view) - ) const renderResult = await render( @@ -603,7 +654,7 @@ describe('multiple RecyclableTestViews', () => { key="first" testID="isolated-recyclable-view-first" style={INITIAL_SIZE} - hybridRef={callback((view) => remountedFirstRef.resolve(view))} + hybridRef={(view) => remountedFirstRef.resolve(view)} isBlue={true} /> { initialRef.resolve(view))} + hybridRef={(view) => initialRef.resolve(view)} isBlue={false} onLayout={({ nativeEvent }) => initialLayout.resolve(nativeEvent.layout) @@ -677,9 +728,9 @@ describe('RecyclableTestView', () => { expectRed(redCapture.pixelCoverage) const updatedRef = deferred() - const updatedHybridRef = callback((view: RecyclableTestViewRef) => + const updatedHybridRef = (view: RecyclableTestViewRef) => updatedRef.resolve(view) - ) + await renderResult.rerender( { firstRef.resolve(view))} + hybridRef={(view) => firstRef.resolve(view)} isBlue={true} />, { timeout: RENDER_TIMEOUT } @@ -766,7 +817,7 @@ describe('RecyclableTestView', () => { secondRef.resolve(view))} + hybridRef={(view) => secondRef.resolve(view)} isBlue={false} nativeDefaultValue={0} onLayout={({ nativeEvent }) => secondLayout.resolve(nativeEvent.layout)} @@ -815,7 +866,7 @@ describe('RecyclableTestView', () => { thirdRef.resolve(view))} + hybridRef={(view) => thirdRef.resolve(view)} isBlue={true} onLayout={({ nativeEvent }) => thirdLayout.resolve(nativeEvent.layout)} /> diff --git a/example/src/screens/ViewScreen.tsx b/example/src/screens/ViewScreen.tsx index c17b84864d..eaf42cc764 100644 --- a/example/src/screens/ViewScreen.tsx +++ b/example/src/screens/ViewScreen.tsx @@ -1,7 +1,7 @@ import * as React from 'react' import { StyleSheet, View, Text, Button, Platform } from 'react-native' -import { callback, NitroModules } from 'react-native-nitro-modules' +import { NitroModules } from 'react-native-nitro-modules' import { useSafeAreaInsets } from 'react-native-safe-area-context' import { useColors } from '../useColors' import { @@ -27,15 +27,15 @@ export function ViewScreenImpl() { return ( { + hybridRef={(ref) => { console.log(`Ref initialized!`) ref.someMethod() const isBlue = HybridTestObjectSwiftKotlin.getIsViewBlue(ref) console.log(`Is View blue: ${isBlue}`) - })} + }} style={styles.view} isBlue={i % 2 === 0} - someCallback={callback(() => console.log(`Callback called!`))} + someCallback={() => console.log(`Callback called!`)} colorScheme="dark" hasBeenCalled={false} onTouchEnd={() => { diff --git a/packages/react-native-nitro-modules/cpp/views/CachedProp.hpp b/packages/react-native-nitro-modules/cpp/views/CachedProp.hpp index ae9581abbe..84e1c430b8 100644 --- a/packages/react-native-nitro-modules/cpp/views/CachedProp.hpp +++ b/packages/react-native-nitro-modules/cpp/views/CachedProp.hpp @@ -98,15 +98,20 @@ class CachedProp final { auto [runtime, value] = static_cast>(*rawValue); +#if NITRO_RAW_FUNCTION_PROPS + // Every prop - including functions - arrives as the raw JSI value it was + // in JS, so there is nothing to unwrap here. + return CachedProp::fromJSIValue(*runtime, std::move(value), previousProp); +#else if constexpr (IsFunctionProp>::value) { - // React Native cannot transport functions as regular props. Nitrogen - // wraps them as `{ f: function }`, so we unwrap `f` before converting - // and caching the JSI value. - jsi::Value function = value.asObject(*runtime).getProperty(*runtime, PropNameIDCache::get(*runtime, "f")); - return CachedProp::fromJSIValue(*runtime, std::move(function), previousProp); + // This version of React Native cannot transport functions as props, so + // `callback(...)` wraps them as `{ f: function }` - unwrap `f` before + // converting and caching the JSI value. + return CachedProp::fromJSIValue(*runtime, unwrapWrappedCallback(*runtime, std::move(value)), previousProp); } else { return CachedProp::fromJSIValue(*runtime, std::move(value), previousProp); } +#endif } catch (const std::exception& exception) { throw std::runtime_error(std::string(viewName) + "." + propName + ": " + exception.what()); } @@ -122,6 +127,26 @@ class CachedProp final { } private: +#if !NITRO_RAW_FUNCTION_PROPS + /** + * Unwraps the actual JS function from a `{ f: function }` object created by + * `callback(...)`, which is how functions are passed to Nitro Views before + * React Native 0.81. + */ + static jsi::Value unwrapWrappedCallback(jsi::Runtime& runtime, jsi::Value&& value) { + if (value.isNull() || value.isUndefined()) { + // The function prop has been removed - there is nothing to unwrap. + return std::move(value); + } + if (!value.isObject()) { + throw std::runtime_error("Expected a function wrapped via `callback(...)`, but got `" + value.toString(runtime).utf8(runtime) + + "`! On react-native 0.78 - 0.80, function props have to be wrapped in `callback(...)`. " + "Alternatively, upgrade to react-native 0.81 or newer where Nitro passes functions to native directly."); + } + return value.asObject(runtime).getProperty(runtime, PropNameIDCache::get(runtime, "f")); + } +#endif + static CachedProp fromJSIValue(jsi::Runtime& runtime, jsi::Value&& value, const CachedProp& previousProp) { if (previousProp.equals(runtime, value)) { // jsi::Value hasn't changed - no need to convert it again! diff --git a/packages/react-native-nitro-modules/cpp/views/RawPropsCompat.hpp b/packages/react-native-nitro-modules/cpp/views/RawPropsCompat.hpp index 7e2b3d0689..b42601cdde 100644 --- a/packages/react-native-nitro-modules/cpp/views/RawPropsCompat.hpp +++ b/packages/react-native-nitro-modules/cpp/views/RawPropsCompat.hpp @@ -9,6 +9,24 @@ #endif #include +/** + * `NITRO_RAW_FUNCTION_PROPS` is `1` if React Native transports JS functions to + * native as raw JSI functions, and `0` if they have to be wrapped in objects. + * + * Before React Native 0.81, every function prop was converted to `true` before + * it reached native - Nitro works around that by wrapping functions in a + * `{ f: function }` object (see `callback(...)`). + * Since React Native 0.81, a View Config can opt out of that conversion by + * declaring a `process` function for the prop (which Nitro always does - see + * `getHostComponent.ts`), so functions arrive as raw `jsi::Function`s. + * @see https://github.com/facebook/react-native/pull/48777 + */ +#if defined(REACT_NATIVE_VERSION_MAJOR) && (REACT_NATIVE_VERSION_MAJOR > 0 || REACT_NATIVE_VERSION_MINOR >= 81) +#define NITRO_RAW_FUNCTION_PROPS 1 +#else +#define NITRO_RAW_FUNCTION_PROPS 0 +#endif + namespace margelo::nitro::RawPropsCompat { /** diff --git a/packages/react-native-nitro-modules/src/views/callback.ts b/packages/react-native-nitro-modules/src/views/callback.ts index 2cbf637eef..6e62beaa09 100644 --- a/packages/react-native-nitro-modules/src/views/callback.ts +++ b/packages/react-native-nitro-modules/src/views/callback.ts @@ -1,17 +1,44 @@ +import { Platform } from 'react-native' import type { NitroViewWrappedCallback } from './getHostComponent' -// TODO: Remove `callback(...)` wrapping once React Native supports passing down -// functions as raw jsi::Values instead of intercepting them for event handling. +let supportsRawFunctions: boolean | undefined + +/** + * Whether this version of react-native can transport JS functions to native as + * raw functions (react-native 0.81.0 and above), instead of converting them to `true`. + * + * This has to match the `NITRO_RAW_FUNCTION_PROPS` flag on the native side (see `RawPropsCompat.hpp`). + */ +function supportsRawFunctionProps(): boolean { + if (supportsRawFunctions == null) { + const version = Platform.constants?.reactNativeVersion + if (version == null) { + // We cannot determine the version of react-native (e.g. in a JS-only + // environment) - assume a recent version. + supportsRawFunctions = true + } else { + supportsRawFunctions = version.major > 0 || version.minor >= 81 + } + } + return supportsRawFunctions +} + /** * Wrap the given {@linkcode func} in a Nitro callback. + * - For react-native 0.81.0 and above, this just returns the function as-is. * - For older versions of react-native, this wraps the callback in a `{ f: T }` object. - * - For newer versions of react-native, this just returns the function as-is. + * + * @deprecated Since react-native 0.81.0, functions can be passed to Nitro Views + * directly - ` console.log(i)} />` instead of + * ` console.log(i))} />`. + * If you are still on react-native 0.78 - 0.80, upgrade to react-native 0.81 or + * newer and remove all `callback(...)` calls. */ export function callback( func: T -): T extends (...args: any[]) => any ? NitroViewWrappedCallback : T +): T extends (...args: any[]) => any ? T | NitroViewWrappedCallback : T export function callback(func: unknown) { - if (typeof func === 'function') { + if (typeof func === 'function' && !supportsRawFunctionProps()) { return { f: func } } return func diff --git a/packages/react-native-nitro-modules/src/views/getHostComponent.ts b/packages/react-native-nitro-modules/src/views/getHostComponent.ts index 5d5710800e..4d6217fa12 100644 --- a/packages/react-native-nitro-modules/src/views/getHostComponent.ts +++ b/packages/react-native-nitro-modules/src/views/getHostComponent.ts @@ -46,15 +46,16 @@ interface DefaultHybridViewProps { * function App() { * return ( * { + * hybridRef={(ref) => { * ref.current.scrollTo(400) - * })} + * }} * /> * ) * } * ``` - * @note If you're wondering about the `callback(...)` syntax, see - * ["Callbacks have to be wrapped"](https://nitro.margelo.com/docs/guides/view-components#callbacks-have-to-be-wrapped). + * @note On react-native 0.78 - 0.80, this has to be wrapped in + * {@linkcode callback | callback(...)}. See + * ["Callbacks"](https://nitro.margelo.com/docs/guides/view-components#callbacks). */ hybridRef?: (ref: RefType) => void } @@ -62,25 +63,28 @@ interface DefaultHybridViewProps { /** * Wraps a callback function in a Nitro-compatible object format. * - * @note Due to a React limitation, functions cannot be passed to native directly - * because RN converts them to booleans (`true`). As a workaround, - * Nitro requires you to wrap each function using `callback(...)`, - * which bypasses React Native's conversion. - * Please see the [Callbacks have to be wrapped](https://nitro.margelo.com/docs/guides/view-components#callbacks-have-to-be-wrapped) section for more information. + * @note Before react-native 0.81, functions could not be passed to native + * directly because react-native converted them to booleans (`true`). + * As a workaround, Nitro required you to wrap each function using `callback(...)`, + * which bypassed react-native's conversion. + * Since react-native 0.81, functions can be passed to Nitro Views directly, so + * this type is only relevant on react-native 0.78 - 0.80. + * Please see the [Callbacks](https://nitro.margelo.com/docs/guides/view-components#callbacks) section for more information. * * @type {Object} NitroViewWrappedCallback * @property {T} f - The wrapped callback function */ export type NitroViewWrappedCallback = { f: T } -// Due to a React limitation, functions cannot be passed to native directly -// because RN converts them to booleans (`true`). Nitro knows this and just -// wraps functions as objects - the original function is stored in `f`. -type WrapFunctionsInObjects = { +// Since react-native 0.81, functions can be passed to native directly. +// On react-native 0.78 - 0.80 they were converted to booleans (`true`), so +// Nitro also accepts functions wrapped in objects via `callback(...)` - the +// original function is then stored in `f`. +type AllowWrappedFunctions = { [K in keyof Props]: Props[K] extends Function - ? NitroViewWrappedCallback + ? Props[K] | NitroViewWrappedCallback : Props[K] extends Function | undefined - ? NitroViewWrappedCallback + ? Props[K] | NitroViewWrappedCallback : Props[K] } @@ -89,14 +93,15 @@ type WrapFunctionsInObjects = { * * @note Every React Native view has a {@linkcode DefaultHybridViewProps.hybridRef hybridRef} which can be used to gain access * to the underlying Nitro {@linkcode HybridView}. - * @note Every function/callback is wrapped as a `{ f: … }` object. Use {@linkcode callback | callback(...)} for this. + * @note On react-native 0.78 - 0.80, every function/callback has to be wrapped as a `{ f: … }` object. + * Use {@linkcode callback | callback(...)} for this. * @note Every method can be called on the Ref. Including setting properties directly. */ export type ReactNativeView< Props extends HybridViewProps, Methods extends HybridViewMethods, > = HostComponent< - WrapFunctionsInObjects< + AllowWrappedFunctions< DefaultHybridViewProps> & Props > & ViewProps @@ -106,6 +111,11 @@ type ValidAttributes = ViewConfig['validAttributes'] /** * Wraps all valid attributes of {@linkcode TProps} using Nitro's * default `diff` and `process` functions. + * + * Both are required for Nitro to receive props unchanged: + * - `diff` opts out of react-native's deep-differ, which ignores functions. + * - `process` opts out of react-native converting function props to `true` + * (react-native 0.81 and above). */ function wrapValidAttributes( attributes: ValidAttributes