ember-validations icon indicating copy to clipboard operation
ember-validations copied to clipboard

destructure Ember.I18n each time render is called

Open iezer opened this issue 9 years ago • 7 comments

Fixes using ember-i18n.

Otherwise the workaround described in https://github.com/DockYard/ember-validations/issues/366#issuecomment-169869004 no longer works. When running multiple tests, ember-validations will use a destroyed version of i18n from the 1st test run, leaking memory in the process. CC @jamesarosen

iezer avatar Dec 01 '16 18:12 iezer

:+1:

bcardarella avatar Dec 01 '16 18:12 bcardarella

Actually, let me back up. I might be following this incorrectly. So Ember.i18n gets recreated each time?

It might make more sense to inject the service instead rather than destructure from the const

bcardarella avatar Dec 01 '16 18:12 bcardarella

Yes injecting the service would be better. We have a hack in an instance-initializer to set Ember.I18n = application.container.lookup('service:i18n')

iezer avatar Dec 01 '16 18:12 iezer

In older versions of ember-i18n, there was a fixed global, window.Ember.I18n. That's the interface that this library built around.

Later, ember-i18n switched to service:i18n as the core entry point.

@iezer referenced a comment I made a while back on how to fake the old interface from the new (to support this library with newer ember-i18n), but that relies on setting up and tearing down window.Ember.I18n for each test. (If you don't do that, then you leak references to the container into the global namespace, which will quickly cause your test suite to run out of memory.)

jamesarosen avatar Dec 01 '16 18:12 jamesarosen

It would be lovely if this library were to rely on service:i18n, but that means here can't be any static code that requires internationalization.

For example, addon/messages.js will have to become a service:

// ember-validations/app/services/validation-messages.js
import Ember from 'ember';
import getOwner from 'ember-getowner-polyfill';

export default Ember.Service.extend({
  // i18n: Ember.inject.service(), // can't do this because service:i18n might not exist

  render(attribute, context) {
    const i18n = getOwner(this).lookup('service:i18n');
    if (i18n) { return i18n.t(`errors.${attribute}`, context);
    ...
  }
});

That, in turn, means that the various validators have to change to get the service:validation-messages:

// ember-validations/addon/validators/local/numericality.js
import Ember from 'ember';
export default Base.extend({
  validationMessages: Ember.inject.service(),

  init() {
    ...
    this.options.messages.numericality = this.get('validationMessages').render('notANumber', this.options);
  }
});

jamesarosen avatar Dec 01 '16 18:12 jamesarosen

It would be lovely if this library were to rely on service:i18n

confirm, I have been planing a rewrite for 2.0. I may reclassify this as a defunct 2.x attempt (we never left alpha for 2.x) and that would be a better place for hooking into i18n correctly. Odds are I would just leave it out of this library and up to the consumer to enable.

bcardarella avatar Dec 01 '16 20:12 bcardarella

How do you feel about shipping this in the meantime?

jamesarosen avatar Dec 01 '16 20:12 jamesarosen