fix(plugin): clean up resize observer and handle sonarqube warnings (#…

…898) * refactor: use css modules in example app * chore: remove finished todos in makeViteConfig * chore: add jira issues to todos * refactor: simplify hosted location string ternary * refactor: remove always truthy condition * fix(plugin): remove resize observer on cleanup * chore: import order * fix: consistent ref * chore: style
dhis2 · Dec 10, 2024 · f113dd5 · f113dd5
1 parent 5b92212
commit f113dd5
Show file tree

Hide file tree

Showing 10 changed files with 35 additions and 35 deletions.
diff --git a/cli/config/makeViteConfig.mjs b/cli/config/makeViteConfig.mjs
@@ -25,7 +25,7 @@ import dynamicImport from 'vite-plugin-dynamic-import'
  * merged with the main config if the `--allowJsxInJs` flag is passed
  * to the CLI
  *
- * todo: deprecate -- this config has a performance cost, especially on startup
+ * ! deprecated -- this config has a performance cost, especially on startup
  */
 const jsxInJsConfig = {
     plugins: [
@@ -178,7 +178,7 @@ export default ({ paths, config, env, host, force, allowJsxInJs }) => {
             dynamicImport(),
             react({
                 babel: { plugins: ['styled-jsx/babel'] },
-                // todo: deprecate with other jsx-in-js config
+                // ! deprecated with other jsx-in-js config
                 // This option allows HMR of JSX-in-JS files,
                 // but it isn't possible to add with merge config:
                 jsxRuntime: allowJsxInJs ? 'classic' : 'automatic',

diff --git a/cli/src/commands/deploy.js b/cli/src/commands/deploy.js
@@ -152,6 +152,7 @@ const handler = async ({ cwd = process.cwd(), timeout, ...params }) => {
     }
 
     // todo: modify for multiple/named plugins
+    // https://dhis2.atlassian.net/browse/LIBS-394
     const appUrl = constructAppUrl({
         baseUrl: dhis2Config.baseUrl,
         config,

diff --git a/cli/src/commands/start.js b/cli/src/commands/start.js
@@ -161,11 +161,12 @@ const handler = async ({
             })
             const server = await createServer(viteConfig)
 
-            const location = config.entryPoints.plugin
-                ? config.entryPoints.app
+            let location = ''
+            if (config.entryPoints.plugin) {
+                location = config.entryPoints.app
                     ? ' at / and /plugin.html'
                     : ' at /plugin.html'
-                : ''
+            }
 
             reporter.print(
                 `The app ${chalk.bold(

diff --git a/cli/src/lib/compiler/compile.js b/cli/src/lib/compiler/compile.js
@@ -142,6 +142,7 @@ const compile = async ({
             inputDir: paths.src,
             outputDir: outDir,
             // todo: handle lib compilations with Vite
+            // https://dhis2.atlassian.net/browse/LIBS-722
             processFileCallback: isAppType ? copyFile : compileFile,
             watch,
         }),

diff --git a/examples/simple-app/src/Alerter.jsx b/examples/simple-app/src/Alerter.jsx
@@ -1,6 +1,7 @@
 import { useAlert } from '@dhis2/app-runtime'
 import { InputField, CheckboxField, Button } from '@dhis2/ui'
 import React, { useState } from 'react'
+import styles from './Alerter.module.css'
 
 export const Alerter = () => {
     const [message, setMessage] = useState('')
@@ -12,7 +13,7 @@ export const Alerter = () => {
     )
 
     return (
-        <div>
+        <div className={styles.flexContainer}>
             <InputField
                 placeholder="Type alert message"
                 value={message}
@@ -25,14 +26,6 @@ export const Alerter = () => {
             />
             <Button onClick={() => show(message)}>Show alert</Button>
             <Button onClick={hide}>Hide alert</Button>
-            <style jsx>{`
-                div {
-                    display: flex;
-                    align-items: center;
-                    justify-content: space-between;
-                    width: 500px;
-                }
-            `}</style>
         </div>
     )
 }
diff --git a/examples/simple-app/src/Alerter.module.css b/examples/simple-app/src/Alerter.module.css
@@ -0,0 +1,6 @@
+.flexContainer {
+    display: flex;
+    align-items: center;
+    justify-content: space-between;
+    width: 500px;
+}
diff --git a/examples/simple-app/src/App.jsx b/examples/simple-app/src/App.jsx
@@ -2,7 +2,7 @@ import { useDataQuery } from '@dhis2/app-runtime'
 import moment from 'moment'
 import React from 'react'
 import { Alerter } from './Alerter.jsx'
-import style from './App.style.js'
+import styles from './App.module.css'
 import i18n from './locales/index.js'
 
 const query = {
@@ -14,8 +14,7 @@ const query = {
 const Component = () => {
     const { error, loading, data } = useDataQuery(query)
     return (
-        <div>
-            <style jsx>{style}</style>
+        <div className={styles.appContainer}>
             {error && <span>ERROR</span>}
             {loading && <span>...</span>}
             {data && (

diff --git a/examples/simple-app/src/App.module.css b/examples/simple-app/src/App.module.css
@@ -0,0 +1,10 @@
+.appContainer {
+    width: 100%;
+    height: 100%;
+
+    display: flex;
+    flex-direction: column;
+    align-items: center;
+    justify-content: center;
+    font-size: 1rem;
+}
diff --git a/examples/simple-app/src/App.style.js b/examples/simple-app/src/App.style.js
diff --git a/shell/src/PluginLoader.jsx b/shell/src/PluginLoader.jsx
@@ -15,13 +15,16 @@ const PluginResizeInner = ({
     const innerDivRef = useRef()
     useEffect(() => {
         if (divRef && divRef.current && resizePluginHeight) {
+            const container = divRef.current
             const resizeObserver = new ResizeObserver(() => {
                 // the additional pixels currently account for possible horizontal scroll bar
-                if (resizePluginHeight) {
-                    resizePluginHeight(divRef.current.offsetHeight + 20)
-                }
+                resizePluginHeight(container.offsetHeight + 20)
             })
-            resizeObserver.observe(divRef.current)
+            resizeObserver.observe(container)
+            return () => {
+                resizeObserver.unobserve(container)
+                resizeObserver.disconnect()
+            }
         }
     }, [resizePluginHeight])