Skip to content

Commit

Permalink
not store instance to global, remove custom I13nNode support (#100)
Browse files Browse the repository at this point in the history
  • Loading branch information
kaesonho authored Aug 10, 2016
1 parent 858e80a commit 9e5392f
Show file tree
Hide file tree
Showing 9 changed files with 92 additions and 59 deletions.
1 change: 0 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
9 changes: 9 additions & 0 deletions docs/api/setupI13n.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": [
Expand Down
20 changes: 10 additions & 10 deletions src/libs/ReactI13n.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
};

/**
Expand All @@ -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');
}
};

/**
Expand All @@ -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;
};

/**
Expand Down Expand Up @@ -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;
};

/**
Expand Down
11 changes: 5 additions & 6 deletions src/mixins/I13nMixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -137,9 +138,6 @@ var I13nMixin = {
* @method componentWillMount
*/
componentWillMount: function () {
if (!this._getReactI13n()) {
return;
}
clearTimeout(pageInitViewportDetectionTimeout);
this._createI13nNode();
this._i13nNode.setReactComponent(this);
Expand Down Expand Up @@ -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());
},

/**
Expand Down
39 changes: 30 additions & 9 deletions src/mixins/I13nUtils.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
var React = require('react');
var ReactI13n = require('../libs/ReactI13n');
var IS_CLIENT = typeof window !== 'undefined';

/**
* React.js I13n Utils Mixin
Expand All @@ -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
Expand All @@ -33,7 +37,8 @@ var I13nUtils = {
i13n: {
executeEvent: this.executeI13nEvent,
getI13nNode: this.getI13nNode,
parentI13nNode: this._i13nNode
parentI13nNode: this._i13nNode,
_reactI13nInstance: this._getReactI13n()
}
};
},
Expand All @@ -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();
}
},
Expand All @@ -63,9 +80,6 @@ var I13nUtils = {
* @return {Object} i13n node
*/
getI13nNode: function () {
if (!this._getReactI13n()) {
return;
}
return this._i13nNode || this._getParentI13nNode();
},

Expand All @@ -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;
},

/**
Expand All @@ -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());
}
}

Expand Down
20 changes: 13 additions & 7 deletions src/utils/setupI13n.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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],
Expand All @@ -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(
Expand Down
1 change: 0 additions & 1 deletion tests/unit/libs/ReactI13n.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});

Expand Down
49 changes: 24 additions & 25 deletions tests/unit/utils/createI13nNode.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down Expand Up @@ -89,6 +87,7 @@ describe('createI13nNode', function () {
return mockData.isViewportEnabled;
}
};
global.window._reactI13nInstance = mockData.reactI13n;

done();
});
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -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});
});
});

0 comments on commit 9e5392f

Please sign in to comment.