Merge pull request #369 from gristlabs/proper-scoped-t

Fixing scoped translation helper - it should ignore namespace and key separators.

app/client/lib/localization.ts

@@ -88,15 +88,20 @@ type InferResult<T> = T extends Record<string, string | number | boolean>|undefi
 /**
  * Resolves the translation of the given key and substitutes. Supports dom elements interpolation.
  */
- export function t<T extends Record<string, any>>(key: string, args?: T|null, instance = i18next): InferResult<T> {
+export function t<T extends Record<string, any>>(key: string, args?: T|null, instance = i18next): InferResult<T> {
   if (!instance.exists(key, args || undefined)) {
     const error = new Error(`Missing translation for key: ${key} and language: ${i18next.language}`);
     reportError(error);
   }
+  // Don't need to bind `t` function.
+  return domT(key, args, instance.t);
+}
+
+function domT(key: string, args: any, tImpl: typeof i18next.t) {
   // If there are any DomElements in args, handle it with missingInterpolationHandler.
   const domElements = !args ? [] : Object.entries(args).filter(([_, value]) => isLikeDomContents(value));
   if (!args || !domElements.length) {
-    return instance.t(key, args || undefined) as any;
+    return tImpl(key, args || undefined) as any;
   } else {
     // Make a copy of the arguments, and remove any dom elements from it. It will instruct
     // i18next library to use `missingInterpolationHandler` handler.
@@ -105,7 +110,7 @@ type InferResult<T> = T extends Record<string, string | number | boolean>|undefi
 
     // Passing `missingInterpolationHandler` will allow as to resolve all missing keys
     // and replace them with a marker.
-    const result: string = instance.t(key, {...copy, missingInterpolationHandler});
+    const result: string = tImpl(key, {...copy, missingInterpolationHandler});
 
     // Now replace all markers with dom elements passed as arguments.
     const parts = result.split(/(\[\[\[[^\]]+?\]\]\])/);
@@ -141,10 +146,30 @@ function isLikeDomContents(value: any): boolean {
 }
 
 /**
- * Helper function to create scoped t function.
+ * Helper function to create a scoped t function. Scoped t function is bounded to a specific
+ * namespace and a key prefix (a scope).
  */
-export function makeT(scope: string) {
-  return function<T extends Record<string, any>>(key: string, args?: T|null, instance = i18next) {
-    return t(`${scope}.${key}`, args, instance);
+export function makeT(scope: string, instance?: typeof i18next) {
+  // Can't create the scopedInstance yet as it might not be initialized.
+  let scopedInstance: null|typeof i18next = null;
+  let scopedResolver: null|typeof i18next.t = null;
+  return function<T extends Record<string, any>>(key: string, args?: T|null) {
+    // Create a scoped instance with disabled namespace and nested features.
+    // This enables keys like `key1.key2:key3` to be resolved properly.
+    if (!scopedInstance) {
+      scopedInstance = (instance ?? i18next).cloneInstance({
+        keySeparator: false,
+        nsSeparator: false,
+      });
+      // Create a version of `t` function that will use the provided prefix as default.
+      scopedResolver = scopedInstance.getFixedT(null, null, scope);
+    }
+    // If the key has interpolation or we did pass some arguments, make sure that
+    // the key exists.
+    if ((args || key.includes("{{")) && !scopedInstance.exists(`${scope}.${key}`, args || undefined)) {
+      const error = new Error(`Missing translation for key: ${key} and language: ${i18next.language}`);
+      reportError(error);
+    }
+    return domT(key, args, scopedResolver!);
   }
 }

test/client/lib/localization.ts

@@ -19,6 +19,7 @@ describe('localization', function() {
             'Argument_variant': 'Variant {{arg1}} {{arg2}}{{end}}',
             'Parent': {
               'Child': 'Translated child {{arg}}',
+              'Not.Valid:Characters': 'Works',
             }
           }
         }
@@ -93,8 +94,8 @@ describe('localization', function() {
   });
 
   it('supports scoping through makeT', function() {
-    const scoped = makeT('Parent');
-    assert.equal(scoped('Child', { arg : 'Arg'}, instance), 'Translated child Arg');
+    const scoped = makeT('Parent', instance);
+    assert.equal(scoped('Child', { arg : 'Arg'}), 'Translated child Arg');
   });
 
   it('infers result from parameters', function() {
@@ -108,17 +109,22 @@ describe('localization', function() {
     typeString = t('Argument', {arg1: 'argument 1', arg2: 'argument 2'}, instance);
     typeString = t('Argument', {arg1: 1, arg2: true}, instance);
     typeString = t('Argument', undefined,  instance);
-    const scoped = makeT('Parent');
-    typeString = scoped('Child', {arg: 'argument 1'}, instance);
-    typeString = scoped('Child', {arg: 1}, instance);
-    typeString = scoped('Child', undefined, instance);
+    const scoped = makeT('Parent', instance);
+    typeString = scoped('Child', {arg: 'argument 1'});
+    typeString = scoped('Child', {arg: 1});
+    typeString = scoped('Child', undefined);
 
     let domContent: DomContents = null; void domContent;
 
     domContent = t('Argument', {arg1: 'argument 1', arg2: dom('span')}, instance);
     domContent = t('Argument', {arg1: 1, arg2: dom.domComputed(observable('test'))}, instance);
     domContent = t('Argument', undefined, instance);
-    domContent = scoped('Child', {arg: dom.create(Component)}, instance);
-    domContent = scoped('Child', {arg: dom.maybe(observable(true), () => dom('span'))}, instance);
+    domContent = scoped('Child', {arg: dom.create(Component)});
+    domContent = scoped('Child', {arg: dom.maybe(observable(true), () => dom('span'))});
+  });
+
+  it('supports : and . characters in scoped function', function() {
+    const scoped = makeT('Parent', instance);
+    assert.equal(scoped('Not.Valid:Characters'), 'Works');
   });
 });

test/nbrowser/ActionLog.ts

@@ -29,6 +29,13 @@ describe('ActionLog', function() {
     await gu.dismissWelcomeTourIfNeeded();
   });
 
+  after(async function() {
+    // If were are debugging the browser won't be reloaded, so we need to close the right panel.
+    if (process.env.NO_CLEANUP) {
+      await driver.find(".test-right-tool-close").click();
+    }
+  });
+
   it("should cross out undone actions", async function() {
     // Open the action-log tab.
     await driver.findWait('.test-tools-log', 1000).click();

test/nbrowser/CustomView.ts

@@ -268,6 +268,8 @@ describe('CustomView', function() {
     const mainSession = await gu.session().teamSite.login();
     await mainSession.tempDoc(cleanup, 'TypeEncoding.grist');
     await gu.toggleSidePanel('right', 'open');
+    await driver.find('.test-right-tab-pagewidget').click();
+    await gu.waitForServer();
     await driver.find('.test-config-data').click();
 
     // The test doc already has a Custom View widget. It just needs to

test/nbrowser/CustomWidgets.ts

@@ -188,7 +188,9 @@ describe('CustomWidgets', function () {
   const reject = () => driver.find(".test-config-widget-access-reject").click();
 
   it('should show widgets in dropdown', async () => {
-    await gu.toggleSidePanel('right');
+    await gu.toggleSidePanel('right', 'open');
+    await driver.find('.test-right-tab-pagewidget').click();
+    await gu.waitForServer();
     await driver.find('.test-config-widget').click();
     await gu.waitForServer(); // Wait for widgets to load.