Skip to content

Commit

Permalink
Revert "Revert "Dispatch commands to both UIManagers from both render…
Browse files Browse the repository at this point in the history
…ers (facebook#17211)" (facebook#17232)" (facebook#17799)

* Revert "Revert "Dispatch commands to both UIManagers from both renderers (facebook#17211)" (facebook#17232)"

This reverts commit d0fc0ba.

* Clean up another __DEV__ warning check
  • Loading branch information
elicwhite committed Jan 8, 2020
1 parent 79aa563 commit d47c894
Show file tree
Hide file tree
Showing 3 changed files with 119 additions and 24 deletions.
34 changes: 19 additions & 15 deletions packages/react-native-renderer/src/ReactFabric.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ import {createPortal} from 'shared/ReactPortal';
import {setBatchingImplementation} from 'legacy-events/ReactGenericBatching';
import ReactVersion from 'shared/ReactVersion';

// Module provided by RN:
import {UIManager} from 'react-native/Libraries/ReactPrivate/ReactNativePrivateInterface';

import NativeMethodsMixin from './NativeMethodsMixin';
import ReactNativeComponent from './ReactNativeComponent';
import {getClosestInstanceFromNode} from './ReactFabricComponentTree';
Expand All @@ -39,8 +42,6 @@ import ReactSharedInternals from 'shared/ReactSharedInternals';
import getComponentName from 'shared/getComponentName';
import warningWithoutStack from 'shared/warningWithoutStack';

const {dispatchCommand: fabricDispatchCommand} = nativeFabricUIManager;

const ReactCurrentOwner = ReactSharedInternals.ReactCurrentOwner;

function findHostInstance_DEPRECATED(
Expand Down Expand Up @@ -161,23 +162,26 @@ const ReactFabric: ReactFabricType = {
findNodeHandle,

dispatchCommand(handle: any, command: string, args: Array<any>) {
const invalid =
handle._nativeTag == null || handle._internalInstanceHandle == null;
if (handle._nativeTag == null) {
if (__DEV__) {
console.error(
"dispatchCommand was called with a ref that isn't a " +
'native component. Use React.forwardRef to get access to the underlying native component',
);
}

if (invalid) {
warningWithoutStack(
!invalid,
"dispatchCommand was called with a ref that isn't a " +
'native component. Use React.forwardRef to get access to the underlying native component',
);
return;
}

fabricDispatchCommand(
handle._internalInstanceHandle.stateNode.node,
command,
args,
);
if (handle._internalInstanceHandle) {
nativeFabricUIManager.dispatchCommand(
handle._internalInstanceHandle.stateNode.node,
command,
args,
);
} else {
UIManager.dispatchViewManagerCommand(handle._nativeTag, command, args);
}
},

render(element: React$Element<any>, containerTag: any, callback: ?Function) {
Expand Down
21 changes: 15 additions & 6 deletions packages/react-native-renderer/src/ReactNativeRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -172,15 +172,24 @@ const ReactNativeRenderer: ReactNativeType = {

dispatchCommand(handle: any, command: string, args: Array<any>) {
if (handle._nativeTag == null) {
warningWithoutStack(
handle._nativeTag != null,
"dispatchCommand was called with a ref that isn't a " +
'native component. Use React.forwardRef to get access to the underlying native component',
);
if (__DEV__) {
console.error(
"dispatchCommand was called with a ref that isn't a " +
'native component. Use React.forwardRef to get access to the underlying native component',
);
}
return;
}

UIManager.dispatchViewManagerCommand(handle._nativeTag, command, args);
if (handle._internalInstanceHandle) {
nativeFabricUIManager.dispatchCommand(
handle._internalInstanceHandle.stateNode.node,
command,
args,
);
} else {
UIManager.dispatchViewManagerCommand(handle._nativeTag, command, args);
}
},

render(element: React$Element<any>, containerTag: any, callback: ?Function) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ let ReactNative;
let UIManager;
let createReactNativeComponentClass;

describe('ReactFabric', () => {
describe('created with ReactFabric called with ReactNative', () => {
beforeEach(() => {
jest.resetModules();
require('react-native/Libraries/ReactPrivate/InitializeNativeFabricUIManager');
Expand Down Expand Up @@ -75,7 +75,7 @@ describe('ReactFabric', () => {
});

it('dispatches commands on Fabric nodes with the RN renderer', () => {
UIManager.dispatchViewManagerCommand.mockReset();
nativeFabricUIManager.dispatchCommand.mockClear();
const View = createReactNativeComponentClass('RCTView', () => ({
validAttributes: {title: true},
uiViewClassName: 'RCTView',
Expand All @@ -84,13 +84,95 @@ describe('ReactFabric', () => {
let ref = React.createRef();

ReactFabric.render(<View title="bar" ref={ref} />, 11);
expect(UIManager.dispatchViewManagerCommand).not.toBeCalled();
expect(nativeFabricUIManager.dispatchCommand).not.toBeCalled();
ReactNative.dispatchCommand(ref.current, 'myCommand', [10, 20]);
expect(nativeFabricUIManager.dispatchCommand).toHaveBeenCalledTimes(1);
expect(nativeFabricUIManager.dispatchCommand).toHaveBeenCalledWith(
expect.any(Object),
'myCommand',
[10, 20],
);
expect(UIManager.dispatchViewManagerCommand).not.toBeCalled();
});
});

describe('created with ReactNative called with ReactFabric', () => {
beforeEach(() => {
jest.resetModules();
require('react-native/Libraries/ReactPrivate/InitializeNativeFabricUIManager');
ReactFabric = require('react-native-renderer/fabric');
jest.resetModules();
UIManager = require('react-native/Libraries/ReactPrivate/ReactNativePrivateInterface')
.UIManager;
jest.mock('shared/ReactFeatureFlags', () =>
require('shared/forks/ReactFeatureFlags.native-oss'),
);
ReactNative = require('react-native-renderer');

React = require('react');
createReactNativeComponentClass = require('react-native/Libraries/ReactPrivate/ReactNativePrivateInterface')
.ReactNativeViewConfigRegistry.register;
});

it('find Paper instances with the Fabric renderer', () => {
const View = createReactNativeComponentClass('RCTView', () => ({
validAttributes: {title: true},
uiViewClassName: 'RCTView',
}));

let ref = React.createRef();

class Component extends React.Component {
render() {
return <View title="foo" />;
}
}

ReactNative.render(<Component ref={ref} />, 11);

let instance = ReactFabric.findHostInstance_DEPRECATED(ref.current);
expect(instance._nativeTag).toBe(3);
});

it('find Paper nodes with the Fabric renderer', () => {
const View = createReactNativeComponentClass('RCTView', () => ({
validAttributes: {title: true},
uiViewClassName: 'RCTView',
}));

let ref = React.createRef();

class Component extends React.Component {
render() {
return <View title="foo" />;
}
}

ReactNative.render(<Component ref={ref} />, 11);

let handle = ReactFabric.findNodeHandle(ref.current);
expect(handle).toBe(3);
});

it('dispatches commands on Paper nodes with the Fabric renderer', () => {
UIManager.dispatchViewManagerCommand.mockReset();
const View = createReactNativeComponentClass('RCTView', () => ({
validAttributes: {title: true},
uiViewClassName: 'RCTView',
}));

let ref = React.createRef();

ReactNative.render(<View title="bar" ref={ref} />, 11);
expect(UIManager.dispatchViewManagerCommand).not.toBeCalled();
ReactFabric.dispatchCommand(ref.current, 'myCommand', [10, 20]);
expect(UIManager.dispatchViewManagerCommand).toHaveBeenCalledTimes(1);
expect(UIManager.dispatchViewManagerCommand).toHaveBeenCalledWith(
expect.any(Number),
'myCommand',
[10, 20],
);

expect(nativeFabricUIManager.dispatchCommand).not.toBeCalled();
});
});

0 comments on commit d47c894

Please sign in to comment.