chore(localize): loadingComplete localizeNamespacesLoaded timing issue

thescientist13 · Jul 21, 2021 · db1ad6d · db1ad6d
1 parent f843278
commit db1ad6d
Show file tree

Hide file tree

Showing 5 changed files with 36 additions and 32 deletions.
diff --git a/packages/calendar/test/lion-calendar.test.js b/packages/calendar/test/lion-calendar.test.js
@@ -1393,7 +1393,7 @@ describe('<lion-calendar>', () => {
       ]);
 
       localize.locale = 'nl-NL';
-      await localize.loadingComplete;
+      await el.localizeNamespacesLoaded;
       await el.updateComplete;
       expect(elObj.nextMonthButtonEl?.getAttribute('aria-label')).to.equal(
         'Volgende maand, december 2019',

diff --git a/packages/localize/src/LocalizeManager.js b/packages/localize/src/LocalizeManager.js
@@ -490,6 +490,8 @@ export class LocalizeManager {
    * @protected
    */
   _onLocaleChanged(newLocale, oldLocale) {
+    // Event firing immediately, does not wait for loading the translations
+    this.dispatchEvent(new CustomEvent('__localeChanging'));
     if (newLocale === oldLocale) {
       return;
     }

diff --git a/packages/localize/src/LocalizeMixin.js b/packages/localize/src/LocalizeMixin.js
@@ -11,7 +11,6 @@ import { localize } from './localize.js';
  * @type {LocalizeMixin}
  */
 const LocalizeMixinImplementation = superclass =>
-  // eslint-disable-next-line
   class LocalizeMixin extends superclass {
     /**
      * @returns {Object.<string,function>[]}
@@ -38,6 +37,12 @@ const LocalizeMixinImplementation = superclass =>
           this.__localizeOnLocaleChanged(event);
         };
 
+      this.__boundLocalizeOnLocaleChanging =
+        /** @param {...Object} args */
+        () => {
+          this.__localizeOnLocaleChanging();
+        };
+
       // should be loaded in advance
       /** @private */
       this.__localizeStartLoadingNamespaces();
@@ -62,27 +67,23 @@ const LocalizeMixinImplementation = superclass =>
     }
 
     connectedCallback() {
-      if (super.connectedCallback) {
-        super.connectedCallback();
-      }
-
+      super.connectedCallback();
       if (this.localizeNamespacesLoaded) {
         this.localizeNamespacesLoaded.then(() => this.onLocaleReady());
       }
-      this.__localizeAddLocaleChangedListener();
+      localize.addEventListener('__localeChanging', this.__boundLocalizeOnLocaleChanging);
+      localize.addEventListener('localeChanged', this.__boundLocalizeOnLocaleChanged);
     }
 
     disconnectedCallback() {
-      if (super.disconnectedCallback) {
-        super.disconnectedCallback();
-      }
-
-      this.__localizeRemoveLocaleChangedListener();
+      super.disconnectedCallback();
+      localize.removeEventListener('__localeChanging', this.__boundLocalizeOnLocaleChanging);
+      localize.removeEventListener('localeChanged', this.__boundLocalizeOnLocaleChanged);
     }
 
     /**
      * @param {string | string[]} keys
-     * @param {Object.<string,?>} variables
+     * @param {Object.<string,?>} [variables]
      * @param {Object} [options]
      * @param {string} [options.locale]
      * @returns {string | DirectiveResult}
@@ -124,14 +125,13 @@ const LocalizeMixinImplementation = superclass =>
       this.localizeNamespacesLoaded = localize.loadNamespaces(this.__getUniqueNamespaces());
     }
 
-    /** @private */
-    __localizeAddLocaleChangedListener() {
-      localize.addEventListener('localeChanged', this.__boundLocalizeOnLocaleChanged);
-    }
-
-    /** @private */
-    __localizeRemoveLocaleChangedListener() {
-      localize.removeEventListener('localeChanged', this.__boundLocalizeOnLocaleChanged);
+    /**
+     * Start loading namespaces on the event that is sent immediately
+     * when localize.locale changes --> 'localeChanging'
+     * @private
+     */
+    __localizeOnLocaleChanging() {
+      this.__localizeStartLoadingNamespaces();
     }
 
     /**
@@ -152,7 +152,6 @@ const LocalizeMixinImplementation = superclass =>
      */
     // eslint-disable-next-line no-unused-vars
     onLocaleChanged(newLocale, oldLocale) {
-      this.__localizeStartLoadingNamespaces();
       this.onLocaleUpdated();
       this.requestUpdate();
     }

diff --git a/packages/localize/test/LocalizeMixin.test.js b/packages/localize/test/LocalizeMixin.test.js
@@ -146,11 +146,13 @@ describe('LocalizeMixin', () => {
     await el.localizeNamespacesLoaded;
     expect(onLocaleChangedSpy.callCount).to.equal(0);
 
+    // Appending to DOM will result in onLocaleChanged to be invoked
     wrapper.appendChild(el);
 
+    // Changing locale will result in onLocaleChanged to be invoked
     localize.locale = 'ru-RU';
     await el.localizeNamespacesLoaded;
-    expect(onLocaleChangedSpy.callCount).to.equal(1);
+    expect(onLocaleChangedSpy.callCount).to.equal(2);
     expect(onLocaleChangedSpy.calledWithExactly('ru-RU', 'nl-NL')).to.be.true;
   });
 
@@ -189,12 +191,10 @@ describe('LocalizeMixin', () => {
 
     localize.locale = 'nl-NL';
     await el.localizeNamespacesLoaded;
-    await nextFrame();
     expect(el.foo).to.equal('bar-nl-NL');
 
     localize.locale = 'ru-RU';
     await el.localizeNamespacesLoaded;
-    await nextFrame();
     expect(el.foo).to.equal('bar-ru-RU');
   });
 
@@ -226,7 +226,6 @@ describe('LocalizeMixin', () => {
 
     localize.locale = 'nl-NL';
     await el.localizeNamespacesLoaded;
-    await nextFrame();
     expect(onLocaleUpdatedSpy.callCount).to.equal(2);
   });
 
@@ -266,7 +265,6 @@ describe('LocalizeMixin', () => {
 
     localize.locale = 'nl-NL';
     await el.localizeNamespacesLoaded;
-    await nextFrame();
     expect(el.label).to.equal('two');
   });
 
@@ -292,6 +290,8 @@ describe('LocalizeMixin', () => {
 
     localize.locale = 'nl-NL';
     await el.localizeNamespacesLoaded;
+
+    // await next frame for requestUpdate to be fired
     await nextFrame();
     expect(updateSpy.callCount).to.equal(1);
   });
@@ -436,7 +436,7 @@ describe('LocalizeMixin', () => {
       localize.locale = 'en-US';
       expect(p.innerText).to.equal('Hi!');
       await el.localizeNamespacesLoaded;
-      await aTimeout(25); // needed because msgLit relies on until directive
+      await nextFrame(); // needed because msgLit relies on until directive to re-render
       await el.updateComplete;
       expect(p.innerText).to.equal('Howdy!');
     }

diff --git a/packages/localize/types/LocalizeMixinTypes.d.ts b/packages/localize/types/LocalizeMixinTypes.d.ts
@@ -60,7 +60,7 @@ declare class LocalizeMixinHost {
 
   static get waitForLocalizeNamespaces(): boolean;
 
-  public localizeNamespacesLoaded(): Promise<Object>;
+  public localizeNamespacesLoaded: Promise<Object> | undefined;
 
   /**
    * Hook into LitElement to only render once all translations are loaded
@@ -74,10 +74,13 @@ declare class LocalizeMixinHost {
   public disconnectedCallback(): void;
   public msgLit(keys: string | string[], variables?: msgVariables, options?: msgOptions): void;
 
-  private __getUniqueNamespaces(): void;
-  private __localizeAddLocaleChangedListener(): void;
-  private __localizeRemoveLocaleChangedListener(): void;
+  private __boundLocalizeOnLocaleChanged(...args: Object[]): void;
+  private __boundLocalizeOnLocaleChanging(...args: Object[]): void;
+  private __getUniqueNamespaces(): string[];
   private __localizeOnLocaleChanged(event: CustomEvent): void;
+  private __localizeMessageSync: boolean;
+  private __localizeStartLoadingNamespaces(): void;
+  private __localizeOnLocaleChanging(): void;
 }
 
 declare function LocalizeMixinImplementation<T extends Constructor<LitElement>>(