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.

Sign up for GitHub

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

[Native] Enable and remove targetAsInstance feature flag. #18182

Merged
merged 1 commit into from
Feb 28, 2020
Merged

[Native] Enable and remove targetAsInstance feature flag. #18182

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 6 additions & 14 deletions packages/react-native-renderer/src/ReactFabricComponentTree.js
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
107 changes: 0 additions & 107 deletions packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js
View file
Open in desktop
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
79 changes: 0 additions & 79 deletions packages/react-native-renderer/src/__tests__/ReactNativeEvents-test.internal.js
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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