You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@couchdb.apache.org by be...@apache.org on 2015/10/05 19:43:15 UTC

fauxton commit: updated refs/heads/master to 604f534

Repository: couchdb-fauxton
Updated Branches:
  refs/heads/master 6fa212ecc -> 604f5346e


Add DOM check before mounting/unmounting React components

The recent API Bar refactor unveiled a rather neat problem that
showed up in the Dashboard only. The Replication page failed to
load and threw a React "Invariant Violation" error which we've
seen show up from time to time but couldn't yet reproduce. What
was happening was that when attempting to mount the component,
the DOM element just didn't exist. The same problem
could occur when unmounting.

Now to go down the rabbithole! This may get boring, be warned...

The reason it occurred in the dash only is because some page
layouts don't contain the #api-navbar element. It looks like
all Fauxton pages do.

Adding in a DOM check to confirm the existence of the apibar
at this point...
https://github.com/apache/couchdb-fauxton/blob/master/app/addons/fauxton/base.js#L64
... has no effect because the core routeObject uses the
`two_pane.html` layout template as a base template when loading,
and that DOES contain the element. But due to the complexity of
the way our pages load, that template gets loaded first, THEN
the route object's template gets loaded. So basically we don't
know for sure whether the element exists until the moment it's
inserted/removed, hence the checks added in this PR.

I dedided to throw a warning in these cases. Seems like it should
be caught.

This fix should solve all of those Invariant Violation errors
of this kind, not just the problem with the API Bar.


Project: http://git-wip-us.apache.org/repos/asf/couchdb-fauxton/repo
Commit: http://git-wip-us.apache.org/repos/asf/couchdb-fauxton/commit/604f5346
Tree: http://git-wip-us.apache.org/repos/asf/couchdb-fauxton/tree/604f5346
Diff: http://git-wip-us.apache.org/repos/asf/couchdb-fauxton/diff/604f5346

Branch: refs/heads/master
Commit: 604f5346e35070efb700c4452f96eaea3174898e
Parents: 6fa212e
Author: Ben Keen <be...@gmail.com>
Authored: Fri Oct 2 11:51:18 2015 -0700
Committer: Ben Keen <be...@gmail.com>
Committed: Mon Oct 5 10:08:04 2015 -0700

----------------------------------------------------------------------
 app/core/routeObject.js           | 12 ++++++++++--
 app/core/tests/routeObjectSpec.js | 23 +++++++++++++++++------
 2 files changed, 27 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/couchdb-fauxton/blob/604f5346/app/core/routeObject.js
----------------------------------------------------------------------
diff --git a/app/core/routeObject.js b/app/core/routeObject.js
index c3705b8..94f6c60 100644
--- a/app/core/routeObject.js
+++ b/app/core/routeObject.js
@@ -133,7 +133,11 @@ function (FauxtonAPI, React, Backbone) {
 
     renderReactComponents: function () {
       _.each(this.reactComponents, function (componentInfo, selector) {
-        React.render(React.createElement(componentInfo.component, componentInfo.props), $(selector)[0]);
+        if ($(selector)[0]) {
+          React.render(React.createElement(componentInfo.component, componentInfo.props), $(selector)[0]);
+        } else {
+          console.warn("Unable to mount reactor component. Missing selector: ", selector);
+        }
       });
     },
 
@@ -256,7 +260,11 @@ function (FauxtonAPI, React, Backbone) {
 
     removeComponent: function (selector) {
       if (_.has(this.reactComponents, selector)) {
-        React.unmountComponentAtNode($(selector)[0]);
+        if ($(selector)[0]) {
+          React.unmountComponentAtNode($(selector)[0]);
+        } else {
+          console.warn("Unable to unmount react component. Missing selector: ", selector);
+        }
         this.reactComponents[selector] = null;
         delete this.reactComponents[selector];
       }

http://git-wip-us.apache.org/repos/asf/couchdb-fauxton/blob/604f5346/app/core/tests/routeObjectSpec.js
----------------------------------------------------------------------
diff --git a/app/core/tests/routeObjectSpec.js b/app/core/tests/routeObjectSpec.js
index a3fb115..bcb8760 100644
--- a/app/core/tests/routeObjectSpec.js
+++ b/app/core/tests/routeObjectSpec.js
@@ -15,8 +15,10 @@ define([
   'testUtils'
 ], function (FauxtonAPI, React, testUtils) {
   var assert = testUtils.assert,
-  restore = testUtils.restore,
-  RouteObject = FauxtonAPI.RouteObject;
+      restore = testUtils.restore,
+      RouteObject = FauxtonAPI.RouteObject,
+      TestUtils = React.addons.TestUtils;
+
 
   describe('RouteObjects', function () {
 
@@ -197,15 +199,24 @@ define([
           restore(React.unmountComponentAtNode);
         });
 
-        it('removes existing component via react', function () {
+        it('removes existing components via React', function () {
           var spy = sinon.stub(React, 'unmountComponentAtNode');
           var fakeSelector = 'remove-selector';
-          testRouteObject.reactComponents[fakeSelector] = React.createElement('div');
 
-          testRouteObject.removeComponent(fakeSelector);
+          var container = document.createElement('div');
+          var Hmm = React.createClass({displayName: "Hmm",
+            render: function () {
+              return (
+                React.createElement("div", {id: "remove-selector"})
+              );
+            }
+          });
 
-          assert.ok(spy.calledOnce);
+          TestUtils.renderIntoDocument(React.createElement('Hmm'), container);
+          testRouteObject.reactComponents[fakeSelector] = React.createElement('div');
+          testRouteObject.removeComponent(fakeSelector);
 
+          assert.ok(_.isUndefined(testRouteObject.reactComponents[fakeSelector]));
         });
 
         it('removes existing components key', function () {