refactor(use query): abort request on unmount; updated tests & lint i…

…ssues

components/organisation-unit-tree/package.json

@@ -44,6 +44,7 @@
         "build"
     ],
     "devDependencies": {
+        "@dhis2-ui/button": "6.12.0",
         "@dhis2/app-runtime": "^2.7.1",
         "@dhis2/d2-i18n": "^1.1.0",
         "@testing-library/react-hooks": "^7.0.1",

components/organisation-unit-tree/src/__stories__/collapsed.js

@@ -2,8 +2,5 @@ import React from 'react'
 import { OrganisationUnitTree } from '../index.js'
 
 export const Collapsed = () => (
-    <OrganisationUnitTree
-        name="Root org unit"
-        roots={['A0000000000']}
-    />
+    <OrganisationUnitTree name="Root org unit" roots={['A0000000000']} />
 )

components/organisation-unit-tree/src/__stories__/custom-requests.js

@@ -1,4 +1,3 @@
-/* eslint-disable react/no-unescaped-entities */
 import { CustomDataProvider, useDataEngine } from '@dhis2/app-runtime'
 import React from 'react'
 import {
@@ -16,17 +15,17 @@ const customDataWithApprovals = {
                 wf: params.wf,
                 pe: params.pe,
                 ou: params.ou,
-                aoc: "aocUID",
-                state: "ACCEPTED_HERE",
-                level: "KaTJLhGmU95",
+                aoc: 'aocUID',
+                state: 'ACCEPTED_HERE',
+                level: 'KaTJLhGmU95',
                 permissions: {
                     mayApprove: true,
                     mayUnapprove: true,
                     mayAccept: false,
                     mayUnaccept: true,
-                    mayReadData: true
-                }
-            }
+                    mayReadData: true,
+                },
+            },
         ]
     },
 }
@@ -47,8 +46,8 @@ export const CustomRequests = () => {
                         wf: 'rIUL4hYOjJc',
                         pe: '202101',
                         ou: id,
-                    }
-                }
+                    },
+                },
             }),
             await fetchOrgData(args),
         ])
@@ -66,9 +65,9 @@ export const CustomRequests = () => {
             initiallyExpanded={['/A0000000000/A0000000001/A0000000003']}
             renderNodeLabel={data => {
                 if (data.loading) {
-                    return OrganisationUnitTreeControllable
-                        .defaultProps
-                        .renderNodeLabel(data)
+                    return OrganisationUnitTreeControllable.defaultProps.renderNodeLabel(
+                        data
+                    )
                 }
 
                 const { approvalStatuses } = data.additional
@@ -80,16 +79,15 @@ export const CustomRequests = () => {
                     ...data,
                     label: (
                         <span>
-                            {data.node.displayName}
-                            {' '}
-                            (mayApprove: {' '}{mayApprove.toString()})
+                            {data.node.displayName} (mayApprove:{' '}
+                            {mayApprove.toString()})
                         </span>
-                    )
+                    ),
                 }
 
-                return OrganisationUnitTreeControllable
-                    .defaultProps
-                    .renderNodeLabel(formatted)
+                return OrganisationUnitTreeControllable.defaultProps.renderNodeLabel(
+                    formatted
+                )
             }}
         />
     )

components/organisation-unit-tree/src/__stories__/dx-wrapper.js

@@ -1,4 +1,4 @@
-/* eslint-disable react/no-unescaped-entities,react/prop-types */
+/* eslint-disable react/prop-types */
 import React, { useState } from 'react'
 import { OrganisationUnitTree } from '../index.js'
 

components/organisation-unit-tree/src/__stories__/replace-roots.js

@@ -4,8 +4,8 @@ export const ReplaceRoots = () => {
     return (
         <p>
             This is currently not working due to limitations of the data engine
-            in the app runtime. Normally the root unit would&apos;ve been replaced
-            after 2000 milliseconds.
+            in the app runtime. Normally the root unit would&apos;ve been
+            replaced after 2000 milliseconds.
         </p>
     )
 

components/organisation-unit-tree/src/helpers/use-query.js

@@ -1,46 +1,66 @@
-import { useCallback, useState, useEffect } from 'react'
+import { useCallback, useRef, useState, useEffect } from 'react'
 
 export const useQuery = (
-    fetchFn,
+    queryOrMutateFn,
     {
-        initialArguments = [],
+        initialArgument = undefined,
         defaultData = null,
         transform = null,
         onComplete = null,
-    }
+    } = {}
 ) => {
+    // use ref not state so setting the value can be done synchronously
+    const signal = useRef()
+
     const [loading, setLoading] = useState(true)
     const [error, setError] = useState(null)
     const [data, setData] = useState(defaultData)
 
     const performRequest = useCallback(
-        (...args) => fetchFn(...args)
-            .then(response => {
-                const data = transform ? transform(response) : response
-                const dataWithDefaults = { ...defaultData, ...data }
-
-                setData(dataWithDefaults)
-                setLoading(false)
-                return dataWithDefaults
-            })
-            .then(data => onComplete && onComplete(data))
-            .catch(error => {
-                setError(error)
-                setLoading(false)
-            }),
+        arg => {
+            if (arg?.signal) {
+                signal.current = arg.signal
+            } else {
+                signal.current = new AbortController()
+            }
+
+            return queryOrMutateFn({ ...arg, signal: signal.current })
+                .then(response => {
+                    const data = transform ? transform(response) : response
+                    const dataWithDefaults = { ...defaultData, ...data }
+
+                    setData(dataWithDefaults)
+                    setLoading(false)
+                    return dataWithDefaults
+                })
+                .then(data => onComplete && onComplete(data))
+                .catch(error => {
+                    setError(error)
+                    setLoading(false)
+                })
+        },
         [setLoading, setError, setData, onComplete]
     )
 
-    const refetch = useCallback((...args) => {
-        setLoading(true)
-        setError(null)
-        setData(defaultData)
+    const refetch = useCallback(
+        arg => {
+            setLoading(true)
+            setError(null)
+            setData(defaultData)
 
-        return performRequest(...args)
-    }, [setLoading, setError, setData, performRequest])
+            return performRequest(arg)
+        },
+        [setLoading, setError, setData, performRequest]
+    )
 
     useEffect(() => {
-        performRequest(...initialArguments)
+        performRequest(initialArgument)
+
+        return () => {
+            if (signal.current) {
+                signal.current.abort()
+            }
+        }
     }, [])
 
     return { refetch, loading, error, data }

components/organisation-unit-tree/src/helpers/use-query.test.js

@@ -0,0 +1,89 @@
+import EventEmitter from 'events'
+import { renderHook, act } from '@testing-library/react-hooks'
+import { useQuery } from './use-query.js'
+
+jest.useFakeTimers()
+
+describe('useQuery', () => {
+    const fetchFn = jest.fn()
+    const onTimeout = jest.fn(resolve => {
+        resolve({ foo: 'bar' })
+    })
+
+    // make sure request can be aborted to prevent state mutation after unmount
+    const emitter = new EventEmitter()
+    jest.spyOn(AbortController.prototype, 'abort').mockImplementation(() => {
+        emitter.emit('abort')
+    })
+
+    fetchFn.mockImplementation(() => {
+        return new Promise((resolve, reject) => {
+            const timeout = setTimeout(() => onTimeout(resolve, reject), 500)
+            emitter.on('abort', () => clearTimeout(timeout))
+        })
+    })
+
+    afterEach(() => {
+        fetchFn.mockClear()
+    })
+
+    it('should start with loading: true, error: null and data: null', async () => {
+        const hookState = renderHook(() => useQuery(fetchFn))
+        expect(hookState.result.current).toEqual(expect.objectContaining({
+            loading: true,
+            error: null,
+            data: null,
+        }))
+    })
+
+    it('should use the "defaultData" option as initial data when provided', () => {
+        const defaultData = { foo: 'bar' }
+        const { result } = renderHook(() => useQuery(fetchFn, { defaultData }))
+        expect(result.current).toEqual(expect.objectContaining({
+            loading: true,
+            error: null,
+            data: { foo: 'bar' },
+        }))
+    })
+
+    it('should perform the initial request right away', () => {
+        const defaultData = { foo: 'bar' }
+        renderHook(() => useQuery(fetchFn, { defaultData }))
+        expect(fetchFn).toHaveBeenCalledTimes(1)
+    })
+
+    it('should set the data after a successful initial request', async () => {
+        const { result, waitForNextUpdate } = renderHook(() => useQuery(fetchFn))
+        expect(fetchFn).toHaveBeenCalledTimes(1)
+
+        await act(async () => {
+            jest.advanceTimersByTime(1000)
+            await waitForNextUpdate()
+        })
+
+        expect(result.current).toEqual(expect.objectContaining({
+            loading: false,
+            error: null,
+            data: { foo: 'bar' },
+        }))
+    })
+
+    it('should set the error after a failed initial request', async () => {
+        const failureError = new Error('I am a failure')
+        onTimeout.mockImplementationOnce((resolve, reject) => reject(failureError))
+
+        const { result, waitForNextUpdate } = renderHook(() => useQuery(fetchFn))
+        expect(fetchFn).toHaveBeenCalledTimes(1)
+
+        await act(async () => {
+            jest.advanceTimersByTime(1000)
+            await waitForNextUpdate()
+        })
+
+        expect(result.current).toEqual(expect.objectContaining({
+            loading: false,
+            error: failureError,
+            data: null,
+        }))
+    })
+})

components/organisation-unit-tree/src/index.js

@@ -1,6 +1,4 @@
-export {
-    OrganisationUnitTree as OrganisationUnitTreeControllable,
-} from './organisation-unit-tree/index.js'
+export { OrganisationUnitTree as OrganisationUnitTreeControllable } from './organisation-unit-tree/index.js'
 export {
     OrganisationUnitTreeConnected as OrganisationUnitTree,
     useFetchOrgData,

components/organisation-unit-tree/src/organisation-unit-node/organisation-unit-node.js

@@ -57,11 +57,9 @@ export const OrganisationUnitNode = ({
         defaultData: {
             organisationUnit: { id, displayName },
         },
-        initialArguments: [
-            {
-                variables: { id, isUserDataViewFallback },
-            },
-        ],
+        initialArgument: {
+            variables: { id, isUserDataViewFallback },
+        },
         transform: data => {
             const { organisationUnit } = data
             const sorted = !suppressAlphabeticalSorting
@@ -71,10 +69,11 @@ export const OrganisationUnitNode = ({
             return { ...data, organisationUnit: sorted }
         },
         onComplete: ({ organisationUnit }) =>
-            onChildrenLoaded(organisationUnit),
+            onChildrenLoaded && onChildrenLoaded(organisationUnit),
     })
 
     const { organisationUnit, ...additional } = data
+
     const childNodes =
         !loading && !error ? computeChildNodes(organisationUnit, filter) : []
     const hasChildren = !!childNodes.length

components/organisation-unit-tree/src/organisation-unit-tree-connected/use-fetch-org-data/use-fetch-org-data.js

@@ -1,4 +1,5 @@
 import { useDataEngine } from '@dhis2/app-runtime'
+import { useCallback, useState } from 'react'
 
 const ORG_DATA_QUERY = {
     organisationUnit: {
@@ -20,5 +21,12 @@ const ORG_DATA_QUERY = {
  */
 export const useFetchOrgData = () => {
     const engine = useDataEngine()
-    return ({ variables }) => engine.query(ORG_DATA_QUERY, { variables })
+    const [persistedEngine] = useState(engine)
+
+    const fetchOrgData = useCallback(
+        ({ variables, signal }) => persistedEngine.query(ORG_DATA_QUERY, { variables, signal }),
+        [persistedEngine]
+    )
+
+    return fetchOrgData
 }