Refactor abcframe

Signed-off-by: Tilman Vatteroth <git@tilmanvatteroth.de>

jest.config.ts

@@ -20,6 +20,7 @@ const customJestConfig = {
     '^@/components/(.*)$': '<rootDir>/src/components/$1',
   },
   roots: ["<rootDir>/src"],
+  testEnvironment: 'jsdom',
   testPathIgnorePatterns: ["/node_modules/", "/cypress/"]
 }
 

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": {
+    "abcJs": {
-            "title": "Register",
+      "errorWhileRendering": "Error while rendering your score. Please check if the code is correct."
-            "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"
     }
+  },
+  "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"
+  }
 }

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<AsyncLoadingBoundaryProps> = ({
+export const AsyncLoadingBoundary: React.FC<AsyncLoadingBoundaryProps> = ({
   loading,
   error,
   componentName,
   children
 }) => {
   useTranslation()
-  if (error) {
+  if (error === true) {
     return (
       <Alert variant={'danger'}>
         <Trans i18nKey={'common.errorWhileLoading'} values={{ name: componentName }} />

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 = (
+      <AbcFrame
+        code={
+          'X:1\nT:Speed the Plough\nM:4/4\nC:Trad.\nK:G\n|:GABc dedB|dedB dedB|c2ec B2dB|c2A2 A2BA|\nGABc dedB|dedB dedB|c2ec B2dB|A2F2 G4:|\n|:g2gf gdBd|g2f2 e2d2|c2ec B2dB|c2A2 A2df|\ng2gf g2Bd|g2f2 e2d2|c2ec B2dB|A2F2 G4:|'
+        }
+      />
+    )
+    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 = (
+      <AbcFrame
+        code={
+          'X:1\nT:Speed the Plough\nM:4/4\nC:Trad.\nK:G\n|:GABc dedB|dedB dedB|c2ec B2dB|c2A2 A2BA|\nGABc dedB|dedB dedB|c2ec B2dB|A2F2 G4:|\n|:g2gf gdBd|g2f2 e2d2|c2ec B2dB|c2A2 A2df|\ng2gf g2Bd|g2f2 e2d2|c2ec B2dB|A2F2 G4:|'
+        }
+      />
+    )
+    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 = (
+      <AbcFrame
+        code={
+          'X:1\nT:Speed the Plough\nM:4/4\nC:Trad.\nK:G\n|:GABc dedB|dedB dedB|c2ec B2dB|c2A2 A2BA|\nGABc dedB|dedB dedB|c2ec B2dB|A2F2 G4:|\n|:g2gf gdBd|g2f2 e2d2|c2ec B2dB|c2A2 A2df|\ng2gf g2Bd|g2f2 e2d2|c2ec B2dB|A2F2 G4:|'
+        }
+      />
+    )
+    const view = render(element)
+    expect(view.container).toMatchSnapshot()
+    expect(await screen.findByText('editor.embeddings.abcJs.errorWhileRendering')).toBeInTheDocument()
+    expect(view.container).toMatchSnapshot()
+  })
+})

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<CodeProps> = ({ code }) => {
   const container = useRef<HTMLDivElement>(null)
 
-  useEffect(() => {
+  const {
-    if (!container.current) {
+    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
+    abcLib.renderAbc(actualContainer, code, {})
-    import(/* webpackChunkName: "abc.js" */ 'abcjs')
+  }, [code, abcLib])
-      .then((importedLibrary) => {
-        importedLibrary.renderAbc(actualContainer, code, {})
-      })
-      .catch((error: Error) => {
-        log.error('Error while loading abcjs', error)
-      })
-  }, [code])
 
   return (
-    <div
+    <AsyncLoadingBoundary loading={loading} error={!!loadingError} componentName={'abc.js'}>
-      ref={container}
+      <ShowIf condition={!!renderError}>
-      className={`${styles['abcjs-score']} bg-white text-black svg-container`}
+        <Alert variant={'danger'}>
-      {...cypressId('abcjs')}
+          <Trans i18nKey={'editor.embeddings.abcJs.errorWhileRendering'} />
-    />
+        </Alert>
+      </ShowIf>
+      <div
+        ref={container}
+        className={`${styles['abcjs-score']} bg-white text-black svg-container`}
+        {...cypressId('abcjs')}>
+        <WaitSpinner />
+      </div>
+    </AsyncLoadingBoundary>
   )
 }

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<HighlightedCodeProps> = ({ code, language
   const wrappedDomLines = useAttachLineNumbers(highlightedLines, startLineNumber)
 
   return (
-    <AsyncLibraryLoadingBoundary loading={loading} error={error} componentName={'highlight.js'}>
+    <AsyncLoadingBoundary loading={loading} error={!!error} componentName={'highlight.js'}>
       <div className={styles['code-highlighter']} {...cypressId('highlighted-code-block')}>
         <code
           {...cypressId('code-highlighter')}
@@ -46,7 +46,7 @@ export const HighlightedCode: React.FC<HighlightedCodeProps> = ({ code, language
           <CopyToClipboardButton content={code} {...cypressId('copy-code-button')} />
         </div>
       </div>
-    </AsyncLibraryLoadingBoundary>
+    </AsyncLoadingBoundary>
   )
 }
 

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<TFunction> => {
+  return i18n.use(initReactI18next).init({
+    lng: 'en',
+    fallbackLng: 'en',
+    ns: ['translationsNS'],
+    defaultNS: 'translationsNS',
+    interpolation: {
+      escapeValue: false
+    },
+    resources: { en: { translationsNS: {} } }
+  })
+}

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<Error | undefined>(undefined)
+
+  useEffect(() => {
+    try {
+      return effect()
+    } catch (error) {
+      setError(error as Error)
+    }
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, deps)
+
+  return error
+}