From 349c8acfdc49e744eb08167e63098727afbb4c05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaros=C5=82aw=20Sadzi=C5=84ski?= Date: Fri, 9 Dec 2022 20:14:59 +0100 Subject: [PATCH 1/3] Ignoring reserved characters in a resource key in a scoped translation helper --- app/client/lib/localization.ts | 39 +++++++++++++++++++++++++++------ test/client/lib/localization.ts | 22 ++++++++++++------- 2 files changed, 46 insertions(+), 15 deletions(-) diff --git a/app/client/lib/localization.ts b/app/client/lib/localization.ts index a628a361..8fe6c155 100644 --- a/app/client/lib/localization.ts +++ b/app/client/lib/localization.ts @@ -88,15 +88,20 @@ type InferResult = T extends Record|undefi /** * Resolves the translation of the given key and substitutes. Supports dom elements interpolation. */ - export function t>(key: string, args?: T|null, instance = i18next): InferResult { +export function t>(key: string, args?: T|null, instance = i18next): InferResult { 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 extends Record|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>(key: string, args?: T|null, instance = i18next) { - return t(`${scope}.${key}`, args, instance); +export function makeT(scope: string, instance?: typeof i18next) { + // Can 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>(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!); } } diff --git a/test/client/lib/localization.ts b/test/client/lib/localization.ts index 0e64f08f..f66fc09f 100644 --- a/test/client/lib/localization.ts +++ b/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'); }); }); From 07f5f86620dfabf0093a9f388bb2bff3af323c43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaros=C5=82aw=20Sadzi=C5=84ski?= Date: Fri, 9 Dec 2022 20:15:08 +0100 Subject: [PATCH 2/3] Fixing flaky tests. --- test/nbrowser/ActionLog.ts | 7 +++++++ test/nbrowser/CustomView.ts | 2 ++ test/nbrowser/CustomWidgets.ts | 4 +++- 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/test/nbrowser/ActionLog.ts b/test/nbrowser/ActionLog.ts index 49459ea4..cb37e20c 100644 --- a/test/nbrowser/ActionLog.ts +++ b/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(); diff --git a/test/nbrowser/CustomView.ts b/test/nbrowser/CustomView.ts index 0c8dfe9e..f9fa7dac 100644 --- a/test/nbrowser/CustomView.ts +++ b/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 diff --git a/test/nbrowser/CustomWidgets.ts b/test/nbrowser/CustomWidgets.ts index 23bfd084..6563a2de 100644 --- a/test/nbrowser/CustomWidgets.ts +++ b/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. From 10ac424514b44df120fdeb225d5a4b3dc751fe0f Mon Sep 17 00:00:00 2001 From: jarek Date: Mon, 12 Dec 2022 11:29:16 +0100 Subject: [PATCH 3/3] Update app/client/lib/localization.ts Co-authored-by: George Gevoian <85144792+georgegevoian@users.noreply.github.com> --- app/client/lib/localization.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/client/lib/localization.ts b/app/client/lib/localization.ts index 8fe6c155..ef28a049 100644 --- a/app/client/lib/localization.ts +++ b/app/client/lib/localization.ts @@ -150,7 +150,7 @@ function isLikeDomContents(value: any): boolean { * namespace and a key prefix (a scope). */ export function makeT(scope: string, instance?: typeof i18next) { - // Can create the scopedInstance yet as it might not be initialized. + // 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>(key: string, args?: T|null) {