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

[Fabric] Add dispatchCommand to React Native renderers #16085

Merged
merged 4 commits into from
Jul 8, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions packages/react-native-renderer/src/ReactFabric.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ import ReactSharedInternals from 'shared/ReactSharedInternals';
import getComponentName from 'shared/getComponentName';
import warningWithoutStack from 'shared/warningWithoutStack';

const {dispatchCommand: fabricDispatchCommand} = nativeFabricUIManager;

const ReactCurrentOwner = ReactSharedInternals.ReactCurrentOwner;

function findNodeHandle(componentOrHandle: any): ?number {
Expand Down Expand Up @@ -116,6 +118,26 @@ const ReactFabric: ReactFabricType = {
return;
},

dispatchCommand(handle: any, command: string, args: Array<any>) {
const invalid =
handle._nativeTag == null || handle._internalInstanceHandle == null;

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,
);
},

render(element: React$Element<any>, containerTag: any, callback: ?Function) {
let root = roots.get(containerTag);

Expand Down
13 changes: 13 additions & 0 deletions packages/react-native-renderer/src/ReactNativeRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,19 @@ const ReactNativeRenderer: ReactNativeType = {

findNodeHandle,

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',
);
return;
}

UIManager.dispatchViewManagerCommand(handle._nativeTag, command, args);
},

setNativeProps,

render(element: React$Element<any>, containerTag: any, callback: ?Function) {
Expand Down
2 changes: 2 additions & 0 deletions packages/react-native-renderer/src/ReactNativeTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ type SecretInternalsFabricType = {
export type ReactNativeType = {
NativeComponent: typeof ReactNativeComponent,
findNodeHandle(componentOrHandle: any): ?number,
dispatchCommand(handle: any, command: string, args: Array<any>): void,
setNativeProps(handle: any, nativeProps: Object): void,
render(
element: React$Element<any>,
Expand All @@ -147,6 +148,7 @@ export type ReactNativeType = {
export type ReactFabricType = {
NativeComponent: typeof ReactNativeComponent,
findNodeHandle(componentOrHandle: any): ?number,
dispatchCommand(handle: any, command: string, args: Array<any>): void,
setNativeProps(handle: any, nativeProps: Object): void,
render(
element: React$Element<any>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,8 @@ const RCTFabricUIManager = {
roots.set(rootTag, newChildSet);
}),

dispatchCommand: jest.fn(),

registerEventHandler: jest.fn(function registerEventHandler(callback) {}),

measure: jest.fn(function measure(node, callback) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ const RCTUIManager = {
viewName: viewName,
});
}),
dispatchViewManagerCommand: jest.fn(),
setJSResponder: jest.fn(),
setChildren: jest.fn(function setChildren(parentTag, reactTags) {
autoCreateRoot(parentTag);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ let NativeMethodsMixin;
const SET_NATIVE_PROPS_NOT_SUPPORTED_MESSAGE =
'Warning: setNativeProps is not currently supported in Fabric';

const DISPATCH_COMMAND_REQUIRES_HOST_COMPONENT =
"Warning: dispatchCommand was called with a ref that isn't a " +
'native component. Use React.forwardRef to get access to the underlying native component';

jest.mock('shared/ReactFeatureFlags', () =>
require('shared/forks/ReactFeatureFlags.native-oss'),
);
Expand Down Expand Up @@ -255,6 +259,85 @@ describe('ReactFabric', () => {
});
});

it('should call dispatchCommand for native refs', () => {
const View = createReactNativeComponentClass('RCTView', () => ({
validAttributes: {foo: true},
uiViewClassName: 'RCTView',
}));

[View].forEach(Component => {
nativeFabricUIManager.dispatchCommand.mockClear();

let viewRef;
ReactFabric.render(
<Component
ref={ref => {
viewRef = ref;
}}
/>,
11,
);

expect(nativeFabricUIManager.dispatchCommand).not.toBeCalled();
ReactFabric.dispatchCommand(viewRef, 'updateCommand', [10, 20]);
expect(nativeFabricUIManager.dispatchCommand).toHaveBeenCalledTimes(1);
expect(nativeFabricUIManager.dispatchCommand).toHaveBeenCalledWith(
expect.any(Object),
'updateCommand',
[10, 20],
);
});
});

it('should warn and no-op if calling dispatchCommand on non native refs', () => {
const View = createReactNativeComponentClass('RCTView', () => ({
validAttributes: {foo: true},
uiViewClassName: 'RCTView',
}));

class BasicClass extends React.Component {
render() {
return <React.Fragment />;
}
}

class Subclass extends ReactFabric.NativeComponent {
render() {
return <View />;
}
}

const CreateClass = createReactClass({
mixins: [NativeMethodsMixin],
render: () => {
return <View />;
},
});

[BasicClass, Subclass, CreateClass].forEach(Component => {
nativeFabricUIManager.dispatchCommand.mockReset();

let viewRef;
ReactFabric.render(
<Component
ref={ref => {
viewRef = ref;
}}
/>,
11,
);

expect(nativeFabricUIManager.dispatchCommand).not.toBeCalled();
expect(() => {
ReactFabric.dispatchCommand(viewRef, 'updateCommand', [10, 20]);
}).toWarnDev([DISPATCH_COMMAND_REQUIRES_HOST_COMPONENT], {
withoutStack: true,
});

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

it('setNativeProps on native refs should no-op', () => {
const View = createReactNativeComponentClass('RCTView', () => ({
validAttributes: {foo: true},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,26 @@ describe('ReactFabric', () => {
expect(handle).toBe(2);
});

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

let ref = React.createRef();

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

it('sets native props with setNativeProps on Fabric nodes with the RN renderer', () => {
UIManager.updateView.mockReset();
const View = createReactNativeComponentClass('RCTView', () => ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ let createReactNativeComponentClass;
let UIManager;
let NativeMethodsMixin;

const DISPATCH_COMMAND_REQUIRES_HOST_COMPONENT =
"Warning: dispatchCommand was called with a ref that isn't a " +
'native component. Use React.forwardRef to get access to the underlying native component';

const SET_NATIVE_PROPS_DEPRECATION_MESSAGE =
'Warning: Calling ref.setNativeProps(nativeProps) ' +
'is deprecated and will be removed in a future release. ' +
Expand Down Expand Up @@ -108,6 +112,85 @@ describe('ReactNative', () => {
expect(UIManager.updateView).toHaveBeenCalledTimes(4);
});

it('should call dispatchCommand for native refs', () => {
const View = createReactNativeComponentClass('RCTView', () => ({
validAttributes: {foo: true},
uiViewClassName: 'RCTView',
}));

[View].forEach(Component => {
UIManager.dispatchViewManagerCommand.mockClear();

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

expect(UIManager.dispatchViewManagerCommand).not.toBeCalled();
ReactNative.dispatchCommand(viewRef, 'updateCommand', [10, 20]);
expect(UIManager.dispatchViewManagerCommand).toHaveBeenCalledTimes(1);
expect(UIManager.dispatchViewManagerCommand).toHaveBeenCalledWith(
expect.any(Number),
'updateCommand',
[10, 20],
);
});
});

it('should warn and no-op if calling dispatchCommand on non native refs', () => {
const View = createReactNativeComponentClass('RCTView', () => ({
validAttributes: {foo: true},
uiViewClassName: 'RCTView',
}));

class BasicClass extends React.Component {
render() {
return <React.Fragment />;
}
}

class Subclass extends ReactNative.NativeComponent {
render() {
return <View />;
}
}

const CreateClass = createReactClass({
mixins: [NativeMethodsMixin],
render: () => {
return <View />;
},
});

[BasicClass, Subclass, CreateClass].forEach(Component => {
UIManager.dispatchViewManagerCommand.mockReset();

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

expect(UIManager.dispatchViewManagerCommand).not.toBeCalled();
expect(() => {
ReactNative.dispatchCommand(viewRef, 'updateCommand', [10, 20]);
}).toWarnDev([DISPATCH_COMMAND_REQUIRES_HOST_COMPONENT], {
withoutStack: true,
});

expect(UIManager.dispatchViewManagerCommand).not.toBeCalled();
});
});

it('should not call UIManager.updateView from ref.setNativeProps for properties that have not changed', () => {
const View = createReactNativeComponentClass('RCTView', () => ({
validAttributes: {foo: true},
Expand Down
7 changes: 7 additions & 0 deletions scripts/flow/react-native-host-hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ declare module 'react-native/Libraries/ReactPrivate/ReactNativePrivateInterface'
rootTag: number,
props: ?Object,
) => void,
dispatchViewManagerCommand: (
reactTag: number,
command: string,
args: Array<any>,
) => void,
manageChildren: (
containerTag: number,
moveFromIndices: Array<number>,
Expand Down Expand Up @@ -120,6 +125,8 @@ declare var nativeFabricUIManager: {
) => void,
) => void,

dispatchCommand: (node: Object, command: string, args: Array<any>) => void,

measure: (node: Node, callback: MeasureOnSuccessCallback) => void,
measureInWindow: (
node: Node,
Expand Down