CustomApps/lyrics-plus/ProviderMusixmatch.js (2)

240-242: LGTM with minor concurrency consideration.

The module-level cache and early return pattern work well for avoiding redundant API calls. However, if getLanguages() is called multiple times concurrently before languageMap is set, all calls will proceed to fetch from the API.

Optional: Prevent duplicate concurrent API calls

If you want to prevent duplicate API calls during concurrent invocations, consider caching the promise itself:

 let languageMap = null;
+let languageMapPromise = null;
 async function getLanguages() {
-	if (languageMap) return languageMap;
+	if (languageMap) return languageMap;
+	if (languageMapPromise) return languageMapPromise;
+	
+	languageMapPromise = (async () => {
+		// existing fetch logic...
+		return languageMap;
+	})();
+	
+	return languageMapPromise;

This is a low-priority optimization and not necessary for correctness.

---

269-287: API integration looks good with minor capitalization consideration.

The fetch logic, error handling, and dual ISO code mapping (lines 277-278) are well implemented. The function gracefully returns an empty object on failure, allowing the UI to fall back to alternative display methods.

One minor consideration: Line 276's capitalization logic (charAt(0).toUpperCase() + slice(1)) only capitalizes the first letter. This works well for single-word language names but may not properly title-case multi-word names (e.g., "united states" becomes "United states" instead of "United States"). However, if the Musixmatch API consistently returns single-word names or already properly formatted names, this is fine.

CustomApps/lyrics-plus/OptionsMenu.js (1)

102-106: Add error handling for the promise.

The defensive check for ProviderMusixmatch existence is good. However, the promise returned by getLanguages() lacks error handling. While the implementation returns an empty object on fetch failures, adding explicit error handling would make the code more robust and prevent potential unhandled promise rejections.

Suggested fix

 react.useEffect(() => {
 	if (typeof ProviderMusixmatch !== "undefined") {
-		ProviderMusixmatch.getLanguages().then(setLanguageMap);
+		ProviderMusixmatch.getLanguages()
+			.then(setLanguageMap)
+			.catch((error) => {
+				console.error("Failed to load language map", error);
+			});
 	}
 }, []);

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

CustomApps/lyrics-plus/ProviderMusixmatch.js (3)

271-282: Consider adding defensive validation in the forEach loop.

The code assumes each item has a language property and that the language object is well-formed. If the API returns malformed data, accessing item.language or its properties could throw. While the outer try-catch will handle it, more granular validation would make debugging easier and prevent partial cache corruption.

🔎 Suggested defensive validation

 languageMap = {};
 body.message.body.language_list.forEach((item) => {
 	const lang = item.language;
-	if (lang.language_name) {
+	if (lang && lang.language_name) {
 		const name = lang.language_name.charAt(0).toUpperCase() + lang.language_name.slice(1);
 		if (lang.language_iso_code_1) languageMap[lang.language_iso_code_1] = name;
 		if (lang.language_iso_code_3) languageMap[lang.language_iso_code_3] = name;
 	}
 });

---

271-289: Operational consideration: Failed fetches will retry on every call.

When the API fetch fails, the function returns an empty object but leaves languageMap as null. This means subsequent calls will retry the fetch, potentially hammering the API if it's experiencing issues. Consider caching the failure state temporarily to implement a backoff strategy, or accept the retry behavior as intentional for transient failures.

---

240-242: Consider returning a defensive copy instead of the cached reference.

The function returns a direct reference to the module-level languageMap object. While currently safe since the only consumer (OptionsMenu.js) treats it as read-only, returning a shallow copy (return { ...languageMap }) would prevent accidental mutations from corrupting the cache and is a more robust defensive programming pattern.

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro