fix leak on server side, not store singleton instance to global object

README.md

@@ -86,7 +86,6 @@ var I13nDempApp = setupI13n(DemoApp, {
 
 Or follow our guide and [create your own](./docs/guides/createPlugins.md).
 
-
 ## I13n Tree
 ![I13n Tree](https://cloud.githubusercontent.com/assets/3829183/7980892/0b38eb70-0a60-11e5-8cc2-712ec42089fc.png)
 

docs/api/setupI13n.md

@@ -29,6 +29,15 @@ var I13nDempApp = setupI13n(DemoApp, {
 // then you could use I13nDemoApp to render you app
 ```
 
+### Create and access the ReactI13n instance
+
+What we do with `setupI13n` is that we will create the `ReactI13n` instance, along with a root node of the I13nTree, passing them via component context to the children. 
+
+It's designed to work within React components, you should be able to just use [utilFuctions](https://github.com/yahoo/react-i13n/blob/master/docs/guides/utilFunctions.md) and trigger i13n events. In case you want to do this out of React components, you can access `window._reactI13nInstance` directly.
+
+If you have multiple React trees in one page, we will create multiple i13n trees based on how many React tree you have. On client side the [utilFuctions](https://github.com/yahoo/react-i13n/blob/master/docs/guides/utilFunctions.md) still work based on the global instance object, while on server side, only the children under `setupI13n` can get the React i13n instance as we don't have a proper way to share the ReactI13n instance without causing [memory leak](https://github.com/yahoo/react-i13n/pull/100).
+
+
 ### Util Functions
 
 You will get i13n util functions automatically via `this.props.i13n` by using `setupI13n`, more detail please refer to [util functions](../guides/utilFunctions.md).

package.json

@@ -46,6 +46,7 @@
     "promise": "^7.0.1",
     "react": "^15.0.0",
     "react-dom": "^15.0.0",
+    "webpack": "^1.13.1",
     "xunit-file": "~0.0.7"
   },
   "keywords": [

src/libs/ReactI13n.js

@@ -10,7 +10,6 @@ var debug = debugLib('ReactI13n');
 var EventsQueue = require('./EventsQueue');
 var I13nNode = require('./I13nNode');
 var DEFAULT_HANDLER_TIMEOUT = 1000;
-var GLOBAL_OBJECT = ('client' === ENVIRONMENT) ? window : global;
 var ENVIRONMENT = (typeof window !== 'undefined') ? 'client' : 'server';
 
 // export the debug lib in client side
@@ -35,16 +34,12 @@ var ReactI13n = function ReactI13n (options) {
     debug('init', options);
     options = options || {};
     this._i13nNodeClass = 'function' === typeof options.i13nNodeClass ? options.i13nNodeClass : I13nNode;
-
     this._plugins = {};
     this._eventsQueues = {};
     this._isViewportEnabled = options.isViewportEnabled || false;
     this._rootModelData = options.rootModelData || {};
     this._handlerTimeout = options.handlerTimeout || DEFAULT_HANDLER_TIMEOUT;
     this._scrollableContainerId = options.scrollableContainerId || undefined;
-
-    // set itself to the global object so that we can get it anywhere by the static function getInstance
-    GLOBAL_OBJECT.reactI13n = this;
 };
 
 /**
@@ -54,7 +49,12 @@ var ReactI13n = function ReactI13n (options) {
  * @static
  */
 ReactI13n.getInstance = function getInstance () {
-    return GLOBAL_OBJECT.reactI13n;
+    if ('client' === ENVIRONMENT) {
+        return window._reactI13nInstance;
+    } else if ('production' !== process.env.NODE_ENV) {
+        console.warn('ReactI13n instance is not avaialble on server side with ReactI13n.getInstance, ' + 
+            'please use this.props.i13n or this.context.i13n to access ReactI13n utils');
+    }
 };
 
 /**
@@ -63,11 +63,11 @@ ReactI13n.getInstance = function getInstance () {
  */
 ReactI13n.prototype.createRootI13nNode = function createRootI13nNode () {
     var I13nNodeClass = this.getI13nNodeClass();
-    GLOBAL_OBJECT.rootI13nNode = new I13nNodeClass(null, this._rootModelData, false);
+    this._rootI13nNode = new I13nNodeClass(null, this._rootModelData, false);
     if ('client' === ENVIRONMENT) {
-        GLOBAL_OBJECT.rootI13nNode.setDOMNode(document.body);
+        this._rootI13nNode.setDOMNode(document.body);
     }
-    return GLOBAL_OBJECT.rootI13nNode;
+    return this._rootI13nNode;
 };
 
 /**
@@ -191,7 +191,7 @@ ReactI13n.prototype.getScrollableContainerDOMNode = function getScrollableContai
  * @return {Object} root react i13n node
  */
 ReactI13n.prototype.getRootI13nNode = function getRootI13nNode () {
-    return GLOBAL_OBJECT.rootI13nNode;
+    return this._rootI13nNode;
 };
 
 /**

src/mixins/I13nMixin.js

@@ -14,6 +14,7 @@ var DebugDashboard = require('../utils/DebugDashboard');
 var I13nUtils = require('./I13nUtils');
 var listen = require('subscribe-ui-event').listen;
 var ReactI13n = require('../libs/ReactI13n');
+var I13nNode = require('../libs/I13nNode');
 var ViewportMixin = require('./viewport/ViewportMixin');
 require('setimmediate');
 var IS_DEBUG_MODE = isDebugMode();
@@ -137,9 +138,6 @@ var I13nMixin = {
      * @method componentWillMount
      */
     componentWillMount: function () {
-        if (!this._getReactI13n()) {
-            return;
-        }
         clearTimeout(pageInitViewportDetectionTimeout);
         this._createI13nNode();
         this._i13nNode.setReactComponent(this);
@@ -366,14 +364,15 @@ var I13nMixin = {
     _createI13nNode: function () {
         // check if reactI13n is initialized successfully, otherwise return
         var self = this;
-        var I13nNode = self._getReactI13n().getI13nNodeClass();
         var parentI13nNode = self._getParentI13nNode();
+        var reactI13n = self._getReactI13n();
+        var I13nNodeClass = (reactI13n && reactI13n.getI13nNodeClass()) || I13nNode;
         // TODO @kaesonho remove BC for model
-        self._i13nNode = new I13nNode(
+        self._i13nNode = new I13nNodeClass(
             parentI13nNode,
             self.props.i13nModel || self.props.model,
             self.isLeafNode(),
-            self._getReactI13n().isViewportEnabled());
+            reactI13n && reactI13n.isViewportEnabled());
     },
 
     /**

src/mixins/I13nUtils.js

@@ -1,5 +1,6 @@
 var React = require('react');
 var ReactI13n = require('../libs/ReactI13n');
+var IS_CLIENT = typeof window !== 'undefined';
 
 /**
  * React.js I13n Utils Mixin
@@ -12,18 +13,21 @@ var I13nUtils = {
         i13n: React.PropTypes.shape({
             executeEvent: React.PropTypes.func,
             getI13nNode: React.PropTypes.func,
-            parentI13nNode: React.PropTypes.object
+            parentI13nNode: React.PropTypes.object,
+            _reactI13nInstance: React.PropTypes.object
         })
     },
 
     childContextTypes: {
         i13n: React.PropTypes.shape({
             executeEvent: React.PropTypes.func,
             getI13nNode: React.PropTypes.func,
-            parentI13nNode: React.PropTypes.object
+            parentI13nNode: React.PropTypes.object,
+            _reactI13nInstance: React.PropTypes.object
         })
     },
 
+
     /**
      * getChildContext
      * @method getChildContext
@@ -33,7 +37,8 @@ var I13nUtils = {
             i13n: {
                 executeEvent: this.executeI13nEvent,
                 getI13nNode: this.getI13nNode,
-                parentI13nNode: this._i13nNode
+                parentI13nNode: this._i13nNode,
+                _reactI13nInstance: this._getReactI13n()
             }
         };
     },
@@ -48,11 +53,23 @@ var I13nUtils = {
      */
     executeI13nEvent: function (eventName, payload, callback) {
         var reactI13nInstance = this._getReactI13n();
+        var errorMessage = '';
         payload = payload || {};
         payload.i13nNode = payload.i13nNode || this.getI13nNode();
         if (reactI13nInstance) {
             reactI13nInstance.execute(eventName, payload, callback);
         } else {
+            if ('production' !== process.env.NODE_ENV) {
+                errorMessage = 'ReactI13n instance is not found, ' + 
+                    'please make sure you have setupI13n on the root component. ';
+                if (!IS_CLIENT) {
+                    errorMessage += 'On server side, ' + 
+                        'you can only execute the i13n event on the components under setupI13n, ' + 
+                        'please make sure you are calling executeI13nEvent correctly';
+                }
+                console && console.warn && console.warn(errorMessage);
+                console && console.trace && console.trace();
+            }
             callback && callback();
         }
     },
@@ -63,9 +80,6 @@ var I13nUtils = {
      * @return {Object} i13n node
      */
     getI13nNode: function () {
-        if (!this._getReactI13n()) {
-            return;
-        }
         return this._i13nNode || this._getParentI13nNode();
     },
 
@@ -76,7 +90,13 @@ var I13nUtils = {
      * @return {Object} react i13n instance
      */
     _getReactI13n: function () {
-        return ReactI13n.getInstance();
+        var globalReactI13n;
+        if (IS_CLIENT) {
+            globalReactI13n = window._reactI13nInstance;
+        }
+        return this._reactI13nInstance || 
+            (this.context && this.context.i13n && this.context.i13n._reactI13nInstance) ||
+            globalReactI13n;
     },
 
     /**
@@ -86,9 +106,10 @@ var I13nUtils = {
      * @return {Object} parent i13n node
      */
     _getParentI13nNode: function () {
-        // https://twitter.com/andreypopp/status/578974316483608576, get the context from parent context
+        var reactI13n = this._getReactI13n();
         var context = this.context;
-        return (context && context.i13n && context.i13n.parentI13nNode) || this._getReactI13n().getRootI13nNode();
+        return (context && context.i13n && context.i13n.parentI13nNode) || 
+            (reactI13n && reactI13n.getRootI13nNode());
     }
 }
 

src/utils/setupI13n.js

@@ -7,6 +7,7 @@
 var React = require('react');
 var ReactI13n = require('../libs/ReactI13n');
 var I13nUtils = require('../mixins/I13nUtils');
+var IS_CLIENT = typeof window !== 'undefined';
 
 /**
  * Create an app level component with i13n setup
@@ -25,11 +26,6 @@ module.exports = function setupI13n (Component, options, plugins) {
     var RootI13nComponent;
     var componentName = Component.displayName || Component.name;
 
-    var reactI13n = new ReactI13n(options);
-    plugins.forEach(function setPlugin(plugin) {
-        reactI13n.plug(plugin);
-    });
-
     RootI13nComponent = React.createClass({
 
         mixins: [I13nUtils],
@@ -43,15 +39,25 @@ module.exports = function setupI13n (Component, options, plugins) {
          * @method componentWillMount
          */
         componentWillMount: function () {
-            var reactI13n = ReactI13n.getInstance();
+            var reactI13n = new ReactI13n(options);
+            this._reactI13nInstance = reactI13n;
+            // we might have case to access reactI13n instance to execute event outside react components
+            // assign reactI13n to window
+            if (IS_CLIENT) {
+                window._reactI13nInstance = reactI13n;
+            }
+            plugins.forEach(function setPlugin(plugin) {
+                reactI13n.plug(plugin);
+            });
             reactI13n.createRootI13nNode();
         },
 
         render: function () {
             var props = Object.assign({}, {
                 i13n: {
                     executeEvent: this.executeI13nEvent,
-                    getI13nNode: this.getI13nNode
+                    getI13nNode: this.getI13nNode,
+                    reactI13nInstance: this._reactI13nInstance
                 }
             }, this.props);
             return React.createElement(

tests/unit/libs/ReactI13n.js

@@ -15,7 +15,6 @@ describe('ReactI13n', function () {
         var reactI13n = new ReactI13n({
             isViewportEnabled: true
         });
-        expect(ReactI13n.getInstance()).to.eql(reactI13n); // static function should be able to get the instance we created
         expect(reactI13n.isViewportEnabled()).to.eql(true);
     });
 

tests/unit/utils/createI13nNode.js

@@ -40,16 +40,14 @@ var mockSubscribe = {
     }
 };
 var mockClickHandler = function () {};
-MockReactI13n.getInstance = function () {
-    return mockData.reactI13n;
-};
 
 function findProps(elem) {
-  try {
-    return elem[Object.keys(elem).find(function (key) {
-      return key.indexOf('__reactInternalInstance') === 0;
-    })]._currentElement.props;
-  } catch (e) {}
+    try {
+        return elem[Object.keys(elem).find(function (key) {
+            return key.indexOf('__reactInternalInstance') === 0 || 
+                key.indexOf('_reactInternalComponent') === 0;
+        })]._currentElement.props;
+    } catch (e) {}
 }
 
 describe('createI13nNode', function () {
@@ -89,6 +87,7 @@ describe('createI13nNode', function () {
                     return mockData.isViewportEnabled;
                 }
             };
+            global.window._reactI13nInstance = mockData.reactI13n;
 
             done();
         });
@@ -182,7 +181,7 @@ describe('createI13nNode', function () {
             }
         });
         var I13nTestComponent = createI13nNode(TestComponent);
-        mockData.reactI13n = null;
+        window._reactI13nInstance = null;
         var container = document.createElement('div');
         var component = ReactDOM.render(React.createElement(I13nTestComponent, {}), container);
         expect(component).to.be.an('object');
@@ -442,21 +441,21 @@ describe('createI13nNode', function () {
     });
 
     it('should not pass i13n props to string components', function () {
-      var props = {
-          i13nModel: {sec: 'foo'},
-          href: '#/foobar'
-      };
-      var I13nTestComponent = createI13nNode('a', {
-          follow: true,
-          isLeafNode: true,
-          bindClickEvent: true,
-          scanLinks: {enable: true}
-      }, {
-          refToWrappedComponent: 'wrappedElement'
-      });
-      mockData.reactI13n.execute = function () {};
-      var container = document.createElement('div');
-      var component = ReactDOM.render(React.createElement(I13nTestComponent, props), container);
-      expect(findProps(component.refs.wrappedElement)).to.eql({href: '#/foobar', children: undefined});
+        var props = {
+            i13nModel: {sec: 'foo'},
+            href: '#/foobar'
+        };
+        var I13nTestComponent = createI13nNode('a', {
+            follow: true,
+            isLeafNode: true,
+            bindClickEvent: true,
+            scanLinks: {enable: true}
+        }, {
+            refToWrappedComponent: 'wrappedElement'
+        });
+        mockData.reactI13n.execute = function () {};
+        var container = document.createElement('div');
+        var component = ReactDOM.render(React.createElement(I13nTestComponent, props), container);
+        expect(findProps(component.refs.wrappedElement)).to.eql({href: '#/foobar', children: undefined});
     });
 });