martinandert / react-translate-component

A component for React that utilizes the Counterpart module to provide multi-lingual/localized text content.
MIT License
322 stars 31 forks source link

Possible EventEmitter memory leak detected #2

Closed andresgutgon closed 10 years ago

andresgutgon commented 10 years ago

Hi, first of all thank you for this component. I appreciate your work.

I'm trying it and it works. but I'm getting this in Chrome console.

image

In the image you can read this message:

(node) warning: possible EventEmitter memory leak detected. 11 listeners added. Use emitter.setMaxListeners() to increase limit. 

It´s a warning. But is anoying :(

What i'm doing

I've moved all my Ì18n` to a file this way:

'use strict';

var Translate = require('react-translate-component')
  , counterpart = require('counterpart')
  , I18nLib = {};

/**
 * initialize translations
 */
I18nLib.initialize = function () {
  this.registerTranslations('en', require('../../locales/en.js'));
  this.registerTranslations('en-US', require('../../locales/en'));
  this.registerTranslations('en-GB', require('../../locales/en'));
  this.registerTranslations('en-BB', require('../../locales/en'));
  this.registerTranslations('es', require('../../locales/es'));
  this.registerTranslations('es-ES', require('../../locales/es'));
  this.registerTranslations('es-AR', require('../../locales/es'));
  this.registerTranslations('ca', require('../../locales/ca'));
};

/**
 * this is a counterpart-style convenience function that
 * returns a React component
 *
 * @param {String} key
 * @return {Object}
 */
I18nLib.t = function (key) {
  return Translate.translate(key);
};

/**
 * This generate a React Component for translations
 * Ex.: <Translate component={React.DOM.option}>this.is.a.key</Translate>
 *
 * @return {Object}
 */
I18nLib.Translate = Translate;

/**
 * Original translation library.
 * Helpful when you want a plain string and not a React component
 *
 * @return {string}
 */
I18nLib.counterpart = counterpart;

/**
 * Set current locale. It is a counterpart method
 *
 * @return {string}
 */
I18nLib.getLocale = I18nLib.counterpart.getLocale;

/**
 * Set current locale. It is a counterpart method
 *
 * @param {string} locale
 */
I18nLib.setLocale = function (locale) {
  I18nLib.counterpart.setLocale(locale);
};

/**
 * Register translations.
 * Data is a file with translations
 *
 * @param {string} locale
 * @param {string} data
 */
I18nLib.registerTranslations = function (locale, data) {
  I18nLib.counterpart.registerTranslations(locale, data);
};

module.exports = I18nLib;

And then I use this way:

<div className="col-lg-12">
  {<Translate>my.welcome.signin.description</Translate>}
</div>
...
<p>{I18n.t('my.welcome.services.s_3')}</p>

Not a solution

If I remove some of the translations I generate less than 10 event binding and the warning disappear. But that is not a real solution :)

martinandert commented 10 years ago

Each Translate component will add an event listener for Counterpart's localechange event. If you are using more than 10 components on a page, Node will show the warning you described. There is no memory leak or whatsoever.

I will set the maximum allowed listeners of Counterpart's event emitter to unlimited. This should fix your problem. I will get back to you when this is resolved.

martinandert commented 10 years ago

I've tagged a new release (v0.4.2) which should resolve this. Please try it out and re-open this issue if necessary.

andresgutgon commented 10 years ago

Yes! That solved my problem. Thanks.

There is no memory leak or whatsoever.

Can you explain why that is not a problem and we ignore the warning? Thanks again

martinandert commented 10 years ago

Can you explain why that is not a problem and we ignore the warning?

Node.js thinks that 10+ listeners for a single event seems odd and so outputs a warning. But in our case (there usually are more than 10 Translate components on a web page which register an event listener) we just tell Node.js to shut up and go on.

http://nodejs.org/api/events.html#events_emitter_setmaxlisteners_n

andresgutgon commented 10 years ago

Thanks @martinandert