From 5a01f5f6a9b55b1771fa5ebb2aa5d3ab1afc0297 Mon Sep 17 00:00:00 2001 From: Ben Alpert Date: Tue, 31 Mar 2015 15:26:08 -0700 Subject: [PATCH] Fix ResponderEventPlugin to work in core React again Test Plan: jest, grunt fasttest --- src/core/ReactInstanceHandles.js | 9 ++---- .../__tests__/ReactInstanceHandles-test.js | 2 +- src/event/EventPluginUtils.js | 12 +++++-- src/event/EventPropagators.js | 9 ++++-- .../eventPlugins/ResponderEventPlugin.js | 15 +++++---- .../eventPlugins/ResponderSyntheticEvent.js | 4 +-- .../ResponderTouchHistoryStore.js | 8 ++--- .../__tests__/ResponderEventPlugin-test.js | 31 +++++-------------- 8 files changed, 41 insertions(+), 49 deletions(-) diff --git a/src/core/ReactInstanceHandles.js b/src/core/ReactInstanceHandles.js index 9a5a114c0c..f8a30ec7a8 100644 --- a/src/core/ReactInstanceHandles.js +++ b/src/core/ReactInstanceHandles.js @@ -115,7 +115,8 @@ function getNextDescendantID(ancestorID, destinationID) { // Skip over the ancestor and the immediate separator. Traverse until we hit // another separator or we reach the end of `destinationID`. var start = ancestorID.length + SEPARATOR_LENGTH; - for (var i = start; i < destinationID.length; i++) { + var i; + for (i = start; i < destinationID.length; i++) { if (isBoundary(destinationID, i)) { break; } @@ -320,11 +321,7 @@ var ReactInstanceHandles = { traverseParentPath('', targetID, cb, arg, true, false); }, - /** - * Exposed for unit testing. - * @private - */ - _getFirstCommonAncestorID: getFirstCommonAncestorID, + getFirstCommonAncestorID: getFirstCommonAncestorID, /** * Exposed for unit testing. diff --git a/src/core/__tests__/ReactInstanceHandles-test.js b/src/core/__tests__/ReactInstanceHandles-test.js index 2ecf5edcaf..58a50cfc8e 100644 --- a/src/core/__tests__/ReactInstanceHandles-test.js +++ b/src/core/__tests__/ReactInstanceHandles-test.js @@ -408,7 +408,7 @@ describe('ReactInstanceHandles', function() { var i; for (i = 0; i < ancestors.length; i++) { var plan = ancestors[i]; - var firstCommon = ReactInstanceHandles._getFirstCommonAncestorID( + var firstCommon = ReactInstanceHandles.getFirstCommonAncestorID( getNodeID(plan.one), getNodeID(plan.two) ); diff --git a/src/event/EventPluginUtils.js b/src/event/EventPluginUtils.js index b04883b15f..3b5fc634ec 100644 --- a/src/event/EventPluginUtils.js +++ b/src/event/EventPluginUtils.js @@ -30,9 +30,9 @@ var injection = { injection.Mount = InjectedMount; if (__DEV__) { warning( - InjectedMount && InjectedMount.getNode, + InjectedMount && InjectedMount.getNode && InjectedMount.getID, 'EventPluginUtils.injection.injectMount(...): Injected Mount ' + - 'module is missing getNode.' + 'module is missing getNode or getID.' ); } } @@ -211,6 +211,14 @@ var EventPluginUtils = { executeDispatchesInOrder: executeDispatchesInOrder, executeDispatchesInOrderStopAtTrue: executeDispatchesInOrderStopAtTrue, hasDispatches: hasDispatches, + + getNode: function(id) { + return injection.Mount.getNode(id); + }, + getID: function(node) { + return injection.Mount.getID(node); + }, + injection: injection }; diff --git a/src/event/EventPropagators.js b/src/event/EventPropagators.js index d77e024065..257266ab0e 100644 --- a/src/event/EventPropagators.js +++ b/src/event/EventPropagators.js @@ -14,6 +14,8 @@ var EventConstants = require('EventConstants'); var EventPluginHub = require('EventPluginHub'); +var warning = require('warning'); + var accumulateInto = require('accumulateInto'); var forEachAccumulated = require('forEachAccumulated'); @@ -38,9 +40,10 @@ function listenerAtPhase(id, event, propagationPhase) { */ function accumulateDirectionalDispatches(domID, upwards, event) { if (__DEV__) { - if (!domID) { - throw new Error('Dispatching id must not be null'); - } + warning( + domID, + 'Dispatching id must not be null' + ); } var phase = upwards ? PropagationPhases.bubbled : PropagationPhases.captured; var listener = listenerAtPhase(domID, event, phase); diff --git a/src/event/eventPlugins/ResponderEventPlugin.js b/src/event/eventPlugins/ResponderEventPlugin.js index 391ed01a93..3dcaa5d5d2 100644 --- a/src/event/eventPlugins/ResponderEventPlugin.js +++ b/src/event/eventPlugins/ResponderEventPlugin.js @@ -14,7 +14,6 @@ var EventConstants = require('EventConstants'); var EventPluginUtils = require('EventPluginUtils'); var EventPropagators = require('EventPropagators'); -var NodeHandle = require('NodeHandle'); var ReactInstanceHandles = require('ReactInstanceHandles'); var ResponderSyntheticEvent = require('ResponderSyntheticEvent'); var ResponderTouchHistoryStore = require('ResponderTouchHistoryStore'); @@ -133,7 +132,7 @@ var eventTypes = { * ---------------- * * - A global, solitary "interaction lock" on a view. - * - If a `NodeHandle` becomes the responder, it should convey visual feedback + * - If a node becomes the responder, it should convey visual feedback * immediately to indicate so, either by highlighting or moving accordingly. * - To be the responder means, that touches are exclusively important to that * responder view, and no other view. @@ -337,7 +336,7 @@ function setResponderAndExtractTransfer( // TODO: stop one short of the the current responder. var bubbleShouldSetFrom = !responderID ? topLevelTargetID : - ReactInstanceHandles._getFirstCommonAncestorID(responderID, topLevelTargetID); + ReactInstanceHandles.getFirstCommonAncestorID(responderID, topLevelTargetID); // When capturing/bubbling the "shouldSet" event, we want to skip the target // (deepest ID) if it happens to be the current responder. The reasoning: @@ -450,12 +449,12 @@ function noResponderTouches(nativeEvent) { var target = activeTouch.target; if (target !== null && target !== undefined && target !== 0) { // Is the original touch location inside of the current responder? - var commonAncestor = - ReactInstanceHandles._getFirstCommonAncestorID( + var isAncestor = + ReactInstanceHandles.isAncestorIDOf( responderID, - NodeHandle.getRootNodeID(target) + EventPluginUtils.getID(target) ); - if (commonAncestor === responderID) { + if (isAncestor) { return false; } } @@ -585,7 +584,7 @@ var ResponderEventPlugin = { */ injectGlobalInteractionHandler: function(GlobalInteractionHandler) { ResponderEventPlugin.GlobalInteractionHandler = GlobalInteractionHandler; - }, + } } }; diff --git a/src/event/eventPlugins/ResponderSyntheticEvent.js b/src/event/eventPlugins/ResponderSyntheticEvent.js index 38245d805a..6a8b0260ad 100644 --- a/src/event/eventPlugins/ResponderSyntheticEvent.js +++ b/src/event/eventPlugins/ResponderSyntheticEvent.js @@ -10,7 +10,7 @@ * @typechecks static-only */ -"use strict"; +'use strict'; var SyntheticEvent = require('SyntheticEvent'); @@ -22,7 +22,7 @@ var SyntheticEvent = require('SyntheticEvent'); var ResponderEventInterface = { touchHistory: function(nativeEvent) { return null; // Actually doesn't even look at the native event. - }, + } }; /** diff --git a/src/event/eventPlugins/ResponderTouchHistoryStore.js b/src/event/eventPlugins/ResponderTouchHistoryStore.js index 1770464441..00f99255db 100644 --- a/src/event/eventPlugins/ResponderTouchHistoryStore.js +++ b/src/event/eventPlugins/ResponderTouchHistoryStore.js @@ -9,7 +9,7 @@ * @providesModule ResponderTouchHistoryStore */ -"use strict"; +'use strict'; var EventPluginUtils = require('EventPluginUtils'); @@ -43,7 +43,7 @@ var touchHistory = { // us having to loop through all of the touches all the time in the most // common case. indexOfSingleActiveTouch: -1, - mostRecentTimeStamp: 0, + mostRecentTimeStamp: 0 }; var timestampForTouch = function(touch) { @@ -69,7 +69,7 @@ var initializeTouchData = function(touch) { currentTimeStamp: timestampForTouch(touch), previousPageX: touch.pageX, previousPageY: touch.pageY, - previousTimeStamp: timestampForTouch(touch), + previousTimeStamp: timestampForTouch(touch) }; }; @@ -178,7 +178,7 @@ var ResponderTouchHistoryStore = { } }, - touchHistory: touchHistory, + touchHistory: touchHistory }; diff --git a/src/event/eventPlugins/__tests__/ResponderEventPlugin-test.js b/src/event/eventPlugins/__tests__/ResponderEventPlugin-test.js index 1db5f403b2..57ffd5d323 100644 --- a/src/event/eventPlugins/__tests__/ResponderEventPlugin-test.js +++ b/src/event/eventPlugins/__tests__/ResponderEventPlugin-test.js @@ -11,12 +11,9 @@ 'use strict'; -jest.autoMockOff(); - var EventPluginHub; var EventConstants; var EventPropagators; -var NodeHandle; var ReactInstanceHandles; var ResponderEventPlugin; var SyntheticEvent; @@ -30,8 +27,6 @@ var CHILD_ID2 = '.0.0.1'; var topLevelTypes; var responderEventTypes; -var eachKeyVal = require('eachKeyVal'); - var touch = function(nodeHandle, i) { return {target: nodeHandle, identifier: i}; }; @@ -92,7 +87,7 @@ var _touchConfig = changedTouchObjects ), topLevelType: topType, - target: targetNodeHandle, // Because of our injected `NodeHandle` impl. + target: targetNodeHandle, targetID: targetNodeHandle, }; }; @@ -240,7 +235,8 @@ var registerTestHandlers = function(eventTestConfig, readableIDToID) { EventPluginHub.putListener(id, registrationName, handler); } }; - eachKeyVal(eventTestConfig, function(eventName, oneEventTypeTestConfig) { + for (var eventName in eventTestConfig) { + var oneEventTypeTestConfig = eventTestConfig[eventName]; var hasTwoPhase = !!oneEventTypeTestConfig.bubbled; if (hasTwoPhase) { registerOneEventType( @@ -257,7 +253,7 @@ var registerTestHandlers = function(eventTestConfig, readableIDToID) { oneEventTypeTestConfig ); } - }); + } return runs; }; @@ -321,36 +317,25 @@ var siblings = { describe('ResponderEventPlugin', function() { beforeEach(function() { - jest.resetModuleRegistry(); + require('mock-modules').dumpCache(); EventConstants = require('EventConstants'); EventPluginHub = require('EventPluginHub'); EventPluginUtils = require('EventPluginUtils'); EventPropagators = require('EventPropagators'); - NodeHandle = require('NodeHandle'); ReactInstanceHandles = require('ReactInstanceHandles'); ResponderEventPlugin = require('ResponderEventPlugin'); SyntheticEvent = require('SyntheticEvent'); EventPluginHub.injection.injectInstanceHandle(ReactInstanceHandles); - // dumpCache, in open-source tests, only resets existing mocks. It does not - // reset module-state though -- so we need to do this explicitly in the test - // for now. Once that's no longer the case, we can delete this line. - EventPluginHub.__purge(); // Only needed because SyntheticEvent supports the `currentTarget` // property. EventPluginUtils.injection.injectMount({ getNode: function(id) { - return id + '_thisDoesntReallyMatterForThisTestCase'; - } - }); - - // Raw event stream event `.target`s must be `NodeHandle`s. For these test - // cases, we'll assume that we create the node handles as the exact IDs - // they represent. - NodeHandle.injection.injectImplementation({ - getRootNodeID: function(nodeHandle) { + return id; + }, + getID: function(nodeHandle) { return nodeHandle; } });