fix: update localization lookup (#34)

* refactor: return the key of unknown resources instead of an empty string * refactor!: update localizations to traverse from the "Localization" node, instead of the root * docs: localization lookup * fix: constructor initialization of i18n * style: fix linting --------- Co-authored-by: Richard Herman <[email protected]>
elgatosf · Apr 15, 2024 · 4daecce · 4daecce
1 parent 08f1d7b
commit 4daecce
Show file tree

Hide file tree

Showing 5 changed files with 65 additions and 83 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -26,13 +26,15 @@
 
 -   `Coordinates` type could erroneously have a non-number type for `row`.
 -   Fix support for allowed types within payloads.
+-   Fix localization lookup.
 
 ### ♻️ Update
 
 -   Update layout and manifest references to propagate from [`@elgato/schemas`](https://github.com/elgatosf/schemas).
 
 ### ➡️ Migration
 
+-   Localizations keys are now indexed from the `Localization` node within the translation file.
 -   `PayloadObject<T>` replaced with `JsonObject`.
 -   JSON schemas have been relocated to a dedicated schemas package, [`@elgato/schemas`](https://github.com/elgatosf/schemas).
 

diff --git a/src/plugin/__tests__/i18n.test.ts b/src/plugin/__tests__/i18n.test.ts
@@ -1,6 +1,5 @@
 import fs, { Dirent } from "node:fs";
 import path from "node:path";
-import type { Manifest } from "../../api";
 import { I18nProvider } from "../i18n";
 import { logger } from "../logging";
 import { LogLevel } from "../logging/log-level";
@@ -13,23 +12,20 @@ describe("I18nProvider", () => {
 	 * Defines a set of mock resources.
 	 */
 	type MockTranslations = {
-		greeting?: string;
-		manifestOnly?: string;
-		englishOnly?: string;
-		frenchOnly?: string;
-		germanOnly?: string;
+		Localization: {
+			greeting?: string;
+			manifestOnly?: string;
+			englishOnly?: string;
+			frenchOnly?: string;
+			germanOnly?: string;
+		};
 	};
 
 	const mockedCwd = "c:\\temp";
 	const mockedResources = new Map<string, MockTranslations>();
-	mockedResources.set("de.json", { greeting: "Hello welt", germanOnly: "German" });
-	mockedResources.set("en.json", { greeting: "Hello world", englishOnly: "English" });
-	mockedResources.set("fr.json", { greeting: "Bonjour le monde", frenchOnly: "French" });
-
-	const mockedManifest = {
-		greeting: "This should never be used",
-		manifestOnly: "Manifest"
-	} as unknown as Manifest;
+	mockedResources.set("de.json", { Localization: { greeting: "Hello welt", germanOnly: "German" } });
+	mockedResources.set("en.json", { Localization: { greeting: "Hello world", englishOnly: "English" } });
+	mockedResources.set("fr.json", { Localization: { greeting: "Bonjour le monde", frenchOnly: "French" } });
 
 	let scopedLogger!: Logger;
 
@@ -43,6 +39,8 @@ describe("I18nProvider", () => {
 		jest.spyOn(process, "cwd").mockReturnValue(mockedCwd);
 	});
 
+	afterEach(() => jest.resetAllMocks());
+
 	/**
 	 * Asserts {@link I18nProvider} uses a scoped {@link Logger}.
 	 */
@@ -54,7 +52,7 @@ describe("I18nProvider", () => {
 		const createScopeSpy = jest.spyOn(logger, "createScope");
 
 		// Act.
-		new I18nProvider("en", mockedManifest, logger);
+		new I18nProvider("en", logger);
 
 		// Assert.
 		expect(createScopeSpy).toHaveBeenCalledTimes(1);
@@ -70,7 +68,8 @@ describe("I18nProvider", () => {
 		const readFileSyncSpy = jest.spyOn(fs, "readFileSync").mockImplementation(() => "{}");
 
 		// Act.
-		new I18nProvider("en", mockedManifest, logger);
+		const i18n = new I18nProvider("en", logger);
+		i18n.translate("test");
 
 		// Assert.
 		expect(readFileSyncSpy).toHaveBeenCalledTimes(6);
@@ -86,25 +85,6 @@ describe("I18nProvider", () => {
 		expect(readFileSyncSpy).not.toHaveBeenCalledWith(path.join(process.cwd(), "manifest.json"), opts);
 	});
 
-	/**
-	 * Asserts {@link I18nProvider} merges the manifest (resources) into the custom English resources.
-	 */
-	it("merges manifest into English", () => {
-		// Arrange.
-		jest.spyOn(fs, "readdirSync").mockReturnValue(["de.json", "en.json", "fr.json"] as unknown[] as Dirent[]);
-		jest.spyOn(fs, "readFileSync").mockImplementation((path) => JSON.stringify(mockedResources.get(path as string)));
-
-		const i18n = new I18nProvider("en", mockedManifest, logger);
-
-		// Act.
-		const greeting = i18n.translate("greeting");
-		const manifestOnly = i18n.translate("manifestOnly");
-
-		// Assert.
-		expect(greeting).toBe("Hello world");
-		expect(manifestOnly).toBe("Manifest");
-	});
-
 	/**
 	 * Asserts {@link I18nProvider} correctly resorts to default language.
 	 */
@@ -113,57 +93,55 @@ describe("I18nProvider", () => {
 		jest.spyOn(fs, "readdirSync").mockReturnValue(["de.json", "en.json", "fr.json"] as unknown[] as Dirent[]);
 		jest.spyOn(fs, "readFileSync").mockImplementation((path) => JSON.stringify(mockedResources.get(path as string)));
 
-		const i18n = new I18nProvider("de", mockedManifest, logger);
+		const i18n = new I18nProvider("de", logger);
 
 		// Act.
 		const greeting = i18n.translate("greeting");
 		const englishOnly = i18n.translate("englishOnly");
-		const manifestOnly = i18n.translate("manifestOnly");
 
 		// Assert.
 		expect(greeting).toBe("Hello welt");
 		expect(englishOnly).toBe("English");
-		expect(manifestOnly).toBe("Manifest");
 	});
 
 	/**
-	 * Asserts {@link I18nProvider} returns an empty string when the resource could not be found in either the current resource, or the default.
+	 * Asserts {@link I18nProvider} returns the key when the resource could not be found in either the current resource, or the default.
 	 */
-	it("returns empty string for unknown key (logMissingKey: true)", () => {
+	it("returns the key for an unknown resource (logMissingKey: true)", () => {
 		// Arrange.
 		jest.spyOn(fs, "readdirSync").mockReturnValue([]);
 		jest.spyOn(fs, "readFileSync").mockReturnValue("{}");
 		const spyOnWarn = jest.spyOn(scopedLogger, "warn");
 
-		const i18n = new I18nProvider("en", mockedManifest, logger);
+		const i18n = new I18nProvider("en", logger);
 
 		// Act.
 		i18n.logMissingKey = true;
 		const result = i18n.translate("hello");
 
 		// Assert.
-		expect(result).toBe("");
+		expect(result).toBe("hello");
 		expect(spyOnWarn).toHaveBeenCalledTimes(1);
 		expect(spyOnWarn).toHaveBeenCalledWith("Missing translation: hello");
 	});
 
 	/**
-	 * Asserts {@link I18nProvider} returns an empty string when the resource could not be found in either the current resource, or the default.
+	 * Asserts {@link I18nProvider} returns the key when the resource could not be found in either the current resource, or the default.
 	 */
-	it("returns empty string for unknown key (logMissingKey: false)", () => {
+	it("returns the key for an unknown resource (logMissingKey: false)", () => {
 		// Arrange.
 		jest.spyOn(fs, "readdirSync").mockReturnValue([]);
 		jest.spyOn(fs, "readFileSync").mockReturnValue("{}");
 		const spyOnWarn = jest.spyOn(scopedLogger, "warn");
 
-		const i18n = new I18nProvider("en", mockedManifest, logger);
+		const i18n = new I18nProvider("en", logger);
 
 		// Act.
 		i18n.logMissingKey = false;
 		const result = i18n.translate("hello");
 
 		// Assert.
-		expect(result).toBe("");
+		expect(result).toBe("hello");
 		expect(spyOnWarn).toHaveBeenCalledTimes(0);
 	});
 
@@ -177,7 +155,8 @@ describe("I18nProvider", () => {
 		const spyOnError = jest.spyOn(scopedLogger, "error");
 
 		// Act.
-		new I18nProvider("en", mockedManifest, logger);
+		const i18n = new I18nProvider("en", logger);
+		i18n.translate("test");
 
 		// Assert.
 		expect(spyOnError).toHaveBeenCalledTimes(1);
@@ -192,13 +171,15 @@ describe("I18nProvider", () => {
 		jest.spyOn(fs, "readdirSync").mockReturnValue(["en.json"] as unknown[] as Dirent[]);
 		jest.spyOn(fs, "readFileSync").mockReturnValue(
 			JSON.stringify({
-				parent: {
-					child: "Hello world"
+				Localization: {
+					parent: {
+						child: "Hello world"
+					}
 				}
 			})
 		);
 
-		const i18n = new I18nProvider("en", mockedManifest, logger);
+		const i18n = new I18nProvider("en", logger);
 
 		// Act.
 		const result = i18n.translate("parent.child");

diff --git a/src/plugin/i18n.ts b/src/plugin/i18n.ts
@@ -1,7 +1,8 @@
 import file from "node:fs";
 import path from "node:path";
 
-import { supportedLanguages, type Language, type Manifest } from "../api";
+import { supportedLanguages, type Language } from "../api";
+import { JsonObject } from "../common/json";
 import { get } from "./common/utils";
 import { Logger } from "./logging";
 
@@ -20,9 +21,9 @@ export class I18nProvider {
 	private static readonly DEFAULT_LANGUAGE: Language = "en";
 
 	/**
-	 * Collection of loaded locales and their translations.
+	 * Private backing field for {@link I18nProvider.locales}.
 	 */
-	private readonly locales = new Map<Language, unknown>();
+	private _locales: Map<Language, JsonObject> | undefined;
 
 	/**
 	 * Logger scoped to this class.
@@ -32,64 +33,62 @@ export class I18nProvider {
 	/**
 	 * Initializes a new instance of the {@link I18nProvider} class.
 	 * @param language The default language to be used when retrieving translations for a given key.
-	 * @param manifest Manifest that accompanies the plugin.
 	 * @param logger Logger responsible for capturing log entries.
 	 */
 	constructor(
 		private readonly language: Language,
-		manifest: Manifest,
 		logger: Logger
 	) {
 		this.logger = logger.createScope("I18nProvider");
-		this.loadLocales(manifest);
 	}
 
 	/**
-	 * Gets the translation for the specified {@link key} from the resources defined for {@link language}. When the key is not found, the default language is checked.
-	 * @param key Key that represents the translation.
-	 * @param language Optional language to get the translation for; otherwise the default language.
-	 * @returns The translation; otherwise an empty string.
+	 * Collection of loaded locales and their translations.
+	 * @returns The locales that contains the translations.
 	 */
-	public translate(key: string, language: Language | undefined = this.language): string {
-		const translation = this.translateOrDefault(key, language);
-
-		if (translation === undefined && this.logMissingKey) {
-			this.logger.warn(`Missing translation: ${key}`);
+	private get locales(): Map<Language, JsonObject> {
+		if (this._locales !== undefined) {
+			return this._locales;
 		}
 
-		return translation || "";
-	}
-
-	/**
-	 * Loads all known locales from the current working directory.
-	 * @param manifest Manifest that accompanies the plugin.
-	 */
-	private loadLocales(manifest: Manifest): void {
+		const locales = new Map<Language, JsonObject>();
 		for (const filePath of file.readdirSync(process.cwd())) {
 			const { ext, name } = path.parse(filePath);
 			const lng = name as Language;
 
 			if (ext.toLowerCase() == ".json" && supportedLanguages.includes(lng)) {
 				const contents = this.readFile(filePath);
 				if (contents !== undefined) {
-					this.locales.set(lng, contents);
+					locales.set(lng, contents);
 				}
 			}
 		}
 
-		// Merge the manifest into the default language, prioritizing explicitly defined resources.
-		this.locales.set(I18nProvider.DEFAULT_LANGUAGE, {
-			...manifest,
-			...(this.locales.get(I18nProvider.DEFAULT_LANGUAGE) || {})
-		});
+		return (this._locales = locales);
+	}
+
+	/**
+	 * Gets the translation for the specified {@link key} from the resources defined for {@link language}. When the key is not found, the default language is checked.
+	 * @param key Key that represents the translation.
+	 * @param language Optional language to get the translation for; otherwise the default language.
+	 * @returns The translation; otherwise an empty string.
+	 */
+	public translate(key: string, language: Language | undefined = this.language): string {
+		const translation = this.translateOrDefault(key, language);
+
+		if (translation === undefined && this.logMissingKey) {
+			this.logger.warn(`Missing translation: ${key}`);
+		}
+
+		return translation || key;
 	}
 
 	/**
 	 * Reads the contents of the {@link filePath} and parses it as JSON.
 	 * @param filePath File path to read.
 	 * @returns Parsed object; otherwise `undefined`.
 	 */
-	private readFile(filePath: string): unknown | undefined {
+	private readFile(filePath: string): JsonObject | undefined {
 		try {
 			const contents = file.readFileSync(filePath, { flag: "r" })?.toString();
 			return JSON.parse(contents);
@@ -105,6 +104,8 @@ export class I18nProvider {
 	 * @returns The resource; otherwise the default language's resource, or `undefined`.
 	 */
 	private translateOrDefault(key: string, language: Language = this.language): string | undefined {
+		key = `Localization.${key}`;
+
 		// When the language and default are the same, only check the language.
 		if (language === I18nProvider.DEFAULT_LANGUAGE) {
 			return get(key, this.locales.get(language))?.toString();

diff --git a/src/plugin/index.ts b/src/plugin/index.ts
@@ -64,7 +64,7 @@ export const streamDeck = {
 	 * @returns Internalization provider.
 	 */
 	get i18n(): I18nProvider {
-		return (i18n ??= new I18nProvider(this.info.application.language, this.manifest, this.logger));
+		return (i18n ??= new I18nProvider(this.info.application.language, this.logger));
 	},
 
 	/**

diff --git a/src/plugin/ui/__tests__/route.test.ts b/src/plugin/ui/__tests__/route.test.ts
@@ -228,7 +228,6 @@ class ActionWithRoutes extends SingletonAction {
 	 */
 	@route("/characters")
 	public getCharacters(req: MessageRequest, res: MessageResponder): Promise<string[]> {
-		console.log("Called spy async");
 		this.spyOnGetCharacters(req, res);
 		return Promise.resolve(["Anduin", "Sylvanas", "Thrall"]);
 	}
@@ -241,7 +240,6 @@ class ActionWithRoutes extends SingletonAction {
 	 */
 	@route("/characters-sync")
 	public getCharactersSync(req: MessageRequest, res: MessageResponder): string[] {
-		console.log("Called spy sync");
 		this.spyOnGetCharactersSync(req, res);
 		return ["Mario", "Luigi", "Peach"];
 	}