From 3373572e15c00b2dab9f005a85bb420fae107b2b Mon Sep 17 00:00:00 2001 From: CommitSyncScript Date: Fri, 21 Jun 2013 16:01:26 -0700 Subject: [PATCH] Use ReactID.{get,set}ID instead of manipulating .id property directly. Another step in the plan to centralize control of React-specific identifers. --- src/core/ReactInstanceHandles.js | 5 +- src/core/ReactMount.js | 3 +- src/core/__tests__/ReactEventEmitter-test.js | 141 ++++++++++-------- src/core/__tests__/ReactIdentity-test.js | 4 +- .../__tests__/ReactInstanceHandles-test.js | 12 +- .../ReactMultiChildReconcile-test.js | 3 +- .../__tests__/ReactNativeComponent-test.js | 3 +- src/test/ReactTestUtils.js | 2 +- 8 files changed, 99 insertions(+), 74 deletions(-) diff --git a/src/core/ReactInstanceHandles.js b/src/core/ReactInstanceHandles.js index d6a8df0941..50564ac00c 100644 --- a/src/core/ReactInstanceHandles.js +++ b/src/core/ReactInstanceHandles.js @@ -285,9 +285,10 @@ var ReactInstanceHandles = { findComponentRoot: function(ancestorNode, id) { var child = ancestorNode.firstChild; while (child) { - if (id === child.id) { + var childId = ReactID.getID(child); + if (id === childId) { return child; - } else if (isAncestorIDOf(child.id, id)) { + } else if (isAncestorIDOf(childId, id)) { return ReactInstanceHandles.findComponentRoot(child, id); } child = child.nextSibling; diff --git a/src/core/ReactMount.js b/src/core/ReactMount.js index 6eaf42cfd6..2d9ddfcdb5 100644 --- a/src/core/ReactMount.js +++ b/src/core/ReactMount.js @@ -21,6 +21,7 @@ var ReactEventEmitter = require('ReactEventEmitter'); var ReactInstanceHandles = require('ReactInstanceHandles'); var ReactEventTopLevelCallback = require('ReactEventTopLevelCallback'); +var ReactID = require('ReactID'); var $ = require('$'); @@ -44,7 +45,7 @@ function getReactRootElementInContainer(container) { */ function getReactRootID(container) { var rootElement = getReactRootElementInContainer(container); - return rootElement && rootElement.id; + return rootElement && ReactID.getID(rootElement); } /** diff --git a/src/core/__tests__/ReactEventEmitter-test.js b/src/core/__tests__/ReactEventEmitter-test.js index c2646d3332..79bf8f2390 100644 --- a/src/core/__tests__/ReactEventEmitter-test.js +++ b/src/core/__tests__/ReactEventEmitter-test.js @@ -22,6 +22,7 @@ require('mock-modules') .dontMock('BrowserScroll') .dontMock('CallbackRegistry') .dontMock('EventPluginHub') + .dontMock('ReactID') .dontMock('ReactEventEmitter') .dontMock('ReactInstanceHandles') .dontMock('EventPluginHub') @@ -34,6 +35,9 @@ var keyOf = require('keyOf'); var mocks = require('mocks'); var EventPluginHub; +var ReactID = require('ReactID'); +var getID = ReactID.getID; +var setID = ReactID.setID; var ReactEventEmitter; var ReactEventTopLevelCallback; var ReactTestUtils; @@ -68,15 +72,15 @@ var ON_TOUCH_TAP_KEY = keyOf({onTouchTap: null}); var CHILD = document.createElement('div'); var PARENT = document.createElement('div'); var GRANDPARENT = document.createElement('div'); -CHILD.id = '.reactRoot.[0].[0].[0]'; -PARENT.id = '.reactRoot.[0].[0]'; -GRANDPARENT.id = '.reactRoot.[0]'; +setID(CHILD, '.reactRoot.[0].[0].[0]'); +setID(PARENT, '.reactRoot.[0].[0]'); +setID(GRANDPARENT, '.reactRoot.[0]'); function registerSimpleTestHandler() { - ReactEventEmitter.putListener(CHILD.id, ON_CLICK_KEY, LISTENER); - var listener = ReactEventEmitter.getListener(CHILD.id, ON_CLICK_KEY); + ReactEventEmitter.putListener(getID(CHILD), ON_CLICK_KEY, LISTENER); + var listener = ReactEventEmitter.getListener(getID(CHILD), ON_CLICK_KEY); expect(listener).toEqual(LISTENER); - return ReactEventEmitter.getListener(CHILD.id, ON_CLICK_KEY); + return ReactEventEmitter.getListener(getID(CHILD), ON_CLICK_KEY); } @@ -85,6 +89,9 @@ describe('ReactEventEmitter', function() { require('mock-modules').dumpCache(); EventPluginHub = require('EventPluginHub'); TapEventPlugin = require('TapEventPlugin'); + ReactID = require('ReactID'); + getID = ReactID.getID; + setID = ReactID.setID; ReactEventEmitter = require('ReactEventEmitter'); ReactTestUtils = require('ReactTestUtils'); ReactEventTopLevelCallback = require('ReactEventTopLevelCallback'); @@ -98,20 +105,20 @@ describe('ReactEventEmitter', function() { it('should store a listener correctly', function() { registerSimpleTestHandler(); - var listener = ReactEventEmitter.getListener(CHILD.id, ON_CLICK_KEY); + var listener = ReactEventEmitter.getListener(getID(CHILD), ON_CLICK_KEY); expect(listener).toBe(LISTENER); }); it('should retrieve a listener correctly', function() { registerSimpleTestHandler(); - var listener = ReactEventEmitter.getListener(CHILD.id, ON_CLICK_KEY); + var listener = ReactEventEmitter.getListener(getID(CHILD), ON_CLICK_KEY); expect(listener).toEqual(LISTENER); }); it('should clear all handlers when asked to', function() { registerSimpleTestHandler(); - ReactEventEmitter.deleteAllListeners(CHILD.id); - var listener = ReactEventEmitter.getListener(CHILD.id, ON_CLICK_KEY); + ReactEventEmitter.deleteAllListeners(getID(CHILD)); + var listener = ReactEventEmitter.getListener(getID(CHILD), ON_CLICK_KEY); expect(listener).toBe(undefined); }); @@ -133,89 +140,89 @@ describe('ReactEventEmitter', function() { it('should bubble simply', function() { ReactEventEmitter.putListener( - CHILD.id, + getID(CHILD), ON_CLICK_KEY, - recordID.bind(null, CHILD.id) + recordID.bind(null, getID(CHILD)) ); ReactEventEmitter.putListener( - PARENT.id, + getID(PARENT), ON_CLICK_KEY, - recordID.bind(null, PARENT.id) + recordID.bind(null, getID(PARENT)) ); ReactEventEmitter.putListener( - GRANDPARENT.id, + getID(GRANDPARENT), ON_CLICK_KEY, - recordID.bind(null, GRANDPARENT.id) + recordID.bind(null, getID(GRANDPARENT)) ); ReactTestUtils.Simulate.click(CHILD); expect(idCallOrder.length).toBe(3); - expect(idCallOrder[0]).toBe(CHILD.id); - expect(idCallOrder[1]).toBe(PARENT.id); - expect(idCallOrder[2]).toBe(GRANDPARENT.id); + expect(idCallOrder[0]).toBe(getID(CHILD)); + expect(idCallOrder[1]).toBe(getID(PARENT)); + expect(idCallOrder[2]).toBe(getID(GRANDPARENT)); }); it('should support stopPropagation()', function() { ReactEventEmitter.putListener( - CHILD.id, + getID(CHILD), ON_CLICK_KEY, - recordID.bind(null, CHILD.id) + recordID.bind(null, getID(CHILD)) ); ReactEventEmitter.putListener( - PARENT.id, + getID(PARENT), ON_CLICK_KEY, - recordIDAndStopPropagation.bind(null, PARENT.id) + recordIDAndStopPropagation.bind(null, getID(PARENT)) ); ReactEventEmitter.putListener( - GRANDPARENT.id, + getID(GRANDPARENT), ON_CLICK_KEY, - recordID.bind(null, GRANDPARENT.id) + recordID.bind(null, getID(GRANDPARENT)) ); ReactTestUtils.Simulate.click(CHILD); expect(idCallOrder.length).toBe(2); - expect(idCallOrder[0]).toBe(CHILD.id); - expect(idCallOrder[1]).toBe(PARENT.id); + expect(idCallOrder[0]).toBe(getID(CHILD)); + expect(idCallOrder[1]).toBe(getID(PARENT)); }); it('should stop after first dispatch if stopPropagation', function() { ReactEventEmitter.putListener( - CHILD.id, + getID(CHILD), ON_CLICK_KEY, - recordIDAndStopPropagation.bind(null, CHILD.id) + recordIDAndStopPropagation.bind(null, getID(CHILD)) ); ReactEventEmitter.putListener( - PARENT.id, + getID(PARENT), ON_CLICK_KEY, - recordID.bind(null, PARENT.id) + recordID.bind(null, getID(PARENT)) ); ReactEventEmitter.putListener( - GRANDPARENT.id, + getID(GRANDPARENT), ON_CLICK_KEY, - recordID.bind(null, GRANDPARENT.id) + recordID.bind(null, getID(GRANDPARENT)) ); ReactTestUtils.Simulate.click(CHILD); expect(idCallOrder.length).toBe(1); - expect(idCallOrder[0]).toBe(CHILD.id); + expect(idCallOrder[0]).toBe(getID(CHILD)); }); it('should stopPropagation if false is returned', function() { ReactEventEmitter.putListener( - CHILD.id, + getID(CHILD), ON_CLICK_KEY, - recordIDAndReturnFalse.bind(null, CHILD.id) + recordIDAndReturnFalse.bind(null, getID(CHILD)) ); ReactEventEmitter.putListener( - PARENT.id, + getID(PARENT), ON_CLICK_KEY, - recordID.bind(null, PARENT.id) + recordID.bind(null, getID(PARENT)) ); ReactEventEmitter.putListener( - GRANDPARENT.id, + getID(GRANDPARENT), ON_CLICK_KEY, - recordID.bind(null, GRANDPARENT.id) + recordID.bind(null, getID(GRANDPARENT)) ); ReactTestUtils.Simulate.click(CHILD); expect(idCallOrder.length).toBe(1); - expect(idCallOrder[0]).toBe(CHILD.id); + expect(idCallOrder[0]).toBe(getID(CHILD)); }); /** @@ -230,10 +237,14 @@ describe('ReactEventEmitter', function() { it('should invoke handlers that were removed while bubbling', function() { var handleParentClick = mocks.getMockFunction(); var handleChildClick = function(event) { - ReactEventEmitter.deleteAllListeners(PARENT.id); + ReactEventEmitter.deleteAllListeners(getID(PARENT)); }; - ReactEventEmitter.putListener(CHILD.id, ON_CLICK_KEY, handleChildClick); - ReactEventEmitter.putListener(PARENT.id, ON_CLICK_KEY, handleParentClick); + ReactEventEmitter.putListener(getID(CHILD), ON_CLICK_KEY, handleChildClick); + ReactEventEmitter.putListener( + getID(PARENT), + ON_CLICK_KEY, + handleParentClick + ); ReactTestUtils.Simulate.click(CHILD); expect(handleParentClick.mock.calls.length).toBe(1); }); @@ -241,18 +252,22 @@ describe('ReactEventEmitter', function() { it('should not invoke newly inserted handlers while bubbling', function() { var handleParentClick = mocks.getMockFunction(); var handleChildClick = function(event) { - ReactEventEmitter.putListener(PARENT.id, ON_CLICK_KEY, handleParentClick); + ReactEventEmitter.putListener( + getID(PARENT), + ON_CLICK_KEY, + handleParentClick + ); }; - ReactEventEmitter.putListener(CHILD.id, ON_CLICK_KEY, handleChildClick); + ReactEventEmitter.putListener(getID(CHILD), ON_CLICK_KEY, handleChildClick); ReactTestUtils.Simulate.click(CHILD); expect(handleParentClick.mock.calls.length).toBe(0); }); it('should infer onTouchTap from a touchStart/End', function() { ReactEventEmitter.putListener( - CHILD.id, + getID(CHILD), ON_TOUCH_TAP_KEY, - recordID.bind(null, CHILD.id) + recordID.bind(null, getID(CHILD)) ); ReactTestUtils.Simulate.touchStart( CHILD, @@ -263,14 +278,14 @@ describe('ReactEventEmitter', function() { ReactTestUtils.nativeTouchData(0, 0) ); expect(idCallOrder.length).toBe(1); - expect(idCallOrder[0]).toBe(CHILD.id); + expect(idCallOrder[0]).toBe(getID(CHILD)); }); it('should infer onTouchTap from when dragging below threshold', function() { ReactEventEmitter.putListener( - CHILD.id, + getID(CHILD), ON_TOUCH_TAP_KEY, - recordID.bind(null, CHILD.id) + recordID.bind(null, getID(CHILD)) ); ReactTestUtils.Simulate.touchStart( CHILD, @@ -281,14 +296,14 @@ describe('ReactEventEmitter', function() { ReactTestUtils.nativeTouchData(0, tapMoveThreshold - 1) ); expect(idCallOrder.length).toBe(1); - expect(idCallOrder[0]).toBe(CHILD.id); + expect(idCallOrder[0]).toBe(getID(CHILD)); }); it('should not onTouchTap from when dragging beyond threshold', function() { ReactEventEmitter.putListener( - CHILD.id, + getID(CHILD), ON_TOUCH_TAP_KEY, - recordID.bind(null, CHILD.id) + recordID.bind(null, getID(CHILD)) ); ReactTestUtils.Simulate.touchStart( CHILD, @@ -304,19 +319,19 @@ describe('ReactEventEmitter', function() { it('should bubble onTouchTap', function() { ReactEventEmitter.putListener( - CHILD.id, + getID(CHILD), ON_TOUCH_TAP_KEY, - recordID.bind(null, CHILD.id) + recordID.bind(null, getID(CHILD)) ); ReactEventEmitter.putListener( - PARENT.id, + getID(PARENT), ON_TOUCH_TAP_KEY, - recordID.bind(null, PARENT.id) + recordID.bind(null, getID(PARENT)) ); ReactEventEmitter.putListener( - GRANDPARENT.id, + getID(GRANDPARENT), ON_TOUCH_TAP_KEY, - recordID.bind(null, GRANDPARENT.id) + recordID.bind(null, getID(GRANDPARENT)) ); ReactTestUtils.Simulate.touchStart( CHILD, @@ -327,9 +342,9 @@ describe('ReactEventEmitter', function() { ReactTestUtils.nativeTouchData(0, 0) ); expect(idCallOrder.length).toBe(3); - expect(idCallOrder[0]).toBe(CHILD.id); - expect(idCallOrder[1]).toBe(PARENT.id); - expect(idCallOrder[2]).toBe(GRANDPARENT.id); + expect(idCallOrder[0]).toBe(getID(CHILD)); + expect(idCallOrder[1]).toBe(getID(PARENT)); + expect(idCallOrder[2]).toBe(getID(GRANDPARENT)); }); }); diff --git a/src/core/__tests__/ReactIdentity-test.js b/src/core/__tests__/ReactIdentity-test.js index f92a4c3e0b..9edf0f73ef 100644 --- a/src/core/__tests__/ReactIdentity-test.js +++ b/src/core/__tests__/ReactIdentity-test.js @@ -22,6 +22,7 @@ var React; var ReactTestUtils; var reactComponentExpect; +var ReactID; describe('ReactIdentity', function() { @@ -30,11 +31,12 @@ describe('ReactIdentity', function() { React = require('React'); ReactTestUtils = require('ReactTestUtils'); reactComponentExpect = require('reactComponentExpect'); + ReactID = require('ReactID'); }); var idExp = /^\.reactRoot\[\d+\](.*)$/; function checkId(child, expectedId) { - var actual = idExp.exec(child.id); + var actual = idExp.exec(ReactID.getID(child)); var expected = idExp.exec(expectedId); expect(actual).toBeTruthy(); expect(expected).toBeTruthy(); diff --git a/src/core/__tests__/ReactInstanceHandles-test.js b/src/core/__tests__/ReactInstanceHandles-test.js index 95e03d8250..34531223fa 100644 --- a/src/core/__tests__/ReactInstanceHandles-test.js +++ b/src/core/__tests__/ReactInstanceHandles-test.js @@ -21,6 +21,7 @@ var React = require('React'); var ReactTestUtils = require('ReactTestUtils'); +var ReactID = require('ReactID'); /** * Ensure that all callbacks are invoked, passing this unique argument. @@ -90,12 +91,15 @@ describe('ReactInstanceHandles', function() { parentNode.appendChild(childNodeA); parentNode.appendChild(childNodeB); - parentNode.id = '.react[0]'; - childNodeA.id = '.react[0].0'; - childNodeB.id = '.react[0].0:1'; + ReactID.setID(parentNode, '.react[0]'); + ReactID.setID(childNodeA, '.react[0].0'); + ReactID.setID(childNodeB, '.react[0].0:1'); expect( - ReactInstanceHandles.findComponentRoot(parentNode, childNodeB.id) + ReactInstanceHandles.findComponentRoot( + parentNode, + ReactID.getID(childNodeB) + ) ).toBe(childNodeB); }); }); diff --git a/src/core/__tests__/ReactMultiChildReconcile-test.js b/src/core/__tests__/ReactMultiChildReconcile-test.js index 597ca8f182..81b680c35b 100644 --- a/src/core/__tests__/ReactMultiChildReconcile-test.js +++ b/src/core/__tests__/ReactMultiChildReconcile-test.js @@ -23,6 +23,7 @@ require('mock-modules'); var React = require('React'); var ReactTestUtils = require('ReactTestUtils'); +var ReactID = require('ReactID'); var objMapKeyVal = require('objMapKeyVal'); @@ -191,7 +192,7 @@ function verifyDomOrderingAccurate(parentInstance, statusDisplays) { var i; var orderedDomIds = []; for (i=0; i < statusDisplayNodes.length; i++) { - orderedDomIds.push(statusDisplayNodes[i].id); + orderedDomIds.push(ReactID.getID(statusDisplayNodes[i])); } var orderedLogicalIds = []; diff --git a/src/core/__tests__/ReactNativeComponent-test.js b/src/core/__tests__/ReactNativeComponent-test.js index 55e5f13ec6..2611f1d442 100644 --- a/src/core/__tests__/ReactNativeComponent-test.js +++ b/src/core/__tests__/ReactNativeComponent-test.js @@ -328,6 +328,7 @@ describe('ReactNativeComponent', function() { it("should clean up listeners", function() { var React = require('React'); var ReactEventEmitter = require('ReactEventEmitter'); + var ReactID = require('ReactID'); var container = document.createElement('div'); document.documentElement.appendChild(container); @@ -337,7 +338,7 @@ describe('ReactNativeComponent', function() { React.renderComponent(instance, container); var rootNode = instance.getDOMNode(); - var rootNodeID = rootNode.id; + var rootNodeID = ReactID.getID(rootNode); expect( ReactEventEmitter.getListener(rootNodeID, 'onClick') ).toBe(callback); diff --git a/src/test/ReactTestUtils.js b/src/test/ReactTestUtils.js index 56c43e0bce..b8784183d6 100644 --- a/src/test/ReactTestUtils.js +++ b/src/test/ReactTestUtils.js @@ -225,7 +225,7 @@ var ReactTestUtils = { var node = ReactID.getNode(reactRootID); fakeNativeEvent.target = node; /* jsdom is returning nodes without id's - fixing that issue here. */ - node.id = reactRootID; + ReactID.setID(node, reactRootID); virtualHandler(fakeNativeEvent); },