From f31de6be7ca20e807cfaf07a0f945bf3c66f8fd9 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sat, 16 Sep 2023 12:34:18 +0200 Subject: [PATCH] core-plugin-api: make useTranslationRef handle and log errors Signed-off-by: Patrik Oldsberg --- .../translation/useTranslationRef.test.tsx | 89 +++++++++++++------ .../src/translation/useTranslationRef.ts | 23 ++++- 2 files changed, 81 insertions(+), 31 deletions(-) diff --git a/packages/core-plugin-api/src/translation/useTranslationRef.test.tsx b/packages/core-plugin-api/src/translation/useTranslationRef.test.tsx index 1d2682e2fb..7b04f6935e 100644 --- a/packages/core-plugin-api/src/translation/useTranslationRef.test.tsx +++ b/packages/core-plugin-api/src/translation/useTranslationRef.test.tsx @@ -14,8 +14,8 @@ * limitations under the License. */ -import React from 'react'; -import { TestApiProvider } from '@backstage/test-utils'; +import React, { ReactNode } from 'react'; +import { TestApiProvider, withLogCollector } from '@backstage/test-utils'; import { renderHook } from '@testing-library/react-hooks'; import { createTranslationRef } from './TranslationRef'; import { useTranslationRef } from './useTranslationRef'; @@ -23,7 +23,11 @@ import { useTranslationRef } from './useTranslationRef'; import { I18nextTranslationApi } from '../../../core-app-api/src/apis/implementations/TranslationApi'; // eslint-disable-next-line @backstage/no-relative-monorepo-imports import { AppLanguageSelector } from '../../..//core-app-api/src/apis/implementations/AppLanguageApi'; -import { createTranslationResource, translationApiRef } from '../alpha'; +import { + createTranslationResource, + TranslationApi, + translationApiRef, +} from '../alpha'; const plainRef = createTranslationRef({ id: 'plain', @@ -33,6 +37,15 @@ const plainRef = createTranslationRef({ }, }); +function makeWrapper(translationApi: TranslationApi) { + return ({ children }: { children: ReactNode }) => ( + + ); +} + describe('useTranslationRef', () => { beforeEach(() => { jest.clearAllMocks(); @@ -43,12 +56,7 @@ describe('useTranslationRef', () => { const translationApi = I18nextTranslationApi.create({ languageApi }); const { result } = renderHook(() => useTranslationRef(plainRef), { - wrapper: ({ children }) => ( - - ), + wrapper: makeWrapper(translationApi), }); const { t } = result.current; @@ -75,12 +83,7 @@ describe('useTranslationRef', () => { const { result, waitForNextUpdate } = renderHook( () => useTranslationRef(plainRef), { - wrapper: ({ children }) => ( - - ), + wrapper: makeWrapper(translationApi), }, ); @@ -112,12 +115,7 @@ describe('useTranslationRef', () => { const { result, waitForNextUpdate } = renderHook( () => useTranslationRef(plainRef), { - wrapper: ({ children }) => ( - - ), + wrapper: makeWrapper(translationApi), }, ); @@ -159,12 +157,7 @@ describe('useTranslationRef', () => { const { result, waitForNextUpdate } = renderHook( () => useTranslationRef(resourceRef), { - wrapper: ({ children }) => ( - - ), + wrapper: makeWrapper(translationApi), }, ); @@ -175,4 +168,46 @@ describe('useTranslationRef', () => { expect(t('key1')).toBe('de1'); expect(t('key2')).toBe('de2'); }); + + it('should log once and then ignore loading errors', async () => { + const ref = createTranslationRef({ + id: 'test', + messages: { + key: 'default', + }, + }); + + const languageApi = AppLanguageSelector.create(); + const translationApi = I18nextTranslationApi.create({ + languageApi, + resources: [ + createTranslationResource({ + ref, + translations: { + en: async () => { + throw new Error('NOPE'); + }, + }, + }), + ], + }); + + const rendered1 = renderHook(() => useTranslationRef(ref), { + wrapper: makeWrapper(translationApi), + }); + const rendered2 = renderHook(() => useTranslationRef(ref), { + wrapper: makeWrapper(translationApi), + }); + + const { error } = await withLogCollector(['error'], async () => { + await rendered2.waitForNextUpdate(); + }); + + expect(error).toEqual([ + "Failed to load translation resource 'test'; caused by Error: NOPE", + ]); + + expect(rendered1.result.current.t('key')).toBe('default'); + expect(rendered2.result.current.t('key')).toBe('default'); + }); }); diff --git a/packages/core-plugin-api/src/translation/useTranslationRef.ts b/packages/core-plugin-api/src/translation/useTranslationRef.ts index 9919ab3b38..4f09d0b970 100644 --- a/packages/core-plugin-api/src/translation/useTranslationRef.ts +++ b/packages/core-plugin-api/src/translation/useTranslationRef.ts @@ -24,6 +24,9 @@ import { } from '../apis/alpha'; import { TranslationRef } from './TranslationRef'; +// Make sure we don't fill the logs with loading errors for the same ref +const loggedRefs = new WeakSet>(); + /** @alpha */ export const useTranslationRef = < TMessages extends { [key in string]: string }, @@ -61,11 +64,23 @@ export const useTranslationRef = < if (!snapshot.ready) { throw new Promise(resolve => { - const subscription = observable.subscribe(next => { - if (next.ready) { - subscription.unsubscribe(); + const subscription = observable.subscribe({ + next(next) { + if (next.ready) { + subscription.unsubscribe(); + resolve(); + } + }, + error(err) { + if (!loggedRefs.has(translationRef)) { + // eslint-disable-next-line no-console + console.error( + `Failed to load translation resource '${translationRef.id}'; caused by ${err}`, + ); + loggedRefs.add(translationRef); + } resolve(); - } + }, }); }); }