From ed6ab1b1fe29969018140a5e6fb0904e46f2c47c Mon Sep 17 00:00:00 2001 From: Tilman Vatteroth Date: Mon, 28 Mar 2022 21:38:03 +0200 Subject: [PATCH] Refactor abcframe Signed-off-by: Tilman Vatteroth --- jest.config.ts | 1 + locales/en.json | 143 +- ...oundary.tsx => async-loading-boundary.tsx} | 8 +- .../__snapshots__/abc-frame.test.tsx.snap | 2442 +++++++++++++++++ .../abcjs/abc-frame.test.tsx | 68 + .../markdown-extension/abcjs/abc-frame.tsx | 56 +- .../highlighted-fence/highlighted-code.tsx | 6 +- .../markdown-renderer/test-utils/mock-i18n.ts | 27 + src/hooks/common/use-effect-with-catch.ts | 30 + 9 files changed, 2687 insertions(+), 94 deletions(-) rename src/components/common/{async-library-loading-boundary.tsx => async-loading-boundary.tsx} (84%) create mode 100644 src/components/markdown-renderer/markdown-extension/abcjs/__snapshots__/abc-frame.test.tsx.snap create mode 100644 src/components/markdown-renderer/markdown-extension/abcjs/abc-frame.test.tsx create mode 100644 src/components/markdown-renderer/test-utils/mock-i18n.ts create mode 100644 src/hooks/common/use-effect-with-catch.ts diff --git a/jest.config.ts b/jest.config.ts index 0ed8a7582..6f3eab5b2 100644 --- a/jest.config.ts +++ b/jest.config.ts @@ -20,6 +20,7 @@ const customJestConfig = { '^@/components/(.*)$': '/src/components/$1', }, roots: ["/src"], + testEnvironment: 'jsdom', testPathIgnorePatterns: ["/node_modules/", "/cypress/"] } diff --git a/locales/en.json b/locales/en.json index 226094300..e9f068b85 100644 --- a/locales/en.json +++ b/locales/en.json @@ -465,76 +465,79 @@ "placeholderText": "Placeholder", "upload": "Upload image" } - } - }, - "views": { - "presentation": {}, - "readOnly": { - "viewCount": "views", - "editNote": "Edit this note", - "loading": "Loading note contents ...", - "error": { - "title": "Error while loading note", - "description": "Probably the requested note does not exist or was deleted." - } - } - }, - "common": { - "yes": "Yes", - "no": "No", - "import": "Import", - "export": "Export", - "refresh": "Refresh", - "cancel": "Cancel", - "dismiss": "Dismiss", - "ok": "OK", - "close": "Close", - "save": "Save", - "delete": "Delete", - "or": "or", - "and": "and", - "avatarOf": "avatar of '{{name}}'", - "why": "Why?", - "loading": "Loading ...", - "errorOccurred": "An error occurred", - "errorWhileLoading": "An unexpected error occurred while loading '{{name}}'.\nCheck the browser console for more information.\nReport this error only if it comes up again.", - "readForMoreInfo": "Read here for more information" - }, - "copyOverlay": { - "error": "Error while copying!", - "success": "Copied!" - }, - "login": { - "chooseMethod": "Choose method", - "signInVia": "Sign in via {{service}}", - "signIn": "Sign In", - "signOut": "Sign Out", - "logoutFailed": "There was an error logging you out.\nClose your browser window to ensure session data is removed.", - "auth": { - "email": "Email", - "password": "Password", - "username": "Username", - "error": { - "openIdLogin": "Invalid OpenID provided", - "usernamePassword": "Invalid username or password", - "loginDisabled": "The login is disabled", - "other": "There was an error logging you in." - } }, - "register": { - "title": "Register", - "passwordAgain": "Password (again)", - "usernameInfo": "The username is your unique identifier for login.", - "passwordInfo": "Choose a unique and secure password. It must contain at least 8 characters.", - "infoTermsPrivacy": "With the registration of my user account I agree to the following terms:", - "error": { - "registrationDisabled": "The registration is disabled", - "usernameExisting": "There is already an account with this username.", - "other": "There was an error while registering your account. Just try it again." - } - } - }, - "motd": { - "title": "Information" + "abcJs": { + "errorWhileRendering": "Error while rendering your score. Please check if the code is correct." } + }, + "views": { + "presentation": {}, + "readOnly": { + "viewCount": "views", + "editNote": "Edit this note", + "loading": "Loading note contents ...", + "error": { + "title": "Error while loading note", + "description": "Probably the requested note does not exist or was deleted." + } + } + }, + "common": { + "yes": "Yes", + "no": "No", + "import": "Import", + "export": "Export", + "refresh": "Refresh", + "cancel": "Cancel", + "dismiss": "Dismiss", + "ok": "OK", + "close": "Close", + "save": "Save", + "delete": "Delete", + "or": "or", + "and": "and", + "avatarOf": "avatar of '{{name}}'", + "why": "Why?", + "loading": "Loading ...", + "errorOccurred": "An error occurred", + "errorWhileLoading": "An unexpected error occurred while loading '{{name}}'.\nCheck the browser console for more information.\nReport this error only if it comes up again.", + "readForMoreInfo": "Read here for more information" + }, + "copyOverlay": { + "error": "Error while copying!", + "success": "Copied!" + }, + "login": { + "chooseMethod": "Choose method", + "signInVia": "Sign in via {{service}}", + "signIn": "Sign In", + "signOut": "Sign Out", + "logoutFailed": "There was an error logging you out.\nClose your browser window to ensure session data is removed.", + "auth": { + "email": "Email", + "password": "Password", + "username": "Username", + "error": { + "openIdLogin": "Invalid OpenID provided", + "usernamePassword": "Invalid username or password", + "loginDisabled": "The login is disabled", + "other": "There was an error logging you in." + } + }, + "register": { + "title": "Register", + "passwordAgain": "Password (again)", + "usernameInfo": "The username is your unique identifier for login.", + "passwordInfo": "Choose a unique and secure password. It must contain at least 8 characters.", + "infoTermsPrivacy": "With the registration of my user account I agree to the following terms:", + "error": { + "registrationDisabled": "The registration is disabled", + "usernameExisting": "There is already an account with this username.", + "other": "There was an error while registering your account. Just try it again." + } + } + }, + "motd": { + "title": "Information" + } } diff --git a/src/components/common/async-library-loading-boundary.tsx b/src/components/common/async-loading-boundary.tsx similarity index 84% rename from src/components/common/async-library-loading-boundary.tsx rename to src/components/common/async-loading-boundary.tsx index 5a0f97e11..f31fb62a8 100644 --- a/src/components/common/async-library-loading-boundary.tsx +++ b/src/components/common/async-loading-boundary.tsx @@ -11,7 +11,7 @@ import { Alert } from 'react-bootstrap' export interface AsyncLoadingBoundaryProps { loading: boolean - error?: Error + error?: boolean componentName: string } @@ -21,17 +21,17 @@ export interface AsyncLoadingBoundaryProps { * * @param loading Indicates that the component is currently loading. Setting this will show a spinner instead of the children. * @param error Indicates that an error occurred during the loading process. Setting this to any non-null value will show an error message instead of the children. - * @param libraryName The name of the component that is currently loading. It will be shown in the error message. + * @param componentName The name of the component that is currently loading. It will be shown in the error message. * @param children The child {@link ReactElement elements} that are only shown if the component isn't in loading or error state */ -export const AsyncLibraryLoadingBoundary: React.FC = ({ +export const AsyncLoadingBoundary: React.FC = ({ loading, error, componentName, children }) => { useTranslation() - if (error) { + if (error === true) { return ( diff --git a/src/components/markdown-renderer/markdown-extension/abcjs/__snapshots__/abc-frame.test.tsx.snap b/src/components/markdown-renderer/markdown-extension/abcjs/__snapshots__/abc-frame.test.tsx.snap new file mode 100644 index 000000000..8e1d74f43 --- /dev/null +++ b/src/components/markdown-renderer/markdown-extension/abcjs/__snapshots__/abc-frame.test.tsx.snap @@ -0,0 +1,2442 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ + +exports[`AbcFrame renders a music sheet 1`] = ` +
+
+ +
+
+`; + +exports[`AbcFrame renders a music sheet 2`] = ` +
+
+ + + + Sheet Music for "Speed the Plough" + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + Speed the Plough + + + + + Trad. + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+
+`; + +exports[`AbcFrame renders an error if abcjs file can't be loaded 1`] = ` +
+
+ +
+
+`; + +exports[`AbcFrame renders an error if abcjs file can't be loaded 2`] = ` +
+ +
+`; + +exports[`AbcFrame renders an error if abcjs render function crashes 1`] = ` +
+
+ +
+
+`; + +exports[`AbcFrame renders an error if abcjs render function crashes 2`] = ` +
+ +
+
+ +
+
+
+`; diff --git a/src/components/markdown-renderer/markdown-extension/abcjs/abc-frame.test.tsx b/src/components/markdown-renderer/markdown-extension/abcjs/abc-frame.test.tsx new file mode 100644 index 000000000..bd27dd464 --- /dev/null +++ b/src/components/markdown-renderer/markdown-extension/abcjs/abc-frame.test.tsx @@ -0,0 +1,68 @@ +/* + * SPDX-FileCopyrightText: 2022 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ + +import React from 'react' +import { render, screen } from '@testing-library/react' +import { AbcFrame } from './abc-frame' +import { mockI18n } from '../../test-utils/mock-i18n' + +describe('AbcFrame', () => { + beforeEach(async () => { + jest.resetModules() + jest.restoreAllMocks() + await mockI18n() + }) + + it('renders a music sheet', async () => { + const element = ( + + ) + const view = render(element) + expect(view.container).toMatchSnapshot() + expect(await screen.findByText('Sheet Music for "Speed the Plough"')).toBeInTheDocument() + expect(view.container).toMatchSnapshot() + }) + + it("renders an error if abcjs file can't be loaded", async () => { + jest.mock('abcjs', () => { + throw new Error('abc is exploded!') + }) + const element = ( + + ) + const view = render(element) + expect(view.container).toMatchSnapshot() + expect(await screen.findByText('common.errorWhileLoading')).toBeInTheDocument() + expect(view.container).toMatchSnapshot() + }) + + it('renders an error if abcjs render function crashes', async () => { + jest.mock('abcjs', () => ({ + renderAbc: () => { + throw new Error('abc is exploded!') + } + })) + const element = ( + + ) + const view = render(element) + expect(view.container).toMatchSnapshot() + expect(await screen.findByText('editor.embeddings.abcJs.errorWhileRendering')).toBeInTheDocument() + expect(view.container).toMatchSnapshot() + }) +}) diff --git a/src/components/markdown-renderer/markdown-extension/abcjs/abc-frame.tsx b/src/components/markdown-renderer/markdown-extension/abcjs/abc-frame.tsx index b63a67c55..fbd1ac78a 100644 --- a/src/components/markdown-renderer/markdown-extension/abcjs/abc-frame.tsx +++ b/src/components/markdown-renderer/markdown-extension/abcjs/abc-frame.tsx @@ -4,36 +4,58 @@ * SPDX-License-Identifier: AGPL-3.0-only */ -import React, { useEffect, useRef } from 'react' +import React, { useRef } from 'react' import styles from './abc.module.scss' import { Logger } from '../../../../utils/logger' import type { CodeProps } from '../../replace-components/code-block-component-replacer' import { cypressId } from '../../../../utils/cypress-attribute' +import { useAsync } from 'react-use' +import { AsyncLoadingBoundary } from '../../../common/async-loading-boundary' +import { WaitSpinner } from '../../../common/wait-spinner/wait-spinner' +import { useEffectWithCatch } from '../../../../hooks/common/use-effect-with-catch' +import { Alert } from 'react-bootstrap' +import { ShowIf } from '../../../common/show-if/show-if' +import { Trans } from 'react-i18next' const log = new Logger('AbcFrame') export const AbcFrame: React.FC = ({ code }) => { const container = useRef(null) - useEffect(() => { - if (!container.current) { + const { + error: loadingError, + loading, + value: abcLib + } = useAsync(async () => { + try { + return await import(/* webpackChunkName: "abc.js" */ 'abcjs') + } catch (error) { + log.error('Error while loading abcjs', error) + throw error + } + }, []) + + const renderError = useEffectWithCatch(() => { + const actualContainer = container.current + if (!actualContainer || !abcLib) { return } - const actualContainer = container.current - import(/* webpackChunkName: "abc.js" */ 'abcjs') - .then((importedLibrary) => { - importedLibrary.renderAbc(actualContainer, code, {}) - }) - .catch((error: Error) => { - log.error('Error while loading abcjs', error) - }) - }, [code]) + abcLib.renderAbc(actualContainer, code, {}) + }, [code, abcLib]) return ( -
+ + + + + + +
+ +
+
) } diff --git a/src/components/markdown-renderer/markdown-extension/highlighted-fence/highlighted-code.tsx b/src/components/markdown-renderer/markdown-extension/highlighted-fence/highlighted-code.tsx index 0c674050a..992697203 100644 --- a/src/components/markdown-renderer/markdown-extension/highlighted-fence/highlighted-code.tsx +++ b/src/components/markdown-renderer/markdown-extension/highlighted-fence/highlighted-code.tsx @@ -8,7 +8,7 @@ import React from 'react' import { CopyToClipboardButton } from '../../../common/copyable/copy-to-clipboard-button/copy-to-clipboard-button' import styles from './highlighted-code.module.scss' import { cypressAttribute, cypressId } from '../../../../utils/cypress-attribute' -import { AsyncLibraryLoadingBoundary } from '../../../common/async-library-loading-boundary' +import { AsyncLoadingBoundary } from '../../../common/async-loading-boundary' import { useAsyncHighlightedCodeDom } from './hooks/use-async-highlighted-code-dom' import { useAttachLineNumbers } from './hooks/use-attach-line-numbers' @@ -33,7 +33,7 @@ export const HighlightedCode: React.FC = ({ code, language const wrappedDomLines = useAttachLineNumbers(highlightedLines, startLineNumber) return ( - +
= ({ code, language
- + ) } diff --git a/src/components/markdown-renderer/test-utils/mock-i18n.ts b/src/components/markdown-renderer/test-utils/mock-i18n.ts new file mode 100644 index 000000000..6594704ca --- /dev/null +++ b/src/components/markdown-renderer/test-utils/mock-i18n.ts @@ -0,0 +1,27 @@ +/* + * SPDX-FileCopyrightText: 2022 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ + +import type { TFunction } from 'i18next' +import i18n from 'i18next' +import { initReactI18next } from 'react-i18next' + +/** + * Initializes i18n with minimal settings and without any data, so it just returns the used key as translation. + * + * @return A promise that resolves if i18n has been initialized + */ +export const mockI18n = (): Promise => { + return i18n.use(initReactI18next).init({ + lng: 'en', + fallbackLng: 'en', + ns: ['translationsNS'], + defaultNS: 'translationsNS', + interpolation: { + escapeValue: false + }, + resources: { en: { translationsNS: {} } } + }) +} diff --git a/src/hooks/common/use-effect-with-catch.ts b/src/hooks/common/use-effect-with-catch.ts new file mode 100644 index 000000000..f9e76618d --- /dev/null +++ b/src/hooks/common/use-effect-with-catch.ts @@ -0,0 +1,30 @@ +/* + * SPDX-FileCopyrightText: 2022 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ + +import type { DependencyList, EffectCallback } from 'react' +import { useEffect, useState } from 'react' + +/** + * Executes a side effects but catches any thrown error. + * + * @param effect The side effect to execute + * @param deps The dependencies of the effect + * @return The produced error (if occurred) + */ +export const useEffectWithCatch = (effect: EffectCallback, deps: DependencyList = []): Error | undefined => { + const [error, setError] = useState(undefined) + + useEffect(() => { + try { + return effect() + } catch (error) { + setError(error as Error) + } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, deps) + + return error +}