From 52e622f1db79d2c871101af58f744e58f435d4ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20O=E2=80=99Shannessy?= Date: Wed, 17 Jul 2013 16:45:38 -0700 Subject: [PATCH 01/35] Version bump for 0.5.0 development --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 24c779d48d..58cf9b810e 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "react-tools", - "version": "0.4.0", + "version": "0.5.0-alpha", "keywords": [ "react", "jsx", From fd2125ee94f5bf30f7b5488cededbb0d1170574f Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Wed, 17 Jul 2013 18:32:23 -0400 Subject: [PATCH 02/35] Use getAttribute instead of getAttributeNode in ReactID.rawGetID. Also known as internalGetID, internally. --- src/core/ReactID.js | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/core/ReactID.js b/src/core/ReactID.js index f658241146..d3a6cfad2b 100644 --- a/src/core/ReactID.js +++ b/src/core/ReactID.js @@ -57,13 +57,7 @@ function getID(node) { } function internalGetID(node) { - if (node && node.getAttributeNode) { - var attributeNode = node.getAttributeNode(ATTR_NAME); - if (attributeNode) { - return attributeNode.value || ''; - } - } - return ''; + return node && node.getAttribute && node.getAttribute(ATTR_NAME) || ''; } /** From 75ce576d3daec14fd0214e33904a288fa319426c Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Wed, 17 Jul 2013 19:56:47 -0700 Subject: [PATCH 03/35] Avoid some innocuous test warnings. This reduces some console.warning spew from `grunt test` output. --- src/core/__tests__/ReactCompositeComponent-test.js | 4 ++++ src/core/__tests__/ReactInstanceHandles-test.js | 5 +++++ src/core/__tests__/refs-test.js | 6 +++++- src/dom/components/__tests__/ReactDOMTextarea-test.js | 11 +++++++++++ 4 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/core/__tests__/ReactCompositeComponent-test.js b/src/core/__tests__/ReactCompositeComponent-test.js index 28356063ef..b79b06c524 100644 --- a/src/core/__tests__/ReactCompositeComponent-test.js +++ b/src/core/__tests__/ReactCompositeComponent-test.js @@ -134,6 +134,8 @@ describe('ReactCompositeComponent', function() { }); it('should auto bind methods and values correctly', function() { + spyOn(console, 'warn'); + var ComponentClass = React.createClass({ getInitialState: function() { return {valueToReturn: 'hi'}; @@ -168,7 +170,9 @@ describe('ReactCompositeComponent', function() { // Next, prove that once mounted, the scope is bound correctly to the actual // component. ReactTestUtils.renderIntoDocument(instance); + expect(console.warn.argsForCall.length).toBe(0); var explicitlyBound = instance.methodToBeExplicitlyBound.bind(instance); + expect(console.warn.argsForCall.length).toBe(1); var autoBound = instance.methodAutoBound; var explicitlyNotBound = instance.methodExplicitlyNotBound; diff --git a/src/core/__tests__/ReactInstanceHandles-test.js b/src/core/__tests__/ReactInstanceHandles-test.js index b5161547c7..093ae845a5 100644 --- a/src/core/__tests__/ReactInstanceHandles-test.js +++ b/src/core/__tests__/ReactInstanceHandles-test.js @@ -138,6 +138,9 @@ describe('ReactInstanceHandles', function() { ReactID.getID(childNodeB) )).toBe(childNodeB); + spyOn(console, 'error'); + expect(console.error.argsForCall.length).toBe(0); + expect(function() { ReactInstanceHandles.findComponentRoot( parentNode, @@ -148,6 +151,8 @@ describe('ReactInstanceHandles', function() { 'Unable to find element. This probably means the DOM was ' + 'unexpectedly mutated (e.g. by the browser).' ); + + expect(console.error.argsForCall.length).toBe(1); }); }); diff --git a/src/core/__tests__/refs-test.js b/src/core/__tests__/refs-test.js index 193c9554ac..4880c31a4d 100644 --- a/src/core/__tests__/refs-test.js +++ b/src/core/__tests__/refs-test.js @@ -44,7 +44,11 @@ var ClickCounter = React.createClass({ var i; for (i=0; i < this.state.count; i++) { children.push( -
+
); } return ( diff --git a/src/dom/components/__tests__/ReactDOMTextarea-test.js b/src/dom/components/__tests__/ReactDOMTextarea-test.js index 5fbb36a7c6..456a8e63c1 100644 --- a/src/dom/components/__tests__/ReactDOMTextarea-test.js +++ b/src/dom/components/__tests__/ReactDOMTextarea-test.js @@ -62,9 +62,12 @@ describe('ReactDOMTextarea', function() { }); it('should treat children like `defaultValue`', function() { + spyOn(console, 'warn'); + var stub = ; var node = renderTextarea(stub); + expect(console.warn.argsForCall.length).toBe(1); expect(node.value).toBe('giraffe'); // Changing children should do nothing, it functions like `defaultValue`. @@ -73,21 +76,29 @@ describe('ReactDOMTextarea', function() { }); it('should allow numbers as children', function() { + spyOn(console, 'warn'); var node = renderTextarea(); + expect(console.warn.argsForCall.length).toBe(1); expect(node.value).toBe('17'); }); it("should throw with multiple or invalid children", function() { + spyOn(console, 'warn'); + expect(function() { ReactTestUtils.renderIntoDocument( ); }).toThrow(); + expect(console.warn.argsForCall.length).toBe(1); + expect(function() { ReactTestUtils.renderIntoDocument( ); }).toThrow(); + + expect(console.warn.argsForCall.length).toBe(2); }); }); From e6812d7e36c70c430dc6e9198a0fa7549f144f1c Mon Sep 17 00:00:00 2001 From: Pete Hunt Date: Wed, 17 Jul 2013 19:57:00 -0700 Subject: [PATCH 04/35] Add iframe attributes to React These are pretty useful for building apps and stuff. --- src/dom/DefaultDOMPropertyConfig.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/dom/DefaultDOMPropertyConfig.js b/src/dom/DefaultDOMPropertyConfig.js index 0ab0a05071..045aad5edc 100644 --- a/src/dom/DefaultDOMPropertyConfig.js +++ b/src/dom/DefaultDOMPropertyConfig.js @@ -38,6 +38,7 @@ var DefaultDOMPropertyConfig = { action: null, ajaxify: MUST_USE_ATTRIBUTE, allowFullScreen: MUST_USE_ATTRIBUTE | HAS_BOOLEAN_VALUE, + allowTransparency: MUST_USE_ATTRIBUTE, alt: null, autoComplete: null, autoFocus: HAS_BOOLEAN_VALUE, @@ -56,6 +57,7 @@ var DefaultDOMPropertyConfig = { disabled: MUST_USE_PROPERTY | HAS_BOOLEAN_VALUE, draggable: null, encType: null, + frameBorder: MUST_USE_ATTRIBUTE, height: MUST_USE_ATTRIBUTE, hidden: MUST_USE_ATTRIBUTE | HAS_BOOLEAN_VALUE, href: null, From 7ef5172d807de6d461090a563a43428544a11ed0 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Fri, 19 Jul 2013 10:58:11 -0400 Subject: [PATCH 05/35] Don't call require("mock-modules").register("test/all", ...). The "test/all" module will never be mocked, nor should it ever need to be reset by `dumpCache`. --- bin/jsx | 1 + 1 file changed, 1 insertion(+) diff --git a/bin/jsx b/bin/jsx index 5e269cda97..aa6ae90ab8 100755 --- a/bin/jsx +++ b/bin/jsx @@ -41,6 +41,7 @@ require("commoner").resolve(function(id) { return context.getProvidedP().then(function(idToPath) { if (id !== "mock-modules" && id !== "mocks" && + id !== "test/all" && idToPath.hasOwnProperty("mock-modules")) { return source + '\nrequire("mock-modules").register(' + JSON.stringify(id) + ', module);\n'; From 5beb48114567988b7abcedc74ddfaa7d718761d7 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Fri, 19 Jul 2013 10:40:43 -0400 Subject: [PATCH 06/35] Abandon '; - }).join("") + inner + ""; + /^(\s*)ENABLE_TESTS_HERE/m, + function(placeholder, leadingSpace) { + return leadingSpace + tests.map(function(testID) { + return "harness.enableTest(" + JSON.stringify(testID) + ");"; + }).join("\n" + leadingSpace); } ); @@ -63,9 +62,6 @@ server.listen(port, function(req, res) { file = "../build/" + file; break; - case "frame.html": - break; - case "": default: file = "index.html"; From 507e58ed9661bbfd7e89ad19460d3c80a6cf572e Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Fri, 19 Jul 2013 11:02:32 -0400 Subject: [PATCH 07/35] Pull in my rewritten Function.prototype.bind polyfill from upstream. We don't sync upstream polyfills (because we don't have a story for how they would be used), so this needs to be updated manually. Sacrificed some negligible performance optimizations to reduce the number of different cases from four to one. It's important to test this implementation in PhantomJS, since that's the only browser that I know of where built-in functions sometimes do not have a `.prototype`. --- src/test/all.js | 46 +++++++++++++++------------------------------- 1 file changed, 15 insertions(+), 31 deletions(-) diff --git a/src/test/all.js b/src/test/all.js index a94a58062d..a5c58f4bab 100644 --- a/src/test/all.js +++ b/src/test/all.js @@ -12,40 +12,24 @@ if (!Fp.bind) { Fp.bind = function(context) { var func = this; var args = slice.call(arguments, 1); - var bound; - if (func.prototype) { - if (args.length > 0) { - bound = function() { - return func.apply( - !(this instanceof func) && context || this, - args.concat(slice.call(arguments)) - ); - }; - } else { - bound = function() { - return func.apply( - !(this instanceof func) && context || this, - arguments - ); - }; - } - - bound.prototype = Object.create(func.prototype); - - } else if (args.length > 0) { - bound = function() { - return func.apply( - context || this, - args.concat(slice.call(arguments)) - ); - }; - } else { - bound = function() { - return func.apply(context || this, arguments); - }; + function bound() { + var invokedAsConstructor = func.prototype && (this instanceof func); + return func.apply( + // Ignore the context parameter when invoking the bound function + // as a constructor. Note that this includes not only constructor + // invocations using the new keyword but also calls to base class + // constructors such as BaseClass.call(this, ...) or super(...). + !invokedAsConstructor && context || this, + args.concat(slice.call(arguments)) + ); } + // The bound function must share the .prototype of the unbound + // function so that any object created by one constructor will count + // as an instance of both constructors. + bound.prototype = func.prototype; + return bound; }; } From bf275a909748a1e64ff06934ce0b6f8f285b86f7 Mon Sep 17 00:00:00 2001 From: Jamie Wong Date: Fri, 19 Jul 2013 12:33:54 -0400 Subject: [PATCH 08/35] Docs Typo Fix: s/distinciton/distinction --- docs/docs/04-multiple-components.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/docs/04-multiple-components.md b/docs/docs/04-multiple-components.md index 82e8b73ec1..ecc7c9baab 100644 --- a/docs/docs/04-multiple-components.md +++ b/docs/docs/04-multiple-components.md @@ -62,7 +62,7 @@ React.renderComponent( In the above example, instances of `Avatar` *own* instances of `ProfilePic` and `ProfileLink`. In React, **an owner is the component that sets the `props` of other components**. More formally, if a component `X` is created in component `Y`'s `render()` method, it is said that `X` is *owned by* `Y`. As discussed earlier, a component cannot mutate its `props` — they are always consistent with what its owner sets them to. This key property leads to UIs that are guaranteed to be consistent. -It's important to draw a distinciton between the owner-ownee relationship and the parent-child relationship. The owner-ownee relationship is specific to React, while the parent-child relationship is simply the one you know and love from the DOM. In the example above, `Avatar` owns the `div`, `ProfilePic` and `ProfileLink` instances, and `div` is the **parent** (but not owner) of the `ProfilePic` and `ProfileLink` instances. +It's important to draw a distinction between the owner-ownee relationship and the parent-child relationship. The owner-ownee relationship is specific to React, while the parent-child relationship is simply the one you know and love from the DOM. In the example above, `Avatar` owns the `div`, `ProfilePic` and `ProfileLink` instances, and `div` is the **parent** (but not owner) of the `ProfilePic` and `ProfileLink` instances. ## Children From 36fbd8d9412c599608888e18b104c820ffaefea4 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Fri, 19 Jul 2013 13:53:15 -0400 Subject: [PATCH 09/35] After each test, remove any stray nodes added to the document. This was not necessary when we were running each test in its own ` + + +## React Presentation + +[Tom Occhino](http://tomocchino.com/) and [Jordan Walke](https://github.com/jordwalke), React developers, did a presentation of React at Facebook Seattle's office. Check out the first 25 minutes for the presentation and the remaining 45 for a Q&A. I highly recommend you watching this video. + +
+ + +## Docs + +[Pete Hunt](http://www.petehunt.net/) rewrote the entirety of the docs for v0.4. The goal was to add more explanation about why we built React and what the best practices are. + +> Guides +> +> * [Why React?](/react/docs/why-react.html) +> * [Displaying Data](/react/docs/displaying-data.html) +> * [JSX in Depth](/react/docs/jsx-in-depth.html) +> * [JSX Gotchas](/react/docs/jsx-gotchas.html) +> * [Interactivity and Dynamic UIs](/react/docs/interactivity-and-dynamic-uis.html) +> * [Multiple Components](/react/docs/multiple-components.html) +> * [Reusable Components](/react/docs/reusable-components.html) +> * [Forms](/react/docs/forms.html) +> * [Working With the Browser](/react/docs/working-with-the-browser.html) +> * [More About Refs](/react/docs/more-about-refs.html) +> * [Tooling integration](/react/docs/tooling-integration.html) +> * [Reference](/react/docs/reference.html) From d1c5cda93f9b912777a7c45ecd071f14aef947e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20O=E2=80=99Shannessy?= Date: Tue, 23 Jul 2013 10:55:04 -0700 Subject: [PATCH 25/35] Use the right home page for react-source gem --- react-source.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/react-source.gemspec b/react-source.gemspec index 0e1b619df4..ead889f5bb 100644 --- a/react-source.gemspec +++ b/react-source.gemspec @@ -8,7 +8,7 @@ gemspec = Gem::Specification.new do |s| s.version = package['version'] s.license = 'Apache-2.0' - s.homepage = 'https://github.com/facebook/react.js' + s.homepage = 'https://github.com/facebook/react' s.summary = 'Ruby bridge to JSX & the React JavaScript library.' s.authors = ['Paul O’Shannessy'] From 7f8b2885d9f9feb396896f7d9d6086ac7731cf36 Mon Sep 17 00:00:00 2001 From: Cheng Lou Date: Tue, 23 Jul 2013 11:42:54 -0400 Subject: [PATCH 26/35] fix jquery-bootstrap example bugs Old one had some bugs: - 'x' on modal wasn't showing. - trying to close modal in unmount, but modal had a closing animation. --- examples/jquery-bootstrap/js/app.js | 84 ++++++++++++++--------------- 1 file changed, 41 insertions(+), 43 deletions(-) diff --git a/examples/jquery-bootstrap/js/app.js b/examples/jquery-bootstrap/js/app.js index 754eed05fc..b522605705 100644 --- a/examples/jquery-bootstrap/js/app.js +++ b/examples/jquery-bootstrap/js/app.js @@ -7,7 +7,7 @@ var BootstrapButton = React.createClass({ // transferPropsTo() is smart enough to merge classes provided // to this component. return this.transferPropsTo( - + {this.props.children} ); @@ -19,12 +19,18 @@ var BootstrapModal = React.createClass({ // integrate with Bootstrap or jQuery! componentDidMount: function() { // When the component is added, turn it into a modal - $(this.getDOMNode()).modal({backdrop: 'static', keyboard: false}); + $(this.getDOMNode()) + .modal({backdrop: 'static', keyboard: false, show: false}) }, componentWillUnmount: function() { - // And when it's destroyed, hide it. + $(this.getDOMNode()).off('hidden', this.handleHidden); + }, + close: function() { $(this.getDOMNode()).modal('hide'); }, + open: function() { + $(this.getDOMNode()).modal('show'); + }, render: function() { var confirmButton = null; var cancelButton = null; @@ -32,92 +38,84 @@ var BootstrapModal = React.createClass({ if (this.props.confirm) { confirmButton = ( + onClick={this.handleConfirm} + className="btn-primary"> {this.props.confirm} ); } if (this.props.cancel) { cancelButton = ( - + {this.props.cancel} ); } return ( -
-
+
+
-
+
{this.props.children}
-
+
{cancelButton} {confirmButton}
); }, - onCancel: function() { + handleCancel: function() { if (this.props.onCancel) { this.props.onCancel(); } - this.close(); }, - onConfirm: function() { + handleConfirm: function() { if (this.props.onConfirm) { this.props.onConfirm(); } - this.close(); - }, - close: function() { - if (this.props.onClose) { - this.props.onClose(); - } } }); var Example = React.createClass({ - getInitialState: function() { - return {modalVisible: false}; - }, - toggleModal: function() { - this.setState({modalVisible: !this.state.modalVisible}); - }, handleCancel: function() { if (confirm('Are you sure you want to cancel?')) { - this.toggleModal(); + this.refs.modal.close(); } }, render: function() { var modal = null; - if (this.state.modalVisible) { - modal = ( - - This is a React component powered by jQuery and Bootstrap! - - ); - } + modal = ( + + This is a React component powered by jQuery and Bootstrap! + + ); return ( -
+
{modal} - Toggle modal + Open modal
); + }, + openModal: function() { + this.refs.modal.open(); + }, + closeModal: function() { + this.refs.modal.close(); } }); From 795a84d60fe9b13b195befdc4b2d6a811e8e5eb3 Mon Sep 17 00:00:00 2001 From: Jakub Malinowski Date: Tue, 23 Jul 2013 23:01:38 +0200 Subject: [PATCH 27/35] Do not export load in JSXTransformer unless in a browser environment --- vendor/browser-transforms.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/vendor/browser-transforms.js b/vendor/browser-transforms.js index 87cc315c04..c553bf2e3e 100644 --- a/vendor/browser-transforms.js +++ b/vendor/browser-transforms.js @@ -32,6 +32,11 @@ exports.exec = function(code) { return eval(transform(code)); }; +if (typeof window === "undefined" || window === null) { + return; +} +headEl = document.getElementsByTagName('head')[0]; + var run = exports.run = function(code) { var jsx = docblock.parseAsObject(docblock.extract(code)).jsx; @@ -42,11 +47,6 @@ var run = exports.run = function(code) { headEl.appendChild(scriptEl); }; -if (typeof window === "undefined" || window === null) { - return; -} -headEl = document.getElementsByTagName('head')[0]; - var load = exports.load = function(url, callback) { var xhr; xhr = window.ActiveXObject ? new window.ActiveXObject('Microsoft.XMLHTTP') From ddb0ef98f7986cf27f767348c7e181d991acb4ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20O=E2=80=99Shannessy?= Date: Wed, 24 Jul 2013 13:13:27 -0700 Subject: [PATCH 28/35] Fix "Suppport" type in docs --- docs/docs/07-working-with-the-browser.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/docs/07-working-with-the-browser.md b/docs/docs/07-working-with-the-browser.md index 7e4b1a979f..538b32f37d 100644 --- a/docs/docs/07-working-with-the-browser.md +++ b/docs/docs/07-working-with-the-browser.md @@ -110,7 +110,7 @@ _Mounted_ composite components also support the following methods: > calling `this.getDOMNode()`. -## Browser Suppport and Polyfills +## Browser Support and Polyfills At Facebook, we support older browsers, including IE8. We've had polyfills in place for a long time to allow us to write forward-thinking JS. This means we don't have a bunch of hacks scattered throughout our codebase and we can still expect our code to "just work". For example, instead of seeing `+new Date()`, we can just write `Date.now()`. Since the open source React is the same as what we use internally, we've carried over this philosophy of using forward thinking JS. From 492407bcc98745577a68c0813c8cf529fa3d4da4 Mon Sep 17 00:00:00 2001 From: Jordan Walke Date: Wed, 24 Jul 2013 17:19:23 -0700 Subject: [PATCH 29/35] Fix OrderedMap. Tim caught a bug. Squashing it so he can rebase on top of it. --- src/utils/OrderedMap.js | 9 +++++++++ src/utils/__tests__/OrderedMap-test.js | 10 ++++++++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/utils/OrderedMap.js b/src/utils/OrderedMap.js index b4ea88216f..1bf238fd32 100644 --- a/src/utils/OrderedMap.js +++ b/src/utils/OrderedMap.js @@ -18,6 +18,7 @@ "use strict"; +var invariant = require('invariant'); var mixInto = require('mixInto'); var throwIf = require('throwIf'); @@ -349,6 +350,10 @@ var OrderedMapMethods = { mapKeyRange: function(cb, startKey, endKey, context) { var startIndex = this.indexOfKey(startKey); var endIndex = this.indexOfKey(endKey); + invariant( + startIndex !== undefined && endIndex !== undefined, + 'mapKeyRange must be given keys that are present.' + ); if (endIndex < startIndex) { throw new Error(RANGE_ARGS); } @@ -358,6 +363,10 @@ var OrderedMapMethods = { forEachKeyRange: function(cb, startKey, endKey, context) { var startIndex = this.indexOfKey(startKey); var endIndex = this.indexOfKey(endKey); + invariant( + startIndex !== undefined && endIndex !== undefined, + 'forEachKeyRange must be given keys that are present.' + ); if (endIndex < startIndex) { throw new Error(RANGE_ARGS); } diff --git a/src/utils/__tests__/OrderedMap-test.js b/src/utils/__tests__/OrderedMap-test.js index a65c996986..361142df24 100644 --- a/src/utils/__tests__/OrderedMap-test.js +++ b/src/utils/__tests__/OrderedMap-test.js @@ -739,10 +739,16 @@ describe('OrderedMap', function() { }).not.toThrow(); expect(function() { om.mapKeyRange(duplicate, 'x' , 3, scope); - }).not.toThrow(); + }).toThrow( + 'Invariant Violation: mapKeyRange must be given keys ' + + 'that are present.' + ); expect(function() { om.forEachKeyRange(duplicate, 'x', 3, scope); - }).not.toThrow(); + }).toThrow( + 'Invariant Violation: forEachKeyRange must be given keys ' + + 'that are present.' + ); expect(function() { om.mapRange(duplicate, 0, 4, scope); From 260d90ba029483c4d2bca77ad89abf8d16e7d545 Mon Sep 17 00:00:00 2001 From: Pete Hunt Date: Wed, 24 Jul 2013 17:19:29 -0700 Subject: [PATCH 30/35] Warn when server-rendered markup is not what we expect on the client As @leebyron and balpert pointed out, if the markup on the server is differnet than what the client expects undefined behavior and chaos may ensue. A good fallback is for us to just inject the client-side markup (as it is the source of truth) and warn the user in __DEV__ that something is wrong. In order to do a fast browser-independent check of the DOM I use an adler32 checksum of the generated markup. I believe this is better than a simple innerHTML compare because different browsers massage innerHTML differently. --- src/core/ReactComponent.js | 20 ++++++- src/core/ReactMount.js | 10 +--- src/core/getReactRootElementInContainer.js | 29 ++++++++++ src/environment/ReactMarkupChecksum.js | 53 +++++++++++++++++ src/environment/ReactServerRendering.js | 5 +- .../__tests__/ReactServerRendering-test.js | 57 ++++++++++++++----- src/utils/adler32.js | 37 ++++++++++++ 7 files changed, 187 insertions(+), 24 deletions(-) create mode 100644 src/core/getReactRootElementInContainer.js create mode 100644 src/environment/ReactMarkupChecksum.js create mode 100644 src/utils/adler32.js diff --git a/src/core/ReactComponent.js b/src/core/ReactComponent.js index 6ed5a1ac58..871abf625f 100644 --- a/src/core/ReactComponent.js +++ b/src/core/ReactComponent.js @@ -20,9 +20,11 @@ "use strict"; +var getReactRootElementInContainer = require('getReactRootElementInContainer'); var ReactCurrentOwner = require('ReactCurrentOwner'); var ReactDOMIDOperations = require('ReactDOMIDOperations'); var ReactID = require('ReactID'); +var ReactMarkupChecksum = require('ReactMarkupChecksum'); var ReactOwner = require('ReactOwner'); var ReactReconcileTransaction = require('ReactReconcileTransaction'); var ReactUpdates = require('ReactUpdates'); @@ -500,7 +502,23 @@ var ReactComponent = { var markup = this.mountComponent(rootID, transaction); if (shouldReuseMarkup) { - return; + if (ReactMarkupChecksum.canReuseMarkup( + markup, + getReactRootElementInContainer(container))) { + return; + } else { + if (__DEV__) { + console.warn( + 'React attempted to use reuse markup in a container but the ' + + 'checksum was invalid. This generally means that you are using ' + + 'server rendering and the markup generated on the server was ' + + 'not what the client was expecting. React injected new markup ' + + 'to compensate which works but you have lost many of the ' + + 'benefits of server rendering. Instead, figure out why the ' + + 'markup being generated is different on the client or server.' + ); + } + } } // Asynchronously inject markup by ensuring that the container is not in diff --git a/src/core/ReactMount.js b/src/core/ReactMount.js index ed10e4a609..8713f1bee4 100644 --- a/src/core/ReactMount.js +++ b/src/core/ReactMount.js @@ -19,6 +19,7 @@ "use strict"; var invariant = require('invariant'); +var getReactRootElementInContainer = require('getReactRootElementInContainer'); var ReactEventEmitter = require('ReactEventEmitter'); var ReactInstanceHandles = require('ReactInstanceHandles'); var ReactEventTopLevelCallback = require('ReactEventTopLevelCallback'); @@ -37,14 +38,6 @@ if (__DEV__) { var rootElementsByReactRootID = {}; } -/** - * @param {DOMElement} container DOM element that may contain a React component - * @return {?*} DOM element that may have the reactRoot ID, or null. - */ -function getReactRootElementInContainer(container) { - return container && container.firstChild; -} - /** * @param {DOMElement} container DOM element that may contain a React component. * @return {?string} A "reactRoot" ID, if a React component is rendered. @@ -338,7 +331,6 @@ var ReactMount = { var reactRoot = ReactMount.findReactContainerForID(id); return ReactInstanceHandles.findComponentRoot(reactRoot, id); } - }; module.exports = ReactMount; diff --git a/src/core/getReactRootElementInContainer.js b/src/core/getReactRootElementInContainer.js new file mode 100644 index 0000000000..ac306a5850 --- /dev/null +++ b/src/core/getReactRootElementInContainer.js @@ -0,0 +1,29 @@ +/** + * Copyright 2013 Facebook, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * @providesModule getReactRootElementInContainer + */ + +"use strict"; + +/** + * @param {DOMElement} container DOM element that may contain a React component + * @return {?*} DOM element that may have the reactRoot ID, or null. + */ +function getReactRootElementInContainer(container) { + return container && container.firstChild; +} + +module.exports = getReactRootElementInContainer; diff --git a/src/environment/ReactMarkupChecksum.js b/src/environment/ReactMarkupChecksum.js new file mode 100644 index 0000000000..6bf56818c8 --- /dev/null +++ b/src/environment/ReactMarkupChecksum.js @@ -0,0 +1,53 @@ +/** + * Copyright 2013 Facebook, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * @providesModule ReactMarkupChecksum + */ + +"use strict"; + +var adler32 = require('adler32'); + +var ReactMarkupChecksum = { + CHECKSUM_ATTR_NAME: 'data-react-checksum', + + /** + * @param {string} markup Markup string + * @return {string} Markup string with checksum attribute attached + */ + addChecksumToMarkup: function(markup) { + var checksum = adler32(markup); + return markup.replace( + '>', + ' ' + ReactMarkupChecksum.CHECKSUM_ATTR_NAME + '="' + checksum + '">' + ); + }, + + /** + * @param {string} markup to use + * @param {DOMElement} element root React element + * @returns {boolean} whether or not the markup is the same + */ + canReuseMarkup: function(markup, element) { + var existingChecksum = element.getAttribute( + ReactMarkupChecksum.CHECKSUM_ATTR_NAME + ); + existingChecksum = existingChecksum && parseInt(existingChecksum, 10); + var markupChecksum = adler32(markup); + return markupChecksum === existingChecksum; + } +}; + +module.exports = ReactMarkupChecksum; diff --git a/src/environment/ReactServerRendering.js b/src/environment/ReactServerRendering.js index 0a0b598927..748f9f35cc 100644 --- a/src/environment/ReactServerRendering.js +++ b/src/environment/ReactServerRendering.js @@ -18,6 +18,7 @@ */ "use strict"; +var ReactMarkupChecksum = require('ReactMarkupChecksum'); var ReactReconcileTransaction = require('ReactReconcileTransaction'); var ReactInstanceHandles = require('ReactInstanceHandles'); @@ -33,7 +34,9 @@ function renderComponentToString(component, callback) { transaction.reinitializeTransaction(); try { transaction.perform(function() { - callback(component.mountComponent(id, transaction)); + var markup = component.mountComponent(id, transaction); + markup = ReactMarkupChecksum.addChecksumToMarkup(markup); + callback(markup); }, null); } finally { ReactReconcileTransaction.release(transaction); diff --git a/src/environment/__tests__/ReactServerRendering-test.js b/src/environment/__tests__/ReactServerRendering-test.js index 7e0ddd3007..192b54cc3b 100644 --- a/src/environment/__tests__/ReactServerRendering-test.js +++ b/src/environment/__tests__/ReactServerRendering-test.js @@ -26,12 +26,16 @@ require('mock-modules') .dontMock('React') .dontMock('ReactID') .dontMock('ReactServerRendering') - .dontMock('ReactTestUtils'); + .dontMock('ReactTestUtils') + .dontMock('ReactMarkupChecksum'); + +var mocks = require('mocks'); var React; var ReactID; var ReactTestUtils; var ReactServerRendering; +var ReactMarkupChecksum; var ExecutionEnvironment; describe('ReactServerRendering', function() { @@ -43,6 +47,7 @@ describe('ReactServerRendering', function() { ExecutionEnvironment = require('ExecutionEnvironment'); ExecutionEnvironment.canUseDOM = false; ReactServerRendering = require('ReactServerRendering'); + ReactMarkupChecksum = require('ReactMarkupChecksum'); }); it('should generate simple markup', function() { @@ -54,7 +59,8 @@ describe('ReactServerRendering', function() { } ); expect(response).toMatch( - 'hello world' + 'hello world' ); }); @@ -77,7 +83,8 @@ describe('ReactServerRendering', function() { } ); expect(response).toMatch( - '
' + + '
' + '' + 'My name is ' + 'child' + @@ -130,7 +137,8 @@ describe('ReactServerRendering', function() { ); expect(response).toMatch( - '' + + '' + 'Component name: ' + 'TestComponent' + '' @@ -148,7 +156,7 @@ describe('ReactServerRendering', function() { var numClicks = 0; var TestComponent = React.createClass({ - componentWillMount: function() { + componentDidMount: function() { mountCount++; }, click: function() { @@ -179,18 +187,41 @@ describe('ReactServerRendering', function() { expect(mountCount).toEqual(2); expect(element.innerHTML).not.toEqual(lastMarkup); - // Now kill the node and render it on top of the old markup, as if + // Now kill the node and render it on top of server-rendered markup, as if // we used server rendering. We should mount again, but the markup should be - // unchanged. - lastMarkup = element.innerHTML; + // unchanged. We will append a sentinel at the end of innerHTML to be sure + // that innerHTML was not changed. React.unmountAndReleaseReactRootNode(element); expect(element.innerHTML).toEqual(''); - element.innerHTML = lastMarkup; - // NOTE: we pass a different name here. This is to ensure that the markup - // being generated is not replaced. - var instance = React.renderComponent(, element); + + ExecutionEnvironment.canUseDOM = false; + ReactServerRendering.renderComponentToString( + , + function(markup) { + lastMarkup = markup; + } + ); + ExecutionEnvironment.canUseDOM = true; + + element.innerHTML = lastMarkup + ' __sentinel__'; + + React.renderComponent(, element); expect(mountCount).toEqual(3); - expect(element.innerHTML).toEqual(lastMarkup); + expect(element.innerHTML.indexOf('__sentinel__') > -1).toBe(true); + React.unmountAndReleaseReactRootNode(element); + expect(element.innerHTML).toEqual(''); + + // Now simulate a situation where the app is not idempotent. React should + // warn but do the right thing. + var _warn = console.warn; + console.warn = mocks.getMockFunction(); + element.innerHTML = lastMarkup; + var instance = React.renderComponent(, element); + expect(mountCount).toEqual(4); + expect(console.warn.mock.calls.length).toBe(1); + expect(element.innerHTML.length > 0).toBe(true); + expect(element.innerHTML).not.toEqual(lastMarkup); + console.warn = _warn; // Ensure the events system works expect(numClicks).toEqual(0); diff --git a/src/utils/adler32.js b/src/utils/adler32.js new file mode 100644 index 0000000000..db9c569309 --- /dev/null +++ b/src/utils/adler32.js @@ -0,0 +1,37 @@ +/** + * Copyright 2013 Facebook, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * @providesModule adler32 + */ + +"use strict"; + +var MOD = 65521; + +// This is a clean-room implementation of adler32 designed for detecting +// if markup is not what we expect it to be. It does not need to be +// cryptographically strong, only reasonable good at detecting if markup +// generated on the server is different than that on the client. +function adler32(data) { + var a = 1; + var b = 0; + for (var i = 0; i < data.length; i++) { + a = (a + data.charCodeAt(i)) % MOD; + b = (b + a) % MOD; + } + return a | (b << 16); +} + +module.exports = adler32; From 2ee66262db9fb91b0d0e9d64cefeb223074f8e05 Mon Sep 17 00:00:00 2001 From: Jordan Walke Date: Wed, 24 Jul 2013 17:19:36 -0700 Subject: [PATCH 31/35] Remove circular dependencies in React Core. There is a circular dependency between `ReactID`, `ReactMount` and `ReactInstanceHandles`. Ben and I talked about this today. It seems like the simplest solution is to consolidate a lot of the code that Ben recently wrote into `ReactMount`. We can later find ways to trim code out of this module without causing circular deps. --- src/core/ReactComponent.js | 6 +- src/core/ReactDOMIDOperations.js | 16 +- src/core/ReactDefaultInjection.js | 3 + src/core/ReactEventEmitter.js | 9 +- src/core/ReactEventTopLevelCallback.js | 9 +- src/core/ReactID.js | 163 ----------- src/core/ReactInstanceHandles.js | 105 +------ src/core/ReactMount.js | 277 ++++++++++++++++-- src/core/ReactNativeComponent.js | 4 +- src/core/ReactTextComponent.js | 4 +- .../__tests__/ReactCompositeComponent-test.js | 6 +- src/core/__tests__/ReactDOM-test.js | 8 +- .../__tests__/ReactDOMIDOperations-test.js | 10 +- src/core/__tests__/ReactEventEmitter-test.js | 14 +- src/core/__tests__/ReactIdentity-test.js | 10 +- .../__tests__/ReactInstanceHandles-test.js | 34 +-- .../ReactMultiChildReconcile-test.js | 4 +- .../__tests__/ReactNativeComponent-test.js | 4 +- .../__tests__/ReactServerRendering-test.js | 22 +- src/eventPlugins/EnterLeaveEventPlugin.js | 9 +- src/test/ReactTestUtils.js | 6 +- 21 files changed, 356 insertions(+), 367 deletions(-) delete mode 100644 src/core/ReactID.js diff --git a/src/core/ReactComponent.js b/src/core/ReactComponent.js index 871abf625f..b08e4ff1b5 100644 --- a/src/core/ReactComponent.js +++ b/src/core/ReactComponent.js @@ -23,8 +23,8 @@ var getReactRootElementInContainer = require('getReactRootElementInContainer'); var ReactCurrentOwner = require('ReactCurrentOwner'); var ReactDOMIDOperations = require('ReactDOMIDOperations'); -var ReactID = require('ReactID'); var ReactMarkupChecksum = require('ReactMarkupChecksum'); +var ReactMount = require('ReactMount'); var ReactOwner = require('ReactOwner'); var ReactReconcileTransaction = require('ReactReconcileTransaction'); var ReactUpdates = require('ReactUpdates'); @@ -256,7 +256,7 @@ var ReactComponent = { this.isMounted(), 'getDOMNode(): A component must be mounted to have a DOM node.' ); - return ReactID.getNode(this._rootNodeID); + return ReactMount.getNode(this._rootNodeID); }, /** @@ -382,7 +382,7 @@ var ReactComponent = { if (props.ref != null) { ReactOwner.removeComponentAsRefFrom(this, props.ref, props[OWNER]); } - ReactID.purgeID(this._rootNodeID); + ReactMount.purgeID(this._rootNodeID); this._rootNodeID = null; this._lifeCycleState = ComponentLifeCycle.UNMOUNTED; }, diff --git a/src/core/ReactDOMIDOperations.js b/src/core/ReactDOMIDOperations.js index c98edcbe8f..7f54ac9ee2 100644 --- a/src/core/ReactDOMIDOperations.js +++ b/src/core/ReactDOMIDOperations.js @@ -24,7 +24,7 @@ var CSSPropertyOperations = require('CSSPropertyOperations'); var DOMChildrenOperations = require('DOMChildrenOperations'); var DOMPropertyOperations = require('DOMPropertyOperations'); -var ReactID = require('ReactID'); +var ReactMount = require('ReactMount'); var getTextContentAccessor = require('getTextContentAccessor'); var invariant = require('invariant'); @@ -65,7 +65,7 @@ var ReactDOMIDOperations = { * @internal */ updatePropertyByID: function(id, name, value) { - var node = ReactID.getNode(id); + var node = ReactMount.getNode(id); invariant( !INVALID_PROPERTY_ERRORS.hasOwnProperty(name), 'updatePropertyByID(...): %s', @@ -91,7 +91,7 @@ var ReactDOMIDOperations = { * @internal */ deletePropertyByID: function(id, name, value) { - var node = ReactID.getNode(id); + var node = ReactMount.getNode(id); invariant( !INVALID_PROPERTY_ERRORS.hasOwnProperty(name), 'updatePropertyByID(...): %s', @@ -128,7 +128,7 @@ var ReactDOMIDOperations = { * @internal */ updateStylesByID: function(id, styles) { - var node = ReactID.getNode(id); + var node = ReactMount.getNode(id); CSSPropertyOperations.setValueForStyles(node, styles); }, @@ -140,7 +140,7 @@ var ReactDOMIDOperations = { * @internal */ updateInnerHTMLByID: function(id, html) { - var node = ReactID.getNode(id); + var node = ReactMount.getNode(id); // HACK: IE8- normalize whitespace in innerHTML, removing leading spaces. // @see quirksmode.org/bugreports/archives/2004/11/innerhtml_and_t.html node.innerHTML = (html && html.__html || '').replace(/^ /g, ' '); @@ -154,7 +154,7 @@ var ReactDOMIDOperations = { * @internal */ updateTextContentByID: function(id, content) { - var node = ReactID.getNode(id); + var node = ReactMount.getNode(id); node[textContentAccessor] = content; }, @@ -167,7 +167,7 @@ var ReactDOMIDOperations = { * @see {Danger.dangerouslyReplaceNodeWithMarkup} */ dangerouslyReplaceNodeWithMarkupByID: function(id, markup) { - var node = ReactID.getNode(id); + var node = ReactMount.getNode(id); DOMChildrenOperations.dangerouslyReplaceNodeWithMarkup(node, markup); }, @@ -176,7 +176,7 @@ var ReactDOMIDOperations = { * Detect if any elements were removed instead of blindly purging. */ manageChildrenByParentID: function(parentID, domOperations) { - var parent = ReactID.getNode(parentID); + var parent = ReactMount.getNode(parentID); DOMChildrenOperations.manageChildren(parent, domOperations); } diff --git a/src/core/ReactDefaultInjection.js b/src/core/ReactDefaultInjection.js index d642a4daf8..7d858bb7c2 100644 --- a/src/core/ReactDefaultInjection.js +++ b/src/core/ReactDefaultInjection.js @@ -24,6 +24,8 @@ var ReactDOMInput = require('ReactDOMInput'); var ReactDOMOption = require('ReactDOMOption'); var ReactDOMSelect = require('ReactDOMSelect'); var ReactDOMTextarea = require('ReactDOMTextarea'); +var ReactEventEmitter = require('ReactEventEmitter'); +var ReactEventTopLevelCallback = require('ReactEventTopLevelCallback'); var DefaultDOMPropertyConfig = require('DefaultDOMPropertyConfig'); var DOMProperty = require('DOMProperty'); @@ -37,6 +39,7 @@ var SimpleEventPlugin = require('SimpleEventPlugin'); var MobileSafariClickEventPlugin = require('MobileSafariClickEventPlugin'); function inject() { + ReactEventEmitter.TopLevelCallbackCreator = ReactEventTopLevelCallback; /** * Inject module for resolving DOM hierarchy and plugin ordering. */ diff --git a/src/core/ReactEventEmitter.js b/src/core/ReactEventEmitter.js index 4f52e383d6..a5971ff789 100644 --- a/src/core/ReactEventEmitter.js +++ b/src/core/ReactEventEmitter.js @@ -257,17 +257,20 @@ var ReactEventEmitter = { * reason, and only in some cases). * * @param {boolean} touchNotMouse Listen to touch events instead of mouse. - * @param {object} TopLevelCallbackCreator */ - ensureListening: function(touchNotMouse, TopLevelCallbackCreator) { + ensureListening: function(touchNotMouse) { invariant( ExecutionEnvironment.canUseDOM, 'ensureListening(...): Cannot toggle event listening in a Worker ' + 'thread. This is likely a bug in the framework. Please report ' + 'immediately.' ); + invariant( + ReactEventEmitter.TopLevelCallbackCreator, + 'ensureListening(...): Cannot be called without a top level callback ' + + 'creator being injected.' + ); if (!_isListening) { - ReactEventEmitter.TopLevelCallbackCreator = TopLevelCallbackCreator; listenAtTopLevel(touchNotMouse); _isListening = true; } diff --git a/src/core/ReactEventTopLevelCallback.js b/src/core/ReactEventTopLevelCallback.js index 215d23da23..b3e93a8c62 100644 --- a/src/core/ReactEventTopLevelCallback.js +++ b/src/core/ReactEventTopLevelCallback.js @@ -21,8 +21,7 @@ var ExecutionEnvironment = require('ExecutionEnvironment'); var ReactEventEmitter = require('ReactEventEmitter'); -var ReactID = require('ReactID'); -var ReactInstanceHandles = require('ReactInstanceHandles'); +var ReactMount = require('ReactMount'); var getEventTarget = require('getEventTarget'); @@ -34,7 +33,7 @@ var _topLevelListenersEnabled = true; /** * Top-level callback creator used to implement event handling using delegation. - * This is used via dependency injection in `ReactEventEmitter.ensureListening`. + * This is used via dependency injection. */ var ReactEventTopLevelCallback = { @@ -73,10 +72,10 @@ var ReactEventTopLevelCallback = { nativeEvent.srcElement !== nativeEvent.target) { nativeEvent.target = nativeEvent.srcElement; } - var topLevelTarget = ReactInstanceHandles.getFirstReactDOM( + var topLevelTarget = ReactMount.getFirstReactDOM( getEventTarget(nativeEvent) ) || ExecutionEnvironment.global; - var topLevelTargetID = ReactID.getID(topLevelTarget) || ''; + var topLevelTargetID = ReactMount.getID(topLevelTarget) || ''; ReactEventEmitter.handleTopLevel( topLevelType, topLevelTarget, diff --git a/src/core/ReactID.js b/src/core/ReactID.js deleted file mode 100644 index 55bb526570..0000000000 --- a/src/core/ReactID.js +++ /dev/null @@ -1,163 +0,0 @@ -/** - * Copyright 2013 Facebook, Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - * @providesModule ReactID - * @typechecks static-only - */ - -"use strict"; - -var invariant = require('invariant'); -var ReactMount = require('ReactMount'); -var ATTR_NAME = 'data-reactid'; -var nodeCache = {}; - -/** - * Accessing node[ATTR_NAME] or calling getAttribute(ATTR_NAME) on a form - * element can return its control whose name or ID equals ATTR_NAME. All - * DOM nodes support `getAttributeNode` but this can also get called on - * other objects so just return '' if we're given something other than a - * DOM node (such as window). - * - * @param {?DOMElement|DOMWindow|DOMDocument|DOMTextNode} node DOM node. - * @return {string} ID of the supplied `domNode`. - */ -function getID(node) { - var id = internalGetID(node); - if (id) { - if (nodeCache.hasOwnProperty(id)) { - var cached = nodeCache[id]; - if (cached !== node) { - invariant( - !isValid(cached, id), - 'ReactID: Two valid but unequal nodes with the same `%s`: %s', - ATTR_NAME, id - ); - - nodeCache[id] = node; - } - } else { - nodeCache[id] = node; - } - } - - return id; -} - -function internalGetID(node) { - // If node is something like a window, document, or text node, none of - // which support attributes or a .getAttribute method, gracefully return - // the empty string, as if the attribute were missing. - return node && node.getAttribute && node.getAttribute(ATTR_NAME) || ''; -} - -/** - * Sets the React-specific ID of the given node. - * - * @param {DOMElement} node The DOM node whose ID will be set. - * @param {string} id The value of the ID attribute. - */ -function setID(node, id) { - var oldID = internalGetID(node); - if (oldID !== id) { - delete nodeCache[oldID]; - } - node.setAttribute(ATTR_NAME, id); - nodeCache[id] = node; -} - -/** - * Finds the node with the supplied React-generated DOM ID. - * - * @param {string} id A React-generated DOM ID. - * @return {DOMElement} DOM node with the suppled `id`. - * @internal - */ -function getNode(id) { - if (!nodeCache.hasOwnProperty(id) || !isValid(nodeCache[id], id)) { - nodeCache[id] = ReactMount.findReactNodeByID(id); - } - return nodeCache[id]; -} - -/** - * A node is "valid" if it is contained by a currently mounted container. - * - * This means that the node does not have to be contained by a document in - * order to be considered valid. - * - * @param {?DOMElement} node The candidate DOM node. - * @param {string} id The expected ID of the node. - * @return {boolean} Whether the node is contained by a mounted container. - */ -function isValid(node, id) { - if (node) { - invariant( - internalGetID(node) === id, - 'ReactID: Unexpected modification of `%s`', - ATTR_NAME - ); - - var container = ReactMount.findReactContainerForID(id); - if (container && contains(container, node)) { - return true; - } - } - - return false; -} - -function contains(ancestor, descendant) { - if (ancestor.contains) { - // Supported natively in virtually all browsers, but not in jsdom. - return ancestor.contains(descendant); - } - - if (descendant === ancestor) { - return true; - } - - if (descendant.nodeType === 3) { - // If descendant is a text node, start from descendant.parentNode - // instead, so that we can assume all ancestors worth considering are - // element nodes with nodeType === 1. - descendant = descendant.parentNode; - } - - while (descendant && descendant.nodeType === 1) { - if (descendant === ancestor) { - return true; - } - descendant = descendant.parentNode; - } - - return false; -} - -/** - * Causes the cache to forget about one React-specific ID. - * - * @param {string} id The ID to forget. - */ -function purgeID(id) { - delete nodeCache[id]; -} - -exports.ATTR_NAME = ATTR_NAME; -exports.getID = getID; -exports.rawGetID = internalGetID; -exports.setID = setID; -exports.getNode = getNode; -exports.purgeID = purgeID; diff --git a/src/core/ReactInstanceHandles.js b/src/core/ReactInstanceHandles.js index e7b91401e2..15bfe216e2 100644 --- a/src/core/ReactInstanceHandles.js +++ b/src/core/ReactInstanceHandles.js @@ -19,8 +19,6 @@ "use strict"; -var ReactID = require('ReactID'); - var invariant = require('invariant'); var SEPARATOR = '.'; @@ -239,103 +237,6 @@ var ReactInstanceHandles = { ); }, - /** - * True if the supplied `node` is rendered by React. - * - * @param {*} node DOM Element to check. - * @return {boolean} True if the DOM Element appears to be rendered by React. - * @internal - */ - isRenderedByReact: function(node) { - if (node.nodeType !== 1) { - // Not a DOMElement, therefore not a React component - return false; - } - var id = ReactID.getID(node); - return id ? id.charAt(0) === SEPARATOR : false; - }, - - /** - * Traverses up the ancestors of the supplied node to find a node that is a - * DOM representation of a React component. - * - * @param {*} node - * @return {?DOMEventTarget} - * @internal - */ - getFirstReactDOM: function(node) { - var current = node; - while (current && current.parentNode !== current) { - if (ReactInstanceHandles.isRenderedByReact(current)) { - return current; - } - current = current.parentNode; - } - return null; - }, - - /** - * Finds a node with the supplied `id` inside of the supplied `ancestorNode`. - * Exploits the ID naming scheme to perform the search quickly. - * - * @param {DOMEventTarget} ancestorNode Search from this root. - * @pararm {string} id ID of the DOM representation of the component. - * @return {DOMEventTarget} DOM node with the supplied `id`. - * @internal - */ - findComponentRoot: function(ancestorNode, id) { - var firstChildren = [ancestorNode.firstChild]; - var childIndex = 0; - - while (childIndex < firstChildren.length) { - var child = firstChildren[childIndex++]; - while (child) { - var childID = ReactID.getID(child); - if (childID) { - if (id === childID) { - return child; - } else if (isAncestorIDOf(childID, id)) { - // If we find a child whose ID is an ancestor of the given ID, - // then we can be sure that we only want to search the subtree - // rooted at this child, so we can throw out the rest of the - // search state. - firstChildren.length = childIndex = 0; - firstChildren.push(child.firstChild); - break; - } else { - // TODO This should not be necessary if the ID hierarchy is - // correct, but is occasionally necessary if the DOM has been - // modified in unexpected ways. - firstChildren.push(child.firstChild); - } - } else { - // If this child had no ID, then there's a chance that it was - // injected automatically by the browser, as when a `` - // element sprouts an extra `` child as a side effect of - // `.innerHTML` parsing. Optimistically continue down this - // branch, but not before examining the other siblings. - firstChildren.push(child.firstChild); - } - child = child.nextSibling; - } - } - - if (__DEV__) { - console.error( - 'Error while invoking `findComponentRoot` with the following ' + - 'ancestor node:', - ancestorNode - ); - } - invariant( - false, - 'findComponentRoot(..., %s): Unable to find element. This probably ' + - 'means the DOM was unexpectedly mutated (e.g. by the browser).', - id, - ReactID.getID(ancestorNode) - ); - }, - /** * Gets the DOM ID of the React component that is the root of the tree that * contains the React component with the supplied DOM ID. @@ -400,7 +301,11 @@ var ReactInstanceHandles = { * Exposed for unit testing. * @private */ - _getNextDescendantID: getNextDescendantID + _getNextDescendantID: getNextDescendantID, + + isAncestorIDOf: isAncestorIDOf, + + SEPARATOR: SEPARATOR }; diff --git a/src/core/ReactMount.js b/src/core/ReactMount.js index 8713f1bee4..ed1def0d4c 100644 --- a/src/core/ReactMount.js +++ b/src/core/ReactMount.js @@ -22,8 +22,11 @@ var invariant = require('invariant'); var getReactRootElementInContainer = require('getReactRootElementInContainer'); var ReactEventEmitter = require('ReactEventEmitter'); var ReactInstanceHandles = require('ReactInstanceHandles'); -var ReactEventTopLevelCallback = require('ReactEventTopLevelCallback'); -var ReactID = require('ReactID'); + +var SEPARATOR = ReactInstanceHandles.SEPARATOR; + +var ATTR_NAME = 'data-reactid'; +var nodeCache = {}; var $ = require('$'); @@ -44,7 +47,138 @@ if (__DEV__) { */ function getReactRootID(container) { var rootElement = getReactRootElementInContainer(container); - return rootElement && ReactID.getID(rootElement); + return rootElement && ReactMount.getID(rootElement); +} + +/** + * Accessing node[ATTR_NAME] or calling getAttribute(ATTR_NAME) on a form + * element can return its control whose name or ID equals ATTR_NAME. All + * DOM nodes support `getAttributeNode` but this can also get called on + * other objects so just return '' if we're given something other than a + * DOM node (such as window). + * + * @param {?DOMElement|DOMWindow|DOMDocument|DOMTextNode} node DOM node. + * @return {string} ID of the supplied `domNode`. + */ +function getID(node) { + var id = internalGetID(node); + if (id) { + if (nodeCache.hasOwnProperty(id)) { + var cached = nodeCache[id]; + if (cached !== node) { + invariant( + !isValid(cached, id), + 'ReactMount: Two valid but unequal nodes with the same `%s`: %s', + ATTR_NAME, id + ); + + nodeCache[id] = node; + } + } else { + nodeCache[id] = node; + } + } + + return id; +} + +function internalGetID(node) { + // If node is something like a window, document, or text node, none of + // which support attributes or a .getAttribute method, gracefully return + // the empty string, as if the attribute were missing. + return node && node.getAttribute && node.getAttribute(ATTR_NAME) || ''; +} + +/** + * Sets the React-specific ID of the given node. + * + * @param {DOMElement} node The DOM node whose ID will be set. + * @param {string} id The value of the ID attribute. + */ +function setID(node, id) { + var oldID = internalGetID(node); + if (oldID !== id) { + delete nodeCache[oldID]; + } + node.setAttribute(ATTR_NAME, id); + nodeCache[id] = node; +} + +/** + * Finds the node with the supplied React-generated DOM ID. + * + * @param {string} id A React-generated DOM ID. + * @return {DOMElement} DOM node with the suppled `id`. + * @internal + */ +function getNode(id) { + if (!nodeCache.hasOwnProperty(id) || !isValid(nodeCache[id], id)) { + nodeCache[id] = ReactMount.findReactNodeByID(id); + } + return nodeCache[id]; +} + +/** + * A node is "valid" if it is contained by a currently mounted container. + * + * This means that the node does not have to be contained by a document in + * order to be considered valid. + * + * @param {?DOMElement} node The candidate DOM node. + * @param {string} id The expected ID of the node. + * @return {boolean} Whether the node is contained by a mounted container. + */ +function isValid(node, id) { + if (node) { + invariant( + internalGetID(node) === id, + 'ReactMount: Unexpected modification of `%s`', + ATTR_NAME + ); + + var container = ReactMount.findReactContainerForID(id); + if (container && contains(container, node)) { + return true; + } + } + + return false; +} + +function contains(ancestor, descendant) { + if (ancestor.contains) { + // Supported natively in virtually all browsers, but not in jsdom. + return ancestor.contains(descendant); + } + + if (descendant === ancestor) { + return true; + } + + if (descendant.nodeType === 3) { + // If descendant is a text node, start from descendant.parentNode + // instead, so that we can assume all ancestors worth considering are + // element nodes with nodeType === 1. + descendant = descendant.parentNode; + } + + while (descendant && descendant.nodeType === 1) { + if (descendant === ancestor) { + return true; + } + descendant = descendant.parentNode; + } + + return false; +} + +/** + * Causes the cache to forget about one React-specific ID. + * + * @param {string} id The ID to forget. + */ +function purgeID(id) { + delete nodeCache[id]; } /** @@ -89,14 +223,10 @@ var ReactMount = { * Ensures that the top-level event delegation listener is set up. This will * be invoked some time before the first time any React component is rendered. * - * @param {object} TopLevelCallbackCreator * @private */ - prepareTopLevelEvents: function(TopLevelCallbackCreator) { - ReactEventEmitter.ensureListening( - ReactMount.useTouchEvents, - TopLevelCallbackCreator - ); + prepareTopLevelEvents: function() { + ReactEventEmitter.ensureListening(ReactMount.useTouchEvents); }, /** @@ -132,7 +262,7 @@ var ReactMount = { * @return {string} reactRoot ID prefix */ _registerComponent: function(nextComponent, container) { - ReactMount.prepareTopLevelEvents(ReactEventTopLevelCallback); + ReactMount.prepareTopLevelEvents(); var reactRootID = ReactMount.registerContainer(container); instanceByReactRootID[reactRootID] = nextComponent; @@ -196,8 +326,7 @@ var ReactMount = { var reactRootElement = getReactRootElementInContainer(container); var containerHasReactMarkup = - reactRootElement && - ReactInstanceHandles.isRenderedByReact(reactRootElement); + reactRootElement && ReactMount.isRenderedByReact(reactRootElement); var shouldReuseMarkup = containerHasReactMarkup && !registeredComponent; @@ -295,15 +424,15 @@ var ReactMount = { var rootElement = rootElementsByReactRootID[reactRootID]; if (rootElement && rootElement.parentNode !== container) { invariant( - // Call rawGetID here because getID calls isValid which calls + // Call internalGetID here because getID calls isValid which calls // findReactContainerForID (this function). - ReactID.rawGetID(rootElement) === reactRootID, + internalGetID(rootElement) === reactRootID, 'ReactMount: Root element ID differed from reactRootID.' ); var containerChild = container.firstChild; if (containerChild && - reactRootID === ReactID.rawGetID(containerChild)) { + reactRootID === internalGetID(containerChild)) { // If the container has a new child with the same ID as the old // root element, then rootElementsByReactRootID[reactRootID] is // just stale and needs to be updated. The case that deserves a @@ -329,8 +458,122 @@ var ReactMount = { */ findReactNodeByID: function(id) { var reactRoot = ReactMount.findReactContainerForID(id); - return ReactInstanceHandles.findComponentRoot(reactRoot, id); - } + return ReactMount.findComponentRoot(reactRoot, id); + }, + + /** + * True if the supplied `node` is rendered by React. + * + * @param {*} node DOM Element to check. + * @return {boolean} True if the DOM Element appears to be rendered by React. + * @internal + */ + isRenderedByReact: function(node) { + if (node.nodeType !== 1) { + // Not a DOMElement, therefore not a React component + return false; + } + var id = ReactMount.getID(node); + return id ? id.charAt(0) === SEPARATOR : false; + }, + + /** + * Traverses up the ancestors of the supplied node to find a node that is a + * DOM representation of a React component. + * + * @param {*} node + * @return {?DOMEventTarget} + * @internal + */ + getFirstReactDOM: function(node) { + var current = node; + while (current && current.parentNode !== current) { + if (ReactMount.isRenderedByReact(current)) { + return current; + } + current = current.parentNode; + } + return null; + }, + + /** + * Finds a node with the supplied `id` inside of the supplied `ancestorNode`. + * Exploits the ID naming scheme to perform the search quickly. + * + * @param {DOMEventTarget} ancestorNode Search from this root. + * @pararm {string} id ID of the DOM representation of the component. + * @return {DOMEventTarget} DOM node with the supplied `id`. + * @internal + */ + findComponentRoot: function(ancestorNode, id) { + var firstChildren = [ancestorNode.firstChild]; + var childIndex = 0; + + while (childIndex < firstChildren.length) { + var child = firstChildren[childIndex++]; + while (child) { + var childID = ReactMount.getID(child); + if (childID) { + if (id === childID) { + return child; + } else if (ReactInstanceHandles.isAncestorIDOf(childID, id)) { + // If we find a child whose ID is an ancestor of the given ID, + // then we can be sure that we only want to search the subtree + // rooted at this child, so we can throw out the rest of the + // search state. + firstChildren.length = childIndex = 0; + firstChildren.push(child.firstChild); + break; + } else { + // TODO This should not be necessary if the ID hierarchy is + // correct, but is occasionally necessary if the DOM has been + // modified in unexpected ways. + firstChildren.push(child.firstChild); + } + } else { + // If this child had no ID, then there's a chance that it was + // injected automatically by the browser, as when a `
` + // element sprouts an extra `` child as a side effect of + // `.innerHTML` parsing. Optimistically continue down this + // branch, but not before examining the other siblings. + firstChildren.push(child.firstChild); + } + child = child.nextSibling; + } + } + + if (__DEV__) { + console.error( + 'Error while invoking `findComponentRoot` with the following ' + + 'ancestor node:', + ancestorNode + ); + } + invariant( + false, + 'findComponentRoot(..., %s): Unable to find element. This probably ' + + 'means the DOM was unexpectedly mutated (e.g. by the browser).', + id, + ReactMount.getID(ancestorNode) + ); + }, + + + /** + * React ID utilities. + */ + + ATTR_NAME: ATTR_NAME, + + getID: getID, + + setID: setID, + + getNode: getNode, + + purgeID: purgeID, + + injection: {} }; module.exports = ReactMount; diff --git a/src/core/ReactNativeComponent.js b/src/core/ReactNativeComponent.js index c5c7aa55fe..11c1c69fad 100644 --- a/src/core/ReactNativeComponent.js +++ b/src/core/ReactNativeComponent.js @@ -25,7 +25,7 @@ var DOMPropertyOperations = require('DOMPropertyOperations'); var ReactComponent = require('ReactComponent'); var ReactEventEmitter = require('ReactEventEmitter'); var ReactMultiChild = require('ReactMultiChild'); -var ReactID = require('ReactID'); +var ReactMount = require('ReactMount'); var escapeTextForBrowser = require('escapeTextForBrowser'); var flattenChildren = require('flattenChildren'); @@ -136,7 +136,7 @@ ReactNativeComponent.Mixin = { } var escapedID = escapeTextForBrowser(this._rootNodeID); - return ret + ' ' + ReactID.ATTR_NAME + '="' + escapedID + '">'; + return ret + ' ' + ReactMount.ATTR_NAME + '="' + escapedID + '">'; }, /** diff --git a/src/core/ReactTextComponent.js b/src/core/ReactTextComponent.js index 1d0af59858..24c07bd704 100644 --- a/src/core/ReactTextComponent.js +++ b/src/core/ReactTextComponent.js @@ -20,7 +20,7 @@ "use strict"; var ReactComponent = require('ReactComponent'); -var ReactID = require('ReactID'); +var ReactMount = require('ReactMount'); var escapeTextForBrowser = require('escapeTextForBrowser'); var mixInto = require('mixInto'); @@ -58,7 +58,7 @@ mixInto(ReactTextComponent, { mountComponent: function(rootID) { ReactComponent.Mixin.mountComponent.call(this, rootID); return ( - '' + + '' + escapeTextForBrowser(this.props.text) + '' ); diff --git a/src/core/__tests__/ReactCompositeComponent-test.js b/src/core/__tests__/ReactCompositeComponent-test.js index b79b06c524..0ee5268bb5 100644 --- a/src/core/__tests__/ReactCompositeComponent-test.js +++ b/src/core/__tests__/ReactCompositeComponent-test.js @@ -25,7 +25,7 @@ var React; var ReactCurrentOwner; var ReactPropTypes; var ReactTestUtils; -var ReactID; +var ReactMount; var ReactDoNotBindDeprecated; var cx; @@ -41,7 +41,7 @@ describe('ReactCompositeComponent', function() { ReactDoNotBindDeprecated = require('ReactDoNotBindDeprecated'); ReactPropTypes = require('ReactPropTypes'); ReactTestUtils = require('ReactTestUtils'); - ReactID = require('ReactID'); + ReactMount = require('ReactMount'); MorphingComponent = React.createClass({ getInitialState: function() { @@ -129,7 +129,7 @@ describe('ReactCompositeComponent', function() { // rerender instance.setProps({renderAnchor: true, anchorClassOn: false}); var anchorID = instance.getAnchorID(); - var actualDOMAnchorNode = ReactID.getNode(anchorID); + var actualDOMAnchorNode = ReactMount.getNode(anchorID); expect(actualDOMAnchorNode.className).toBe(''); }); diff --git a/src/core/__tests__/ReactDOM-test.js b/src/core/__tests__/ReactDOM-test.js index 6068a81327..fdd88c6757 100644 --- a/src/core/__tests__/ReactDOM-test.js +++ b/src/core/__tests__/ReactDOM-test.js @@ -25,7 +25,7 @@ var React = require('React'); var ReactDOM = require('ReactDOM'); var ReactTestUtils = require('ReactTestUtils'); var React = require('React'); -var ReactID = require('ReactID'); +var ReactMount = require('ReactMount'); describe('ref swapping', function() { // TODO: uncomment this test once we can run in phantom, which @@ -61,7 +61,7 @@ describe('ref swapping', function() { var argDiv = ReactTestUtils.renderIntoDocument( ReactDOM.div(null, 'child') ); - var argNode = ReactID.getNode(argDiv._rootNodeID); + var argNode = ReactMount.getNode(argDiv._rootNodeID); expect(argNode.innerHTML).toBe('child'); }); @@ -69,7 +69,7 @@ describe('ref swapping', function() { var conflictDiv = ReactTestUtils.renderIntoDocument( ReactDOM.div({children: 'fakechild'}, 'child') ); - var conflictNode = ReactID.getNode(conflictDiv._rootNodeID); + var conflictNode = ReactMount.getNode(conflictDiv._rootNodeID); expect(conflictNode.innerHTML).toBe('child'); }); @@ -111,7 +111,7 @@ describe('ref swapping', function() { theBird:
} }); - var root = ReactID.getNode(myDiv._rootNodeID); + var root = ReactMount.getNode(myDiv._rootNodeID); var dog = root.childNodes[0]; expect(dog.className).toBe('bigdog'); }); diff --git a/src/core/__tests__/ReactDOMIDOperations-test.js b/src/core/__tests__/ReactDOMIDOperations-test.js index 77a64475c6..0dfd2bfd47 100644 --- a/src/core/__tests__/ReactDOMIDOperations-test.js +++ b/src/core/__tests__/ReactDOMIDOperations-test.js @@ -23,11 +23,11 @@ describe('ReactDOMIDOperations', function() { var DOMPropertyOperations = require('DOMPropertyOperations'); var ReactDOMIDOperations = require('ReactDOMIDOperations'); - var ReactID = require('ReactID'); + var ReactMount = require('ReactMount'); var keyOf = require('keyOf'); it('should disallow updating special properties', function() { - spyOn(ReactID, "getNode"); + spyOn(ReactMount, "getNode"); spyOn(DOMPropertyOperations, "setValueForProperty"); expect(function() { @@ -39,7 +39,7 @@ describe('ReactDOMIDOperations', function() { }).toThrow(); expect( - ReactID.getNode.argsForCall[0][0] + ReactMount.getNode.argsForCall[0][0] ).toBe('testID'); expect( @@ -49,7 +49,7 @@ describe('ReactDOMIDOperations', function() { it('should update innerHTML and special-case whitespace', function() { var stubNode = document.createElement('div'); - spyOn(ReactID, "getNode").andReturn(stubNode); + spyOn(ReactMount, "getNode").andReturn(stubNode); ReactDOMIDOperations.updateInnerHTMLByID( 'testID', @@ -57,7 +57,7 @@ describe('ReactDOMIDOperations', function() { ); expect( - ReactID.getNode.argsForCall[0][0] + ReactMount.getNode.argsForCall[0][0] ).toBe('testID'); expect(stubNode.innerHTML).toBe(' testContent'); diff --git a/src/core/__tests__/ReactEventEmitter-test.js b/src/core/__tests__/ReactEventEmitter-test.js index 79bf8f2390..5b5e64ccd7 100644 --- a/src/core/__tests__/ReactEventEmitter-test.js +++ b/src/core/__tests__/ReactEventEmitter-test.js @@ -22,7 +22,7 @@ require('mock-modules') .dontMock('BrowserScroll') .dontMock('CallbackRegistry') .dontMock('EventPluginHub') - .dontMock('ReactID') + .dontMock('ReactMount') .dontMock('ReactEventEmitter') .dontMock('ReactInstanceHandles') .dontMock('EventPluginHub') @@ -35,9 +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 ReactMount = require('ReactMount'); +var getID = ReactMount.getID; +var setID = ReactMount.setID; var ReactEventEmitter; var ReactEventTopLevelCallback; var ReactTestUtils; @@ -89,9 +89,9 @@ describe('ReactEventEmitter', function() { require('mock-modules').dumpCache(); EventPluginHub = require('EventPluginHub'); TapEventPlugin = require('TapEventPlugin'); - ReactID = require('ReactID'); - getID = ReactID.getID; - setID = ReactID.setID; + ReactMount = require('ReactMount'); + getID = ReactMount.getID; + setID = ReactMount.setID; ReactEventEmitter = require('ReactEventEmitter'); ReactTestUtils = require('ReactTestUtils'); ReactEventTopLevelCallback = require('ReactEventTopLevelCallback'); diff --git a/src/core/__tests__/ReactIdentity-test.js b/src/core/__tests__/ReactIdentity-test.js index fac5cde23d..bd883436e0 100644 --- a/src/core/__tests__/ReactIdentity-test.js +++ b/src/core/__tests__/ReactIdentity-test.js @@ -22,7 +22,7 @@ var React; var ReactTestUtils; var reactComponentExpect; -var ReactID; +var ReactMount; describe('ReactIdentity', function() { @@ -31,12 +31,12 @@ describe('ReactIdentity', function() { React = require('React'); ReactTestUtils = require('ReactTestUtils'); reactComponentExpect = require('reactComponentExpect'); - ReactID = require('ReactID'); + ReactMount = require('ReactMount'); }); var idExp = /^\.r\[.+?\](.*)$/; function checkId(child, expectedId) { - var actual = idExp.exec(ReactID.getID(child)); + var actual = idExp.exec(ReactMount.getID(child)); var expected = idExp.exec(expectedId); expect(actual).toBeTruthy(); expect(expected).toBeTruthy(); @@ -281,11 +281,11 @@ describe('ReactIdentity', function() { React.renderComponent(wrapped, document.createElement('div')); - var beforeID = ReactID.getID(wrapped.getDOMNode().firstChild); + var beforeID = ReactMount.getID(wrapped.getDOMNode().firstChild); wrapped.swap(); - var afterID = ReactID.getID(wrapped.getDOMNode().firstChild); + var afterID = ReactMount.getID(wrapped.getDOMNode().firstChild); expect(beforeID).not.toEqual(afterID); diff --git a/src/core/__tests__/ReactInstanceHandles-test.js b/src/core/__tests__/ReactInstanceHandles-test.js index 093ae845a5..6d665a2b9b 100644 --- a/src/core/__tests__/ReactInstanceHandles-test.js +++ b/src/core/__tests__/ReactInstanceHandles-test.js @@ -21,7 +21,7 @@ var React = require('React'); var ReactTestUtils = require('ReactTestUtils'); -var ReactID = require('ReactID'); +var ReactMount = require('ReactMount'); /** * Ensure that all callbacks are invoked, passing this unique argument. @@ -78,7 +78,7 @@ describe('ReactInstanceHandles', function() { describe('isRenderedByReact', function() { it('should not crash on text nodes', function() { expect(function() { - ReactInstanceHandles.isRenderedByReact(document.createTextNode('yolo')); + ReactMount.isRenderedByReact(document.createTextNode('yolo')); }).not.toThrow(); }); }); @@ -91,14 +91,14 @@ describe('ReactInstanceHandles', function() { parentNode.appendChild(childNodeA); parentNode.appendChild(childNodeB); - ReactID.setID(parentNode, '.react[0]'); - ReactID.setID(childNodeA, '.react[0].0'); - ReactID.setID(childNodeB, '.react[0].0:1'); + ReactMount.setID(parentNode, '.react[0]'); + ReactMount.setID(childNodeA, '.react[0].0'); + ReactMount.setID(childNodeB, '.react[0].0:1'); expect( - ReactInstanceHandles.findComponentRoot( + ReactMount.findComponentRoot( parentNode, - ReactID.getID(childNodeB) + ReactMount.getID(childNodeB) ) ).toBe(childNodeB); }); @@ -110,14 +110,14 @@ describe('ReactInstanceHandles', function() { parentNode.appendChild(childNodeA); parentNode.appendChild(childNodeB); - ReactID.setID(parentNode, '.react[0]'); + ReactMount.setID(parentNode, '.react[0]'); // No ID on `childNodeA`. - ReactID.setID(childNodeB, '.react[0].0:1'); + ReactMount.setID(childNodeB, '.react[0].0:1'); expect( - ReactInstanceHandles.findComponentRoot( + ReactMount.findComponentRoot( parentNode, - ReactID.getID(childNodeB) + ReactMount.getID(childNodeB) ) ).toBe(childNodeB); }); @@ -129,22 +129,22 @@ describe('ReactInstanceHandles', function() { parentNode.appendChild(childNodeA); childNodeA.appendChild(childNodeB); - ReactID.setID(parentNode, '.react[0]'); + ReactMount.setID(parentNode, '.react[0]'); // No ID on `childNodeA`, it was "rendered by the browser". - ReactID.setID(childNodeB, '.react[0].1:0'); + ReactMount.setID(childNodeB, '.react[0].1:0'); - expect(ReactInstanceHandles.findComponentRoot( + expect(ReactMount.findComponentRoot( parentNode, - ReactID.getID(childNodeB) + ReactMount.getID(childNodeB) )).toBe(childNodeB); spyOn(console, 'error'); expect(console.error.argsForCall.length).toBe(0); expect(function() { - ReactInstanceHandles.findComponentRoot( + ReactMount.findComponentRoot( parentNode, - ReactID.getID(childNodeB) + ":junk" + ReactMount.getID(childNodeB) + ":junk" ); }).toThrow( 'Invariant Violation: findComponentRoot(..., .react[0].1:0:junk): ' + diff --git a/src/core/__tests__/ReactMultiChildReconcile-test.js b/src/core/__tests__/ReactMultiChildReconcile-test.js index 0ff3cf2eab..4d47e89207 100644 --- a/src/core/__tests__/ReactMultiChildReconcile-test.js +++ b/src/core/__tests__/ReactMultiChildReconcile-test.js @@ -23,7 +23,7 @@ require('mock-modules'); var React = require('React'); var ReactTestUtils = require('ReactTestUtils'); -var ReactID = require('ReactID'); +var ReactMount = require('ReactMount'); var objMapKeyVal = require('objMapKeyVal'); @@ -191,7 +191,7 @@ function verifyDomOrderingAccurate(parentInstance, statusDisplays) { var i; var orderedDomIds = []; for (i=0; i < statusDisplayNodes.length; i++) { - orderedDomIds.push(ReactID.getID(statusDisplayNodes[i])); + orderedDomIds.push(ReactMount.getID(statusDisplayNodes[i])); } var orderedLogicalIds = []; diff --git a/src/core/__tests__/ReactNativeComponent-test.js b/src/core/__tests__/ReactNativeComponent-test.js index b7462325e9..8e0481ecea 100644 --- a/src/core/__tests__/ReactNativeComponent-test.js +++ b/src/core/__tests__/ReactNativeComponent-test.js @@ -317,7 +317,7 @@ describe('ReactNativeComponent', function() { it("should clean up listeners", function() { var React = require('React'); var ReactEventEmitter = require('ReactEventEmitter'); - var ReactID = require('ReactID'); + var ReactMount = require('ReactMount'); var container = document.createElement('div'); document.documentElement.appendChild(container); @@ -327,7 +327,7 @@ describe('ReactNativeComponent', function() { React.renderComponent(instance, container); var rootNode = instance.getDOMNode(); - var rootNodeID = ReactID.getID(rootNode); + var rootNodeID = ReactMount.getID(rootNode); expect( ReactEventEmitter.getListener(rootNodeID, 'onClick') ).toBe(callback); diff --git a/src/environment/__tests__/ReactServerRendering-test.js b/src/environment/__tests__/ReactServerRendering-test.js index 192b54cc3b..7fd3f3138f 100644 --- a/src/environment/__tests__/ReactServerRendering-test.js +++ b/src/environment/__tests__/ReactServerRendering-test.js @@ -24,7 +24,7 @@ require('mock-modules') .dontMock('ExecutionEnvironment') .dontMock('React') - .dontMock('ReactID') + .dontMock('ReactMount') .dontMock('ReactServerRendering') .dontMock('ReactTestUtils') .dontMock('ReactMarkupChecksum'); @@ -32,7 +32,7 @@ require('mock-modules') var mocks = require('mocks'); var React; -var ReactID; +var ReactMount; var ReactTestUtils; var ReactServerRendering; var ReactMarkupChecksum; @@ -42,7 +42,7 @@ describe('ReactServerRendering', function() { beforeEach(function() { require('mock-modules').dumpCache(); React = require('React'); - ReactID = require('ReactID'); + ReactMount = require('ReactMount'); ReactTestUtils = require('ReactTestUtils'); ExecutionEnvironment = require('ExecutionEnvironment'); ExecutionEnvironment.canUseDOM = false; @@ -59,7 +59,7 @@ describe('ReactServerRendering', function() { } ); expect(response).toMatch( - 'hello world' ); }); @@ -83,11 +83,11 @@ describe('ReactServerRendering', function() { } ); expect(response).toMatch( - '
' + - '' + - 'My name is ' + - 'child' + + '' + + 'My name is ' + + 'child' + '' + '
' ); @@ -137,10 +137,10 @@ describe('ReactServerRendering', function() { ); expect(response).toMatch( - '' + - 'Component name: ' + - 'TestComponent' + + 'Component name: ' + + 'TestComponent' + '' ); expect(lifecycle).toEqual( diff --git a/src/eventPlugins/EnterLeaveEventPlugin.js b/src/eventPlugins/EnterLeaveEventPlugin.js index f7a385ba18..3a3af3fd51 100644 --- a/src/eventPlugins/EnterLeaveEventPlugin.js +++ b/src/eventPlugins/EnterLeaveEventPlugin.js @@ -22,14 +22,13 @@ var EventConstants = require('EventConstants'); var EventPropagators = require('EventPropagators'); var ExecutionEnvironment = require('ExecutionEnvironment'); -var ReactInstanceHandles = require('ReactInstanceHandles'); var SyntheticMouseEvent = require('SyntheticMouseEvent'); -var ReactID = require('ReactID'); +var ReactMount = require('ReactMount'); var keyOf = require('keyOf'); var topLevelTypes = EventConstants.topLevelTypes; -var getFirstReactDOM = ReactInstanceHandles.getFirstReactDOM; +var getFirstReactDOM = ReactMount.getFirstReactDOM; var eventTypes = { mouseEnter: {registrationName: keyOf({onMouseEnter: null})}, @@ -85,8 +84,8 @@ var EnterLeaveEventPlugin = { return null; } - var fromID = from ? ReactID.getID(from) : ''; - var toID = to ? ReactID.getID(to) : ''; + var fromID = from ? ReactMount.getID(from) : ''; + var toID = to ? ReactMount.getID(to) : ''; var leave = SyntheticMouseEvent.getPooled( eventTypes.mouseLeave, diff --git a/src/test/ReactTestUtils.js b/src/test/ReactTestUtils.js index b8784183d6..9eb71e5983 100644 --- a/src/test/ReactTestUtils.js +++ b/src/test/ReactTestUtils.js @@ -21,7 +21,7 @@ var React = require('React'); var ReactComponent = require('ReactComponent'); var ReactEventEmitter = require('ReactEventEmitter'); var ReactTextComponent = require('ReactTextComponent'); -var ReactID = require('ReactID'); +var ReactMount = require('ReactMount'); var mergeInto = require('mergeInto'); @@ -222,10 +222,10 @@ var ReactTestUtils = { ReactEventEmitter.TopLevelCallbackCreator.createTopLevelCallback( topLevelType ); - var node = ReactID.getNode(reactRootID); + var node = ReactMount.getNode(reactRootID); fakeNativeEvent.target = node; /* jsdom is returning nodes without id's - fixing that issue here. */ - ReactID.setID(node, reactRootID); + ReactMount.setID(node, reactRootID); virtualHandler(fakeNativeEvent); }, From 759425fc90642722d5d9f111d373eb55ca84e790 Mon Sep 17 00:00:00 2001 From: Tim Yung Date: Wed, 24 Jul 2013 17:19:45 -0700 Subject: [PATCH 32/35] Use `invariant` in `OrderedMap` Just some therapeutic cleanup. --- src/utils/OrderedMap.js | 131 +++++++++++++++++++--------------------- 1 file changed, 62 insertions(+), 69 deletions(-) diff --git a/src/utils/OrderedMap.js b/src/utils/OrderedMap.js index 1bf238fd32..8c139263d7 100644 --- a/src/utils/OrderedMap.js +++ b/src/utils/OrderedMap.js @@ -20,40 +20,9 @@ var invariant = require('invariant'); var mixInto = require('mixInto'); -var throwIf = require('throwIf'); var PREFIX = 'key:'; -var ARRAY_MUST_CB = 'ARRAY_MUST_CB'; -var INVALID_KEY = 'INVALID_KEY'; -var DUPLICATE_KEY = 'DUPLICATE_KEY'; -var OPERATION_ARGS = 'OPERATION_ARGS'; -var RANGE_ARGS = 'RANGE_ARGS'; -var NO_EXIST = 'NO_EXIST'; - -if (__DEV__) { - ARRAY_MUST_CB = - 'If providing an array to an OrderedMap constructor, you must provide ' + - 'a callback that may determine the unique key for each entry. ' + - 'The key that you return from the callback should uniquely define that ' + - 'entity. What is returned from that function must answer the question: ' + - '"If you were to shuffle the array of entities, how would you be able to ' + - 'determine the identity of the Array entry?"'; - INVALID_KEY = - 'Key must be non-empty, non-null, string or number'; - DUPLICATE_KEY = - 'At creation time, you must extract out IDs that are unique.'; - OPERATION_ARGS = - 'Invalid argument type for construction or operation on OrderedMap. ' + - 'from/merge accepts another OrderedMap. fromArray accepts an Array ' + - 'and a callback to extract a unique ID.'; - RANGE_ARGS = - 'OrderedMap.mapRange requires end of range to be >= start, start to be ' + - '>= 0 and end < length.'; - NO_EXIST = - 'The requested key does not exist in the OrderedMap'; -} - /** * Utility to extract a backing object from an initialization `Array`, allowing * the caller to assist in resolving the unique ID for each entry via the @@ -70,9 +39,12 @@ function extractObjectFromArray(arr, keyExtractor) { for (var i=0; i < arr.length; i++) { var item = arr[i]; var key = keyExtractor(item); - validatePublicKey(key); + assertValidPublicKey(key); var normalizedKey = PREFIX + key; - throwIf(normalizedKey in normalizedObj, DUPLICATE_KEY); + invariant( + !(normalizedKey in normalizedObj), + 'OrderedMap: IDs returned by the key extraction function must be unique.' + ); normalizedObj[normalizedKey] = item; } return normalizedObj; @@ -120,12 +92,11 @@ function OrderedMapImpl(normalizedObj, computedLength) { * `OrderedMap`. * @throws Error if key is not appropriate for use in `OrderedMap`. */ -function validatePublicKey(key) { - var isEmpty = key === ''; - var correctKeyType = typeof key === 'string' || typeof key === 'number'; - if (isEmpty || !correctKeyType) { - throw new Error(INVALID_KEY); - } +function assertValidPublicKey(key) { + invariant( + key !== '' && (typeof key === 'string' || typeof key === 'number'), + 'OrderedMap: Key must be non-empty, non-null string or number.' + ); } /** @@ -137,15 +108,16 @@ function validatePublicKey(key) { * exceeded. * @return {void} description */ -function validateRangeIndices(start, length, actualLen) { - var startType = typeof start; - var lengthType = typeof length; - var invalid = - startType !== 'number' || lengthType !== 'number' || - length < 0 || start < 0 || start + length > actualLen; - if (invalid) { - throw new Error(RANGE_ARGS); - } +function assertValidRangeIndices(start, length, actualLen) { + invariant( + typeof start === 'number' && + typeof length === 'number' && + length >= 0 && + start >= 0 && + start + length <= actualLen, + 'OrderedMap: `mapRange` and `forEachRange` expect non-negative start and ' + + 'length arguments within the bounds of the instance.' + ); } /** @@ -157,10 +129,12 @@ function validateRangeIndices(start, length, actualLen) { * @return {OrderedMap} new `OrderedMap` that results in merging `a` and `b`. */ function _fromNormalizedObjects(a, b) { - // Second optional, both must be a Plain Old JavaScript Object. - var invalidArgs = - !a || a.constructor !== Object || (b && b.constructor !== Object); - throwIf(invalidArgs, OPERATION_ARGS); + // Second optional, both must be plain JavaScript objects. + invariant( + a && a.constructor === Object && (!b || b.constructor === Object), + 'OrderedMap: Corrupted instance of OrderedMap detected.' + ); + var newSet = {}; var length = 0; var key; @@ -203,7 +177,7 @@ var OrderedMapMethods = { * @throws Error if provided known invalid key. */ has: function(key) { - validatePublicKey(key); + assertValidPublicKey(key); var normalizedKey = PREFIX + key; return normalizedKey in this._normalizedObj; }, @@ -217,7 +191,7 @@ var OrderedMapMethods = { * @throws Error if provided known invalid key. */ get: function(key) { - validatePublicKey(key); + assertValidPublicKey(key); var normalizedKey = PREFIX + key; return this.has(key) ? this._normalizedObj[normalizedKey] : undefined; }, @@ -235,7 +209,10 @@ var OrderedMapMethods = { * merge. */ merge: function(orderedMap) { - throwIf(!(orderedMap instanceof OrderedMapImpl), OPERATION_ARGS); + invariant( + orderedMap instanceof OrderedMapImpl, + 'OrderedMap.merge(...): Expected an OrderedMap instance.' + ); return _fromNormalizedObjects( this._normalizedObj, orderedMap._normalizedObj @@ -267,7 +244,7 @@ var OrderedMapMethods = { var thisSet = this._normalizedObj; var newSet = {}; var i = 0; - validateRangeIndices(start, length, this.length); + assertValidRangeIndices(start, length, this.length); var end = start + length - 1; for (var key in thisSet) { if (thisSet.hasOwnProperty(key)) { @@ -323,7 +300,7 @@ var OrderedMapMethods = { }, forEachRange: function(cb, start, length, context) { - validateRangeIndices(start, length, this.length); + assertValidRangeIndices(start, length, this.length); var thisSet = this._normalizedObj; var i = 0; var end = start + length - 1; @@ -354,9 +331,10 @@ var OrderedMapMethods = { startIndex !== undefined && endIndex !== undefined, 'mapKeyRange must be given keys that are present.' ); - if (endIndex < startIndex) { - throw new Error(RANGE_ARGS); - } + invariant( + endIndex >= startIndex, + 'OrderedMap.mapKeyRange(...): `endKey` must not come before `startIndex`.' + ); return this.mapRange(cb, startIndex, (endIndex - startIndex) + 1, context); }, @@ -367,9 +345,11 @@ var OrderedMapMethods = { startIndex !== undefined && endIndex !== undefined, 'forEachKeyRange must be given keys that are present.' ); - if (endIndex < startIndex) { - throw new Error(RANGE_ARGS); - } + invariant( + endIndex >= startIndex, + 'OrderedMap.forEachKeyRange(...): `endKey` must not come before ' + + '`startIndex`.' + ); this.forEachRange(cb, startIndex, (endIndex - startIndex) + 1, context); }, @@ -413,7 +393,11 @@ var OrderedMapMethods = { */ nthKeyAfter: function(key, n) { var curIndex = this.indexOfKey(key); - throwIf(curIndex === undefined, NO_EXIST); + invariant( + curIndex !== undefined, + 'OrderedMap.nthKeyAfter: The key `%s` does not exist in this instance.', + key + ); return this.keyAtIndex(curIndex + n); }, @@ -434,7 +418,7 @@ var OrderedMapMethods = { * key is not found. */ indexOfKey: function(key) { - validatePublicKey(key); + assertValidPublicKey(key); var normalizedKey = PREFIX + key; var computedPositions = this._getOrComputePositions(); var computedPosition = computedPositions.indexByKey[normalizedKey]; @@ -505,14 +489,23 @@ mixInto(OrderedMapImpl, OrderedMapMethods); var OrderedMap = { from: function(orderedMap) { - var invalidArg = !(orderedMap instanceof OrderedMapImpl); - throwIf(invalidArg, OPERATION_ARGS); + invariant( + orderedMap instanceof OrderedMapImpl, + 'OrderedMap.from(...): Expected an OrderedMap instance.' + ); return _fromNormalizedObjects(orderedMap._normalizedObj, null); }, fromArray: function(arr, keyExtractor) { - throwIf(!Array.isArray(arr), OPERATION_ARGS); - throwIf(!keyExtractor, ARRAY_MUST_CB); + invariant( + Array.isArray(arr), + 'OrderedMap.fromArray(...): First argument must be an array.' + ); + invariant( + typeof keyExtractor === 'function', + 'OrderedMap.fromArray(...): Second argument must be a function used ' + + 'to determine the unique key for each entry.' + ); return new OrderedMapImpl( extractObjectFromArray(arr, keyExtractor), arr.length From 8d48610b7e6a9d31a5ea9044b180c73053d066b7 Mon Sep 17 00:00:00 2001 From: Tim Yung Date: Wed, 24 Jul 2013 17:19:49 -0700 Subject: [PATCH 33/35] Typecheck ImmutableObject Just some therapeutic cleanup. --- src/utils/ImmutableObject.js | 149 ++++++++++++++++++----------------- 1 file changed, 76 insertions(+), 73 deletions(-) diff --git a/src/utils/ImmutableObject.js b/src/utils/ImmutableObject.js index 91476eb323..ed595691d8 100644 --- a/src/utils/ImmutableObject.js +++ b/src/utils/ImmutableObject.js @@ -14,112 +14,111 @@ * limitations under the License. * * @providesModule ImmutableObject + * @typechecks */ "use strict"; -var keyMirror = require('keyMirror'); +var invariant = require('invariant'); var merge = require('merge'); var mergeInto = require('mergeInto'); var mergeHelpers = require('mergeHelpers'); -var throwIf = require('throwIf'); var checkMergeObjectArgs = mergeHelpers.checkMergeObjectArgs; var isTerminal = mergeHelpers.isTerminal; /** - * Simple wrapper around javascript key/value objects that provide a dev time - * guarantee of immutability (assuming all modules that may potentially mutate - * include a "use strict" declaration). Retaining immutability requires CPU - * cycles (in order to perform the freeze), but this computation can be avoided - * in production. The fact that mutation attempts in __DEV__ will be caught, - * allows us to reasonably assume that mutation on those objects won't even be - * attempted in production. This means that an object being an instanceof - * `ImmutableObject` implies that the object may never change. - * - * TODO: Require strict mode in source files that use ImmutableObject (lint - * rule). - * - * @class ImmutableObject + * Wrapper around JavaScript objects that provide a guarantee of immutability at + * developer time when strict mode is used. The extra computations required to + * enforce immutability is stripped out in production for performance reasons. */ - -var ERRORS; var ImmutableObject; -if (__DEV__) { - ERRORS = { - INVALID_MAP_SET_ARG: 'You have attempted to set fields on an object that ' + - 'is not an instance of ImmutableObject' - }; +function assertImmutableObject(immutableObject) { + invariant( + immutableObject instanceof ImmutableObject, + 'ImmutableObject: Attempted to set fields on an object that is not an ' + + 'instance of ImmutableObject.' + ); +} +if (__DEV__) { /** * Constructs an instance of `ImmutableObject`. * - * @param {!Object} initMap The initial set of properties. + * @param {?object} initialProperties The initial set of properties. * @constructor */ - ImmutableObject = function(initMap) { - mergeInto(this, initMap); - deepFreeze(this, initMap); + ImmutableObject = function ImmutableObject(initialProperties) { + mergeInto(this, initialProperties); + deepFreeze(this); }; /** - * Objects that are instances of `ImmutableObject` are assumed to be deep - * frozen. - * @param {!Object} o The object to deep freeze. - * @return {!boolean} Whether or not deep freeze is needed. - */ - var shouldRecurseFreeze = function(o) { - return (typeof o) === 'object' && - !(o instanceof ImmutableObject) && o !== null; - }; - - /** - * Freezes an object `o` deeply. Invokes `shouldRecurseFreeze` to determine if - * further freezing is needed. + * Checks if an object should be deep frozen. Instances of `ImmutableObject` + * are assumed to have already been deep frozen. * - * @param {!Object} o The object to freeze. + * @param {*} object The object to check. + * @return {boolean} Whether or not deep freeze is needed. */ - var deepFreeze = function(o) { - var prop; - Object.freeze(o); // First freeze the object. - for (prop in o) { - var field = o[prop]; - if (o.hasOwnProperty(prop) && shouldRecurseFreeze(field)) { + var shouldRecurseFreeze = function(object) { + return ( + typeof object === 'object' && + !(object instanceof ImmutableObject) && + object !== null + ); + }; + + /** + * Freezes the supplied object deeply. + * + * @param {*} object The object to freeze. + */ + var deepFreeze = function(object) { + Object.freeze(object); // First freeze the object. + for (var prop in object) { + var field = object[prop]; + if (object.hasOwnProperty(prop) && shouldRecurseFreeze(field)) { deepFreeze(field); } } }; /** - * Returns a new ImmutableObject that is that is the same as the parameter - * `immutableObject` but with the differences specified in `put`. - * @param {!ImmutableObject} ImmutableObject The ImmutableObject object to set - * fields on. - * @param {!Object} put Subset of fields to merge into the returned result. - * @return {!ImmutableObject} The result of merging in `put` fields. + * Returns a new ImmutableObject that is identical to the supplied object but + * with the supplied changes, `put`. + * + * @param {ImmutableObject} immutableObject Starting object. + * @param {?object} put Fields to merge into the object. + * @return {ImmutableObject} The result of merging in `put` fields. */ ImmutableObject.set = function(immutableObject, put) { - throwIf( - !(immutableObject instanceof ImmutableObject), - ERRORS.INVALID_MAP_SET_ARG - ); + assertImmutableObject(immutableObject); var totalNewFields = merge(immutableObject, put); return new ImmutableObject(totalNewFields); }; } else { - ERRORS = keyMirror({INVALID_MAP_SET_ARG: null}); - - ImmutableObject = function(initMap) { - mergeInto(this, initMap); + /** + * Constructs an instance of `ImmutableObject`. + * + * @param {?object} initialProperties The initial set of properties. + * @constructor + */ + ImmutableObject = function ImmutableObject(initialProperties) { + mergeInto(this, initialProperties); }; + /** + * Returns a new ImmutableObject that is identical to the supplied object but + * with the supplied changes, `put`. + * + * @param {ImmutableObject} immutableObject Starting object. + * @param {?object} put Fields to merge into the object. + * @return {ImmutableObject} The result of merging in `put` fields. + */ ImmutableObject.set = function(immutableObject, put) { - throwIf( - !(immutableObject instanceof ImmutableObject), - ERRORS.INVALID_MAP_SET_ARG - ); + assertImmutableObject(immutableObject); var newMap = new ImmutableObject(immutableObject); mergeInto(newMap, put); return newMap; @@ -127,8 +126,12 @@ if (__DEV__) { } /** - * Sugar for `ImmutableObject.set(ImmutableObject, {fieldName: putField})` - * @see ImmutableObject.set + * Sugar for `ImmutableObject.set(ImmutableObject, {fieldName: putField})`. + * + * @param {ImmutableObject} immutableObject Object on which to set field. + * @param {string} fieldName Name of the field to set. + * @param {*} putField Value of the field to set. + * @return {ImmutableObject} [description] */ ImmutableObject.setField = function(immutableObject, fieldName, putField) { var put = {}; @@ -137,15 +140,15 @@ ImmutableObject.setField = function(immutableObject, fieldName, putField) { }; /** - * Returns a new ImmutableObject that is that is the same as the parameter - * `immutableObject` but with the differences specified in `put` recursively - * applied. + * Returns a new ImmutableObject that is identical to the supplied object but + * with the supplied changes recursively applied. + * + * @param {ImmutableObject} immutableObject Object on which to set fields. + * @param {object} put Fields to merge into the object. + * @return {ImmutableObject} The result of merging in `put` fields. */ ImmutableObject.setDeep = function(immutableObject, put) { - throwIf( - !(immutableObject instanceof ImmutableObject), - ERRORS.INVALID_MAP_SET_ARG - ); + assertImmutableObject(immutableObject); return _setDeep(immutableObject, put); }; From bdf2a9bb124b8fc34120949b322202497741f239 Mon Sep 17 00:00:00 2001 From: Tim Yung Date: Wed, 24 Jul 2013 17:19:54 -0700 Subject: [PATCH 34/35] Use `invariant` in `react/utils` Just some therapeutic cleanup. --- src/utils/OrderedMap.js | 2 - src/utils/Transaction.js | 75 +++++++++++++++---------------- src/utils/accumulate.js | 60 +++++++++---------------- src/utils/escapeTextForBrowser.js | 31 +++++++------ src/utils/flattenChildren.js | 18 ++++---- src/utils/keyMirror.js | 38 ++++++++-------- src/utils/mapAllChildren.js | 16 +++---- src/utils/mergeDeepInto.js | 26 +++-------- src/utils/mergeHelpers.js | 73 ++++++++++-------------------- src/utils/traverseAllChildren.js | 29 +++--------- 10 files changed, 147 insertions(+), 221 deletions(-) diff --git a/src/utils/OrderedMap.js b/src/utils/OrderedMap.js index 8c139263d7..57cfa54383 100644 --- a/src/utils/OrderedMap.js +++ b/src/utils/OrderedMap.js @@ -85,8 +85,6 @@ function OrderedMapImpl(normalizedObj, computedLength) { * Validates a "public" key - that is, one that the public facing API supplies. * The key is then normalized for internal storage. In order to be considered * valid, all keys must be non-empty, defined, non-null strings or numbers. - * Since this already costs a function invocation, will avoid additional call to - * `throwIf`. * * @param {string?} key Validates that the key is suitable for use in a * `OrderedMap`. diff --git a/src/utils/Transaction.js b/src/utils/Transaction.js index 8e630d88ab..861919d3d1 100644 --- a/src/utils/Transaction.js +++ b/src/utils/Transaction.js @@ -18,23 +18,7 @@ "use strict"; -var throwIf = require('throwIf'); - -var DUAL_TRANSACTION = 'DUAL_TRANSACTION'; -var MISSING_TRANSACTION = 'MISSING_TRANSACTION'; -if (__DEV__) { - DUAL_TRANSACTION = - 'Cannot initialize transaction when there is already an outstanding ' + - 'transaction. Common causes of this are trying to render a component ' + - 'when you are already rendering a component or attempting a state ' + - 'transition while in a render function. Another possibility is that ' + - 'you are rendering new content (or state transitioning) in a ' + - 'componentDidRender callback. If this is not the case, please report the ' + - 'issue immediately.'; - - MISSING_TRANSACTION = - 'Cannot close transaction when there is none open.'; -} +var invariant = require('invariant'); /** * `Transaction` creates a black box that is able to wrap any method such that @@ -156,26 +140,33 @@ var Mixin = { * @return Return value from `method`. */ perform: function(method, scope, a, b, c, d, e, f) { - throwIf(this.isInTransaction(), DUAL_TRANSACTION); + invariant( + !this.isInTransaction(), + 'Transaction.perform(...): Cannot initialize a transaction when there ' + + 'is already an outstanding transaction.' + ); var memberStart = Date.now(); - var err = null; + var errorToThrow = null; var ret; try { this.initializeAll(); ret = method.call(scope, a, b, c, d, e, f); - } catch (ie_requires_catch) { - err = ie_requires_catch; + } catch (error) { + // IE8 requires `catch` in order to use `finally`. + errorToThrow = error; } finally { var memberEnd = Date.now(); this.methodInvocationTime += (memberEnd - memberStart); try { this.closeAll(); - } catch (closeAllErr) { - err = err || closeAllErr; + } catch (closeError) { + // If `method` throws, prefer to show that stack trace over any thrown + // by invoking `closeAll`. + errorToThrow = errorToThrow || closeError; } } - if (err) { - throw err; + if (errorToThrow) { + throw errorToThrow; } return ret; }, @@ -184,15 +175,17 @@ var Mixin = { this._isInTransaction = true; var transactionWrappers = this.transactionWrappers; var wrapperInitTimes = this.timingMetrics.wrapperInitTimes; - var err = null; + var errorToThrow = null; for (var i = 0; i < transactionWrappers.length; i++) { var initStart = Date.now(); var wrapper = transactionWrappers[i]; try { - this.wrapperInitData[i] = - wrapper.initialize ? wrapper.initialize.call(this) : null; - } catch (initErr) { - err = err || initErr; // Remember the first error. + this.wrapperInitData[i] = wrapper.initialize ? + wrapper.initialize.call(this) : + null; + } catch (initError) { + // Prefer to show the stack trace of the first error. + errorToThrow = errorToThrow || initError; this.wrapperInitData[i] = Transaction.OBSERVED_ERROR; } finally { var curInitTime = wrapperInitTimes[i]; @@ -200,8 +193,8 @@ var Mixin = { wrapperInitTimes[i] = (curInitTime || 0) + (initEnd - initStart); } } - if (err) { - throw err; + if (errorToThrow) { + throw errorToThrow; } }, @@ -212,10 +205,13 @@ var Mixin = { * invoked). */ closeAll: function() { - throwIf(!this.isInTransaction(), MISSING_TRANSACTION); + invariant( + this.isInTransaction(), + 'Transaction.closeAll(): Cannot close transaction when none are open.' + ); var transactionWrappers = this.transactionWrappers; var wrapperCloseTimes = this.timingMetrics.wrapperCloseTimes; - var err = null; + var errorToThrow = null; for (var i = 0; i < transactionWrappers.length; i++) { var wrapper = transactionWrappers[i]; var closeStart = Date.now(); @@ -224,8 +220,9 @@ var Mixin = { if (initData !== Transaction.OBSERVED_ERROR) { wrapper.close && wrapper.close.call(this, initData); } - } catch (closeErr) { - err = err || closeErr; // Remember the first error. + } catch (closeError) { + // Prefer to show the stack trace of the first error. + errorToThrow = errorToThrow || closeError; } finally { var closeEnd = Date.now(); var curCloseTime = wrapperCloseTimes[i]; @@ -234,14 +231,16 @@ var Mixin = { } this.wrapperInitData.length = 0; this._isInTransaction = false; - if (err) { - throw err; + if (errorToThrow) { + throw errorToThrow; } } }; var Transaction = { + Mixin: Mixin, + /** * Token to look for to determine if an error occured. */ diff --git a/src/utils/accumulate.js b/src/utils/accumulate.js index 86f58b966a..7905f2bf9c 100644 --- a/src/utils/accumulate.js +++ b/src/utils/accumulate.js @@ -18,50 +18,34 @@ "use strict"; -var throwIf = require('throwIf'); - -var INVALID_ARGS = 'INVALID_ACCUM_ARGS'; - -if (__DEV__) { - INVALID_ARGS = - 'accumulate requires non empty (non-null, defined) next ' + - 'values. All arrays accumulated must not contain any empty items.'; -} +var invariant = require('invariant'); /** - * Accumulates items that must never be empty, into a result in a manner that - * conserves memory - avoiding allocation of arrays until they are needed. The - * accumulation may start and/or end up being a single element or an array - * depending on the total count (if greater than one, an array is allocated). - * Handles most common case first (starting with an empty current value and - * acquiring one). - * @return {Accumulation} An accumulation which is either a single item or an - * Array of items. + * Accumulates items that must not be null or undefined. + * + * This is used to conserve memory by avoiding array allocations. + * + * @return {*|array<*>} An accumulation of items. */ -function accumulate(cur, next) { - var curValIsEmpty = cur == null; // Will test for emptiness (null/undef) - var nextValIsEmpty = next === null; - if (__DEV__) { - throwIf(nextValIsEmpty, INVALID_ARGS); - } - if (nextValIsEmpty) { - return cur; +function accumulate(current, next) { + invariant( + next != null, + 'accumulate(...): Accumulated items must be not be null or undefined.' + ); + if (current == null) { + return next; } else { - if (curValIsEmpty) { - return next; + // Both are not empty. Warning: Never call x.concat(y) when you are not + // certain that x is an Array (x could be a string with concat method). + var currentIsArray = Array.isArray(current); + var nextIsArray = Array.isArray(next); + if (currentIsArray) { + return current.concat(next); } else { - // Both are not empty. Warning: Never call x.concat(y) when you are not - // certain that x is an Array (x could be a string with concat method). - var curIsArray = Array.isArray(cur); - var nextIsArray = Array.isArray(next); - if (curIsArray) { - return cur.concat(next); + if (nextIsArray) { + return [current].concat(next); } else { - if (nextIsArray) { - return [cur].concat(next); - } else { - return [cur, next]; - } + return [current, next]; } } } diff --git a/src/utils/escapeTextForBrowser.js b/src/utils/escapeTextForBrowser.js index eba906b9ab..8e49d31e9e 100644 --- a/src/utils/escapeTextForBrowser.js +++ b/src/utils/escapeTextForBrowser.js @@ -14,19 +14,12 @@ * limitations under the License. * * @providesModule escapeTextForBrowser + * @typechecks static-only */ "use strict"; -var throwIf = require('throwIf'); - -var ESCAPE_TYPE_ERR; - -if (__DEV__) { - ESCAPE_TYPE_ERR = - 'The React core has attempted to escape content that is of a ' + - 'mysterious type (object etc) Escaping only works on numbers and strings'; -} +var invariant = require('invariant'); var ESCAPE_LOOKUP = { "&": "&", @@ -41,13 +34,19 @@ function escaper(match) { return ESCAPE_LOOKUP[match]; } -var escapeTextForBrowser = function (text) { +/** + * Escapes text to prevent scripting attacks. + * + * @param {number|string} text Text value to escape. + * @return {string} An escaped string. + */ +function escapeTextForBrowser(text) { var type = typeof text; - var invalid = type === 'object'; - if (__DEV__) { - throwIf(invalid, ESCAPE_TYPE_ERR); - } - if (text === '' || invalid) { + invariant( + type !== 'object', + 'escapeTextForBrowser(...): Attempted to escape an object.' + ); + if (text === '') { return ''; } else { if (type === 'string') { @@ -56,6 +55,6 @@ var escapeTextForBrowser = function (text) { return (''+text).replace(/[&><"'\/]/g, escaper); } } -}; +} module.exports = escapeTextForBrowser; diff --git a/src/utils/flattenChildren.js b/src/utils/flattenChildren.js index ba2d6bdf77..17b24ef220 100644 --- a/src/utils/flattenChildren.js +++ b/src/utils/flattenChildren.js @@ -18,7 +18,7 @@ "use strict"; -var throwIf = require('throwIf'); +var invariant = require('invariant'); var traverseAllChildren = require('traverseAllChildren'); /** @@ -27,14 +27,14 @@ var traverseAllChildren = require('traverseAllChildren'); * @param {!string} name String name of key path to child. */ function flattenSingleChildIntoContext(traverseContext, child, name) { - // We found a component instance + // We found a component instance. var result = traverseContext; - if (__DEV__) { - throwIf( - result.hasOwnProperty(name), - traverseAllChildren.DUPLICATE_KEY_ERROR - ); - } + invariant( + !result.hasOwnProperty(name), + 'flattenChildren(...): Encountered two children with the same key, `%s`. ' + + 'Children keys must be unique.', + name + ); result[name] = child; } @@ -43,7 +43,7 @@ function flattenSingleChildIntoContext(traverseContext, child, name) { * @return {!object} flattened children keyed by name. */ function flattenChildren(children) { - if (children === null || children === undefined) { + if (children == null) { return children; } var result = {}; diff --git a/src/utils/keyMirror.js b/src/utils/keyMirror.js index bc64cabd9d..4e078e85a4 100644 --- a/src/utils/keyMirror.js +++ b/src/utils/keyMirror.js @@ -14,36 +14,38 @@ * limitations under the License. * * @providesModule keyMirror + * @typechecks static-only */ "use strict"; -var throwIf = require('throwIf'); - -var NOT_OBJECT_ERROR = 'NOT_OBJECT_ERROR'; -if (__DEV__) { - NOT_OBJECT_ERROR = 'keyMirror only works on objects'; -} +var invariant = require('invariant'); /** - * Utility for constructing enums with keys being equal to the associated - * values, even when using advanced key crushing. This is useful for debugging, - * but also for using the values themselves as lookups into the enum. - * Example: - * var COLORS = keyMirror({blue: null, red: null}); - * var myColor = COLORS.blue; - * var isColorValid = !!COLORS[myColor] + * Constructs an enumeration with keys equal to their value. + * + * For example: + * + * var COLORS = keyMirror({blue: null, red: null}); + * var myColor = COLORS.blue; + * var isColorValid = !!COLORS[myColor]; + * * The last line could not be performed if the values of the generated enum were * not equal to their keys. - * Input: {key1: val1, key2: val2} - * Output: {key1: key1, key2: key2} + * + * Input: {key1: val1, key2: val2} + * Output: {key1: key1, key2: key2} + * + * @param {object} obj + * @return {object} */ var keyMirror = function(obj) { var ret = {}; var key; - - throwIf(!(obj instanceof Object) || Array.isArray(obj), NOT_OBJECT_ERROR); - + invariant( + obj instanceof Object && !Array.isArray(obj), + 'keyMirror(...): Argument must be an object.' + ); for (key in obj) { if (!obj.hasOwnProperty(key)) { continue; diff --git a/src/utils/mapAllChildren.js b/src/utils/mapAllChildren.js index c035d9797d..9eff484798 100644 --- a/src/utils/mapAllChildren.js +++ b/src/utils/mapAllChildren.js @@ -20,7 +20,7 @@ var PooledClass = require('PooledClass'); -var throwIf = require('throwIf'); +var invariant = require('invariant'); var traverseAllChildren = require('traverseAllChildren'); var threeArgumentPooler = PooledClass.threeArgumentPooler; @@ -50,13 +50,13 @@ function mapSingleChildIntoContext(traverseContext, child, name, i) { var mapFunction = mapBookKeeping.mapFunction; var mapContext = mapBookKeeping.mapContext; var mappedChild = mapFunction.call(mapContext, child, name, i); - // We found a component instance - if (__DEV__) { - throwIf( - mapResult.hasOwnProperty(name), - traverseAllChildren.DUPLICATE_KEY_ERROR - ); - } + // We found a component instance. + invariant( + !mapResult.hasOwnProperty(name), + 'mapAllChildren(...): Encountered two children with the same key, `%s`. ' + + 'Children keys must be unique.', + name + ); mapResult[name] = mappedChild; } diff --git a/src/utils/mergeDeepInto.js b/src/utils/mergeDeepInto.js index 36131bb320..e86c95729f 100644 --- a/src/utils/mergeDeepInto.js +++ b/src/utils/mergeDeepInto.js @@ -21,9 +21,8 @@ "use strict"; -var keyMirror = require('keyMirror'); +var invariant = require('invariant'); var mergeHelpers = require('mergeHelpers'); -var throwIf = require('throwIf'); var ArrayStrategies = mergeHelpers.ArrayStrategies; var checkArrayStrategy = mergeHelpers.checkArrayStrategy; @@ -33,20 +32,6 @@ var checkMergeObjectArgs = mergeHelpers.checkMergeObjectArgs; var isTerminal = mergeHelpers.isTerminal; var normalizeMergeArg = mergeHelpers.normalizeMergeArg; -var ERRORS = keyMirror({ - RUN_TIME_ARRAY_MERGE_FAIL: null -}); - -if (__DEV__) { - ERRORS = { - RUN_TIME_ARRAY_MERGE_FAIL: - "The caller has not supplied an ArrayStrategy. This is supported as " + - "long as the data structures being merged do not contain two Arrays " + - "that must be merged together, which is exactly what has just " + - "happened. Change the call site to supply an Array merge resolver." - }; -} - /** * Every deep merge function must handle merging in each of the following cases * at every level. We may refer to letters below in implementations. For each @@ -161,9 +146,10 @@ var mergeSingleFieldDeep = function(one, two, key, arrayStrategy, level) { if (twoValIsTerminal) { // [E] one[key] = twoVal; } else if (twoValIsArray) { // [F] - throwIf( - !ArrayStrategies[arrayStrategy], - ERRORS.RUN_TIME_ARRAY_MERGE_FAIL + invariant( + ArrayStrategies[arrayStrategy], + 'mergeDeepInto(...): Attempted to merge two arrays, but a valid ' + + 'ArrayStrategy was not specified.' ); // Else: At this point, the only other valid option is `IndexByIndex` if (arrayStrategy === ArrayStrategies.Clobber) { @@ -234,7 +220,7 @@ var mergeSingleFieldDeep = function(one, two, key, arrayStrategy, level) { */ var mergeDeepInto = function(one, twoParam, arrayStrategy) { var two = normalizeMergeArg(twoParam); - checkArrayStrategy(arrayStrategy); // Will be checked twice, for now + checkArrayStrategy(arrayStrategy); // Will be checked twice, for now. mergeDeepIntoObjects(one, two, arrayStrategy, 0); }; diff --git a/src/utils/mergeHelpers.js b/src/utils/mergeHelpers.js index 5b46ff5d8e..d25cfcff87 100644 --- a/src/utils/mergeHelpers.js +++ b/src/utils/mergeHelpers.js @@ -20,8 +20,8 @@ "use strict"; +var invariant = require('invariant'); var keyMirror = require('keyMirror'); -var throwIf = require('throwIf'); /** * Maximum number of levels to traverse. Will catch circular structures. @@ -29,41 +29,6 @@ var throwIf = require('throwIf'); */ var MAX_MERGE_DEPTH = 36; -var ERRORS = keyMirror({ - MERGE_ARRAY_FAIL: null, - MERGE_CORE_FAILURE: null, - MERGE_TYPE_USAGE_FAILURE: null, - MERGE_DEEP_MAX_LEVELS: null, - MERGE_DEEP_NO_ARR_STRATEGY: null -}); - -if (__DEV__) { - ERRORS = { - MERGE_ARRAY_FAIL: - 'Unsupported type passed to a merge function. You may have passed a ' + - 'structure that contains an array and the merge function does not know ' + - 'how to merge arrays. ', - - MERGE_CORE_FAILURE: - 'Critical assumptions about the merge functions have been violated. ' + - 'This is the fault of the merge functions themselves, not necessarily ' + - 'the callers.', - - MERGE_TYPE_USAGE_FAILURE: - 'Calling merge function with invalid types. You may call merge ' + - 'functions (non-array non-terminal) OR (null/undefined) arguments. ' + - 'mergeInto functions have the same requirements but with an added ' + - 'restriction that the first parameter must not be null/undefined.', - - MERGE_DEEP_MAX_LEVELS: - 'Maximum deep merge depth exceeded. You may attempting to merge ' + - 'circular structures in an unsupported way.', - MERGE_DEEP_NO_ARR_STRATEGY: - 'You must provide an array strategy to deep merge functions to ' + - 'instruct the deep merge how to resolve merging two arrays.' - }; -} - /** * We won't worry about edge cases like new String('x') or new Boolean(true). * Functions are considered terminals, and arrays are not. @@ -99,9 +64,11 @@ var mergeHelpers = { * @param {*} two Array to merge from. */ checkMergeArrayArgs: function(one, two) { - throwIf( - !Array.isArray(one) || !Array.isArray(two), - ERRORS.MERGE_CORE_FAILURE + invariant( + Array.isArray(one) && Array.isArray(two), + 'Critical assumptions about the merge functions have been violated. ' + + 'This is the fault of the merge functions themselves, not necessarily ' + + 'the callers.' ); }, @@ -118,7 +85,12 @@ var mergeHelpers = { * @param {*} arg */ checkMergeObjectArg: function(arg) { - throwIf(isTerminal(arg) || Array.isArray(arg), ERRORS.MERGE_CORE_FAILURE); + invariant( + !isTerminal(arg) && !Array.isArray(arg), + 'Critical assumptions about the merge functions have been violated. ' + + 'This is the fault of the merge functions themselves, not necessarily ' + + 'the callers.' + ); }, /** @@ -128,19 +100,23 @@ var mergeHelpers = { * @param {number} Level of recursion to validate against maximum. */ checkMergeLevel: function(level) { - throwIf(level >= MAX_MERGE_DEPTH, ERRORS.MERGE_DEEP_MAX_LEVELS); + invariant( + level < MAX_MERGE_DEPTH, + 'Maximum deep merge depth exceeded. You may be attempting to merge ' + + 'circular structures in an unsupported way.' + ); }, /** - * Checks that a merge was not given a circular object or an object that had - * too great of depth. + * Checks that the supplied merge strategy is valid. * - * @param {number} Level of recursion to validate against maximum. + * @param {string} Array merge strategy. */ checkArrayStrategy: function(strategy) { - throwIf( - strategy !== undefined && !(strategy in mergeHelpers.ArrayStrategies), - ERRORS.MERGE_DEEP_NO_ARR_STRATEGY + invariant( + strategy === undefined || strategy in mergeHelpers.ArrayStrategies, + 'You must provide an array strategy to deep merge functions to ' + + 'instruct the deep merge how to resolve merging two arrays.' ); }, @@ -154,9 +130,8 @@ var mergeHelpers = { ArrayStrategies: keyMirror({ Clobber: true, IndexByIndex: true - }), + }) - ERRORS: ERRORS }; module.exports = mergeHelpers; diff --git a/src/utils/traverseAllChildren.js b/src/utils/traverseAllChildren.js index 9fa6f1881f..21dd506d84 100644 --- a/src/utils/traverseAllChildren.js +++ b/src/utils/traverseAllChildren.js @@ -21,22 +21,7 @@ var ReactComponent = require('ReactComponent'); var ReactTextComponent = require('ReactTextComponent'); -var throwIf = require('throwIf'); - -/** - * @polyFill Array.isArray - */ -var DUPLICATE_KEY_ERROR = 'DUPLICATE_KEY_ERROR'; -var INVALID_CHILD = 'INVALID_CHILD'; -if (__DEV__) { - INVALID_CHILD = - 'You may not pass a child of that type to a React component. It ' + - 'is a common mistake to try to pass a standard browser DOM element ' + - 'as a child of a React component.'; - DUPLICATE_KEY_ERROR = - 'You have two children with identical keys. Make sure that you set the ' + - '"key" property to a unique value such as a row ID.'; -} +var invariant = require('invariant'); /** * TODO: Test that: @@ -88,7 +73,11 @@ var traverseAllChildrenImpl = subtreeCount = 1; } else { if (type === 'object') { - throwIf(children && children.nodeType === 1, INVALID_CHILD); + invariant( + children || children.nodeType !== 1, + 'traverseAllChildren(...): Encountered an invalid child; DOM ' + + 'elements are not valid children of React components.' + ); for (var key in children) { if (children.hasOwnProperty(key)) { subtreeCount += traverseAllChildrenImpl( @@ -135,10 +124,4 @@ function traverseAllChildren(children, callback, traverseContext) { } } -/** - * Export the error code for use in other walking/mapping code. - */ -traverseAllChildren.DUPLICATE_KEY_ERROR = DUPLICATE_KEY_ERROR; - - module.exports = traverseAllChildren; From 2e43de20cccaab8c09adaffcefdd58a63c465f86 Mon Sep 17 00:00:00 2001 From: Paul O'Shannessy Date: Fri, 26 Jul 2013 11:22:43 -0700 Subject: [PATCH 35/35] Cleanup 2nd param to ReactEventEmitter.ensureListening It was removed, so these callsites aren't actually doing anything. --- src/core/ReactEventEmitter.js | 2 +- src/core/__tests__/ReactEventEmitter-test.js | 4 +--- src/eventPlugins/__tests__/AnalyticsEventPlugin-test.js | 4 +--- 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/core/ReactEventEmitter.js b/src/core/ReactEventEmitter.js index a5971ff789..419a94381e 100644 --- a/src/core/ReactEventEmitter.js +++ b/src/core/ReactEventEmitter.js @@ -244,7 +244,7 @@ var ReactEventEmitter = { /** * React references `ReactEventTopLevelCallback` using this property in order - * to allow dependency injection via `ensureListening`. + * to allow dependency injection. */ TopLevelCallbackCreator: null, diff --git a/src/core/__tests__/ReactEventEmitter-test.js b/src/core/__tests__/ReactEventEmitter-test.js index 5b5e64ccd7..d62fba45bd 100644 --- a/src/core/__tests__/ReactEventEmitter-test.js +++ b/src/core/__tests__/ReactEventEmitter-test.js @@ -39,7 +39,6 @@ var ReactMount = require('ReactMount'); var getID = ReactMount.getID; var setID = ReactMount.setID; var ReactEventEmitter; -var ReactEventTopLevelCallback; var ReactTestUtils; var TapEventPlugin; @@ -94,10 +93,9 @@ describe('ReactEventEmitter', function() { setID = ReactMount.setID; ReactEventEmitter = require('ReactEventEmitter'); ReactTestUtils = require('ReactTestUtils'); - ReactEventTopLevelCallback = require('ReactEventTopLevelCallback'); idCallOrder = []; tapMoveThreshold = TapEventPlugin.tapMoveThreshold; - ReactEventEmitter.ensureListening(false, ReactEventTopLevelCallback); + ReactEventEmitter.ensureListening(false); EventPluginHub.injection.injectEventPluginsByName({ TapEventPlugin: TapEventPlugin }); diff --git a/src/eventPlugins/__tests__/AnalyticsEventPlugin-test.js b/src/eventPlugins/__tests__/AnalyticsEventPlugin-test.js index fb6dbf0749..0fc9cde79d 100644 --- a/src/eventPlugins/__tests__/AnalyticsEventPlugin-test.js +++ b/src/eventPlugins/__tests__/AnalyticsEventPlugin-test.js @@ -27,7 +27,6 @@ describe('AnalyticsEventPlugin', function() { var EventPluginRegistry; var React; var ReactEventEmitter; - var ReactEventTopLevelCallback; var ReactTestUtils; var DefaultEventPluginOrder; @@ -42,7 +41,6 @@ describe('AnalyticsEventPlugin', function() { EventPluginRegistry = require('EventPluginRegistry'); React = require('React'); ReactEventEmitter = require('ReactEventEmitter'); - ReactEventTopLevelCallback = require('ReactEventTopLevelCallback'); ReactTestUtils = require('ReactTestUtils'); EventPluginRegistry._resetEventPlugins(); @@ -63,7 +61,7 @@ describe('AnalyticsEventPlugin', function() { 'ChangeEventPlugin': ChangeEventPlugin }); - ReactEventEmitter.ensureListening(false, ReactEventTopLevelCallback); + ReactEventEmitter.ensureListening(false); }); it('should count events correctly', function() {