Skip to content

Commit

Permalink
[Native] Enable and remove targetAsInstance feature flag. (#18182)
Browse files Browse the repository at this point in the history
  • Loading branch information
elicwhite committed Feb 28, 2020
1 parent 4469700 commit 26aa198
Show file tree
Hide file tree
Showing 17 changed files with 20 additions and 253 deletions.
20 changes: 6 additions & 14 deletions packages/react-native-renderer/src/ReactFabricComponentTree.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,25 +7,17 @@

import invariant from 'shared/invariant';

import {enableNativeTargetAsInstance} from 'shared/ReactFeatureFlags';

function getInstanceFromInstance(instanceHandle) {
return instanceHandle;
}

function getTagFromInstance(inst) {
if (enableNativeTargetAsInstance) {
let nativeInstance = inst.stateNode.canonical;
invariant(
nativeInstance._nativeTag,
'All native instances should have a tag.',
);
return nativeInstance;
} else {
let tag = inst.stateNode.canonical._nativeTag;
invariant(tag, 'All native instances should have a tag.');
return tag;
}
let nativeInstance = inst.stateNode.canonical;
invariant(
nativeInstance._nativeTag,
'All native instances should have a tag.',
);
return nativeInstance;
}

export {
Expand Down
15 changes: 5 additions & 10 deletions packages/react-native-renderer/src/ReactFabricEventEmitter.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import {registrationNameModules} from 'legacy-events/EventPluginRegistry';
import {batchedUpdates} from 'legacy-events/ReactGenericBatching';
import accumulateInto from 'legacy-events/accumulateInto';

import {enableNativeTargetAsInstance} from 'shared/ReactFeatureFlags';
import {plugins} from 'legacy-events/EventPluginRegistry';
import getListener from 'legacy-events/getListener';
import {runEventsInBatch} from 'legacy-events/EventBatching';
Expand Down Expand Up @@ -85,16 +84,12 @@ export function dispatchEvent(
const targetFiber = (target: null | Fiber);

let eventTarget = null;
if (enableNativeTargetAsInstance) {
if (targetFiber != null) {
const stateNode = targetFiber.stateNode;
// Guard against Fiber being unmounted
if (stateNode != null) {
eventTarget = stateNode.canonical;
}
if (targetFiber != null) {
const stateNode = targetFiber.stateNode;
// Guard against Fiber being unmounted
if (stateNode != null) {
eventTarget = stateNode.canonical;
}
} else {
eventTarget = nativeEvent.target;
}

batchedUpdates(function() {
Expand Down
25 changes: 7 additions & 18 deletions packages/react-native-renderer/src/ReactNativeComponentTree.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@

import invariant from 'shared/invariant';

import {enableNativeTargetAsInstance} from 'shared/ReactFeatureFlags';

const instanceCache = new Map();
const instanceProps = new Map();

Expand All @@ -26,23 +24,14 @@ function getInstanceFromTag(tag) {
}

function getTagFromInstance(inst) {
if (enableNativeTargetAsInstance) {
let nativeInstance = inst.stateNode;
let tag = nativeInstance._nativeTag;
if (tag === undefined) {
nativeInstance = nativeInstance.canonical;
tag = nativeInstance._nativeTag;
}
invariant(tag, 'All native instances should have a tag.');
return nativeInstance;
} else {
let tag = inst.stateNode._nativeTag;
if (tag === undefined) {
tag = inst.stateNode.canonical._nativeTag;
}
invariant(tag, 'All native instances should have a tag.');
return tag;
let nativeInstance = inst.stateNode;
let tag = nativeInstance._nativeTag;
if (tag === undefined) {
nativeInstance = nativeInstance.canonical;
tag = nativeInstance._nativeTag;
}
invariant(tag, 'All native instances should have a tag.');
return nativeInstance;
}

export {
Expand Down
9 changes: 2 additions & 7 deletions packages/react-native-renderer/src/ReactNativeEventEmitter.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import {PLUGIN_EVENT_SYSTEM} from 'legacy-events/EventSystemFlags';
import {registrationNameModules} from 'legacy-events/EventPluginRegistry';
import {batchedUpdates} from 'legacy-events/ReactGenericBatching';
import {runEventsInBatch} from 'legacy-events/EventBatching';
import {enableNativeTargetAsInstance} from 'shared/ReactFeatureFlags';
import {plugins} from 'legacy-events/EventPluginRegistry';
import getListener from 'legacy-events/getListener';
import accumulateInto from 'legacy-events/accumulateInto';
Expand Down Expand Up @@ -104,12 +103,8 @@ function _receiveRootNodeIDEvent(
const inst = getInstanceFromNode(rootNodeID);

let target = null;
if (enableNativeTargetAsInstance) {
if (inst != null) {
target = inst.stateNode;
}
} else {
target = nativeEvent.target;
if (inst != null) {
target = inst.stateNode;
}

batchedUpdates(function() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@

let React;
let ReactFabric;
let ReactFeatureFlags;
let createReactNativeComponentClass;
let UIManager;
let StrictMode;
Expand All @@ -38,7 +37,6 @@ describe('ReactFabric', () => {
React = require('react');
StrictMode = React.StrictMode;
ReactFabric = require('react-native-renderer/fabric');
ReactFeatureFlags = require('shared/ReactFeatureFlags');
UIManager = require('react-native/Libraries/ReactPrivate/ReactNativePrivateInterface')
.UIManager;
createReactNativeComponentClass = require('react-native/Libraries/ReactPrivate/ReactNativePrivateInterface')
Expand Down Expand Up @@ -649,112 +647,7 @@ describe('ReactFabric', () => {
expect(touchStart2).toBeCalled();
});

it('dispatches event with target as reactTag', () => {
ReactFeatureFlags.enableNativeTargetAsInstance = false;

const View = createReactNativeComponentClass('RCTView', () => ({
validAttributes: {
id: true,
},
uiViewClassName: 'RCTView',
directEventTypes: {
topTouchStart: {
registrationName: 'onTouchStart',
},
topTouchEnd: {
registrationName: 'onTouchEnd',
},
},
}));

function getViewById(id) {
const [
reactTag,
,
,
,
instanceHandle,
] = nativeFabricUIManager.createNode.mock.calls.find(
args => args[3] && args[3].id === id,
);

return {reactTag, instanceHandle};
}

const ref1 = React.createRef();
const ref2 = React.createRef();

ReactFabric.render(
<View id="parent">
<View
ref={ref1}
id="one"
onResponderStart={event => {
expect(ref1.current).not.toBeNull();
expect(ReactFabric.findNodeHandle(ref1.current)).toEqual(
event.target,
);
expect(ReactFabric.findNodeHandle(ref1.current)).toEqual(
event.currentTarget,
);
}}
onStartShouldSetResponder={() => true}
/>
<View
ref={ref2}
id="two"
onResponderStart={event => {
expect(ref2.current).not.toBeNull();
expect(ReactFabric.findNodeHandle(ref2.current)).toEqual(
event.target,
);
expect(ReactFabric.findNodeHandle(ref2.current)).toEqual(
event.currentTarget,
);
}}
onStartShouldSetResponder={() => true}
/>
</View>,
1,
);

let [
dispatchEvent,
] = nativeFabricUIManager.registerEventHandler.mock.calls[0];

dispatchEvent(getViewById('one').instanceHandle, 'topTouchStart', {
target: getViewById('one').reactTag,
identifier: 17,
touches: [],
changedTouches: [],
});
dispatchEvent(getViewById('one').instanceHandle, 'topTouchEnd', {
target: getViewById('one').reactTag,
identifier: 17,
touches: [],
changedTouches: [],
});

dispatchEvent(getViewById('two').instanceHandle, 'topTouchStart', {
target: getViewById('two').reactTag,
identifier: 17,
touches: [],
changedTouches: [],
});

dispatchEvent(getViewById('two').instanceHandle, 'topTouchEnd', {
target: getViewById('two').reactTag,
identifier: 17,
touches: [],
changedTouches: [],
});

expect.assertions(6);
});

it('dispatches event with target as instance', () => {
ReactFeatureFlags.enableNativeTargetAsInstance = true;

const View = createReactNativeComponentClass('RCTView', () => ({
validAttributes: {
id: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ let PropTypes;
let RCTEventEmitter;
let React;
let ReactNative;
let ReactFeatureFlags;
let ResponderEventPlugin;
let UIManager;
let createReactNativeComponentClass;
Expand Down Expand Up @@ -69,7 +68,6 @@ beforeEach(() => {
.RCTEventEmitter;
React = require('react');
ReactNative = require('react-native-renderer');
ReactFeatureFlags = require('shared/ReactFeatureFlags');
ResponderEventPlugin = require('legacy-events/ResponderEventPlugin').default;
UIManager = require('react-native/Libraries/ReactPrivate/ReactNativePrivateInterface')
.UIManager;
Expand Down Expand Up @@ -459,84 +457,7 @@ it('handles events without target', () => {
]);
});

it('dispatches event with target as reactTag', () => {
ReactFeatureFlags.enableNativeTargetAsInstance = false;
const EventEmitter = RCTEventEmitter.register.mock.calls[0][0];

const View = fakeRequireNativeComponent('View', {id: true});

function getViewById(id) {
return UIManager.createView.mock.calls.find(
args => args[3] && args[3].id === id,
)[0];
}

const ref1 = React.createRef();
const ref2 = React.createRef();

ReactNative.render(
<View id="parent">
<View
ref={ref1}
id="one"
onResponderStart={event => {
expect(ref1.current).not.toBeNull();
expect(ReactNative.findNodeHandle(ref1.current)).toEqual(
event.target,
);
expect(ReactNative.findNodeHandle(ref1.current)).toEqual(
event.currentTarget,
);
}}
onStartShouldSetResponder={() => true}
/>
<View
ref={ref2}
id="two"
onResponderStart={event => {
expect(ref2.current).not.toBeNull();
expect(ReactNative.findNodeHandle(ref2.current)).toEqual(
event.target,
);
expect(ReactNative.findNodeHandle(ref2.current)).toEqual(
event.currentTarget,
);
}}
onStartShouldSetResponder={() => true}
/>
</View>,
1,
);

EventEmitter.receiveTouches(
'topTouchStart',
[{target: getViewById('one'), identifier: 17}],
[0],
);

EventEmitter.receiveTouches(
'topTouchEnd',
[{target: getViewById('one'), identifier: 17}],
[0],
);

EventEmitter.receiveTouches(
'topTouchStart',
[{target: getViewById('two'), identifier: 18}],
[0],
);

EventEmitter.receiveTouches(
'topTouchEnd',
[{target: getViewById('two'), identifier: 18}],
[0],
);

expect.assertions(6);
});

it('dispatches event with target as instance', () => {
ReactFeatureFlags.enableNativeTargetAsInstance = true;
const EventEmitter = RCTEventEmitter.register.mock.calls[0][0];

const View = fakeRequireNativeComponent('View', {id: true});
Expand Down
3 changes: 0 additions & 3 deletions packages/shared/ReactFeatureFlags.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,6 @@ export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;

export const enableTrustedTypesIntegration = false;

// Flag to turn event.target and event.currentTarget in ReactNative from a reactTag to a component instance
export const enableNativeTargetAsInstance = false;

// Controls sequence of passive effect destroy and create functions.
// If this flag is off, destroy and create functions may be interleaved.
// When the flag is on, all destroy functions will be run (for all fibers)
Expand Down
5 changes: 0 additions & 5 deletions packages/shared/forks/ReactFeatureFlags.native-fb.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,6 @@ import invariant from 'shared/invariant';
import typeof * as FeatureFlagsType from 'shared/ReactFeatureFlags';
import typeof * as ExportsType from './ReactFeatureFlags.native-fb';

// Uncomment to re-export dynamic flags from the fbsource version.
export const {
enableNativeTargetAsInstance,
} = require('../shims/ReactFeatureFlags');

// The rest of the flags are static for better dead code elimination.
export const enableUserTimingAPI = __DEV__;
export const enableProfilerTimer = __PROFILE__;
Expand Down
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.native-oss.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ export const warnAboutStringRefs = false;
export const disableLegacyContext = false;
export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
export const enableTrustedTypesIntegration = false;
export const enableNativeTargetAsInstance = false;
export const disableTextareaChildren = false;
export const disableMapsAsChildren = false;
export const warnUnstableRenderSubtreeIntoContainer = false;
Expand Down
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.persistent.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ export const warnAboutStringRefs = false;
export const disableLegacyContext = false;
export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
export const enableTrustedTypesIntegration = false;
export const enableNativeTargetAsInstance = false;
export const disableTextareaChildren = false;
export const disableMapsAsChildren = false;
export const warnUnstableRenderSubtreeIntoContainer = false;
Expand Down
Loading

0 comments on commit 26aa198

Please sign in to comment.