feat: support network request aborting and refetching

.eslintrc.js

@@ -2,4 +2,9 @@ const { config } = require('@dhis2/cli-style')
 
 module.exports = {
     extends: [config.eslint],
+    plugins: ['react-hooks'],
+    rules: {
+        'react-hooks/rules-of-hooks': 'error',
+        'react-hooks/exhaustive-deps': 'error',
+    },
 }

.yarnclean

@@ -0,0 +1 @@
+@types/**/node_modules

examples/cra/yarn.lock

@@ -815,7 +815,7 @@
   integrity sha512-6It2EVfGskxZCQhuykrfnALg7oVeiI6KclWSmGDqB0AiInVrTGB9Jp9i4/Ad21u9Jde/voVQz6eFX/eSg/UsPA==
 
 "@dhis2/app-runtime@file:../../runtime":
-  version "1.4.3"
+  version "1.4.2"
 
 "@hapi/address@2.x.x":
   version "2.0.0"

package.json

@@ -23,18 +23,19 @@
         "@testing-library/react": "^9.1.1",
         "@types/jest": "^24.0.17",
         "@types/node": "^12.7.1",
-        "@types/react": "^16.8.8",
-        "@types/react-dom": "^16.8.2",
+        "@types/react": "^16.9.1",
+        "@types/react-dom": "^16.8.5",
         "@typescript-eslint/eslint-plugin": "^1.13.0",
         "@typescript-eslint/parser": "^1.13.0",
         "concurrently": "^4.1.0",
         "eslint": "^5",
         "eslint-config-prettier": "^6.0.0",
+        "eslint-plugin-react-hooks": "^1.6.1",
         "jest": "^24.8.0",
         "loop": "^3.3.2",
         "prop-types": "^15.7.2",
-        "react": "^16.8.6",
-        "react-dom": "^16.8.6",
+        "react": "^16.9.0",
+        "react-dom": "^16.9.0",
         "rimraf": "^2.6.3",
         "rollup": "^1.19.4",
         "rollup-plugin-analyzer": "^3.1.2",

services/data/src/__tests__/integration.test.tsx

@@ -1,9 +1,8 @@
 import React from 'react'
-import { useDataQuery } from '../hooks/useDataQuery'
-import { render, waitForElement } from '@testing-library/react'
+import { render, waitForElement, act } from '@testing-library/react'
 import { CustomDataProvider } from '../components/CustomDataProvider'
 import { DataQuery } from '../components/DataQuery'
-import { QueryRenderInput } from '../types/Query'
+import { QueryRenderInput, QueryDefinition, RefetchCallback } from '../types/Query'
 
 const customData = {
     answer: 42,
@@ -31,26 +30,26 @@ describe('Testing custom data provider and useQuery hook', () => {
         expect(renderFunction).toHaveBeenCalledTimes(1)
         expect(renderFunction).toHaveBeenLastCalledWith({
             loading: true,
+            refetch: expect.any(Function)
         })
         await waitForElement(() => getByText(/data: /i))
         expect(renderFunction).toHaveBeenCalledTimes(2)
         expect(renderFunction).toHaveBeenLastCalledWith({
             loading: false,
             data: customData,
+            refetch: expect.any(Function)
         })
         expect(getByText(/data: /i)).toHaveTextContent(
             `data: ${customData.answer}`
         )
     })
-})
 
-describe('Testing custom data provider and useQuery hook', () => {
     it('Should render an error', async () => {
         const renderFunction = jest.fn(
             ({ loading, error, data }: QueryRenderInput) => {
                 if (loading) return 'loading'
                 if (error) return <div>error: {error.message}</div>
-                return <div>data: {data}</div>
+                return <div>data: {data.test}</div>
             }
         )
 
@@ -66,6 +65,7 @@ describe('Testing custom data provider and useQuery hook', () => {
         expect(renderFunction).toHaveBeenCalledTimes(1)
         expect(renderFunction).toHaveBeenLastCalledWith({
             loading: true,
+            refetch: expect.any(Function)
         })
         await waitForElement(() => getByText(/error: /i))
         expect(renderFunction).toHaveBeenCalledTimes(2)
@@ -76,4 +76,80 @@ describe('Testing custom data provider and useQuery hook', () => {
         //     `data: ${customData.answer}`
         // )
     })
+
+    it('Should abort the fetch when unmounted', async () => {
+        const renderFunction = jest.fn(
+            ({ loading, error, data }: QueryRenderInput) => {
+                if (loading) return 'loading'
+                if (error) return <div>error: {error.message}</div>
+                return <div>data: {data.test}</div>
+            }
+        )
+
+        let signal: AbortSignal | null | undefined
+        const mockData = {
+            factory: jest.fn((_: QueryDefinition, options: RequestInit) => {
+                signal = options.signal
+                return 'done'
+            })
+        }
+
+        const { unmount } = render(
+            <CustomDataProvider data={mockData}>
+                <DataQuery query={{ test: { resource: 'factory' } }}>
+                    {renderFunction}
+                </DataQuery>
+            </CustomDataProvider>
+        )
+
+        expect(renderFunction).toHaveBeenCalledTimes(1)
+        expect(mockData.factory).toHaveBeenCalledTimes(1)
+        act(() => { unmount() })
+        expect(signal && signal.aborted).toBe(true)
+    })
+
+    it('Should abort the fetch when refetching', async () => {
+        let refetch: RefetchCallback | undefined;
+        const renderFunction = jest.fn(
+            ({ loading, error, data, refetch: _refetch }: QueryRenderInput) => {
+                refetch = _refetch
+                if (loading) return 'loading'
+                if (error) return <div>error: {error.message}</div>
+                return <div>data: {data.test}</div>
+            }
+        )
+
+        let signal: AbortSignal | null | undefined
+        const mockData = {
+            factory: jest.fn((_: QueryDefinition, options: RequestInit) => {
+                if (!signal) {
+                    signal = options.signal // only capture first signal
+                }
+                return 'test'
+            })
+        }
+
+        render(
+            <CustomDataProvider data={mockData}>
+                <DataQuery query={{ test: { resource: 'factory' } }}>
+                    {renderFunction}
+                </DataQuery>
+            </CustomDataProvider>
+        )
+
+        expect(renderFunction).toHaveBeenCalledTimes(1)
+        expect(mockData.factory).toHaveBeenCalledTimes(1)
+
+        expect(refetch).not.toBeUndefined();
+        act(() => {
+            if (!refetch) {
+                throw 'help'
+            }
+            refetch()
+        })
+
+        expect(renderFunction).toHaveBeenCalledTimes(2)
+        expect(mockData.factory).toHaveBeenCalledTimes(2)
+        expect(signal && signal.aborted).toBe(true)
+    })
 })

services/data/src/components/DataQuery.tsx

@@ -7,7 +7,7 @@ interface QueryInput {
 }
 
 export const DataQuery = ({ query, children }: QueryInput) => {
-    const { loading, error, data } = useDataQuery(query)
+    const queryState = useDataQuery(query)
 
-    return children({ loading, error, data })
+    return children(queryState)
 }

services/data/src/context/makeCustomContext.ts

@@ -13,7 +13,8 @@ export type CustomResourceLiteral =
     | object
     | FetchErrorPayload
 export type CustomResourceFactory = (
-    query: QueryDefinition
+    query: QueryDefinition,
+    options?: RequestInit
 ) => Promise<CustomResourceLiteral>
 export type CustomResource = CustomResourceLiteral | CustomResourceFactory
 export interface CustomContextData {
@@ -22,12 +23,13 @@ export interface CustomContextData {
 export interface CustomContextOptions {
     loadForever?: boolean
     failOnMiss?: boolean
+    options?: RequestInit
 }
 
 const resolveCustomResource = async (
     customResource: CustomResource,
     query: QueryDefinition,
-    { failOnMiss }: CustomContextOptions
+    { failOnMiss, options }: CustomContextOptions
 ): Promise<CustomResource> => {
     switch (typeof customResource) {
         case 'string':
@@ -37,7 +39,7 @@ const resolveCustomResource = async (
             return customResource
         case 'function':
             // function
-            const result = await customResource(query)
+            const result = await customResource(query, options)
             if (!result && failOnMiss) {
                 throw new Error(
                     `The custom function for resource ${query.resource} must always return a value but returned ${result}`
@@ -55,7 +57,7 @@ export const makeCustomContext = (
     { failOnMiss = true, loadForever = false }: CustomContextOptions = {}
 ): ContextType => {
     const apiUrl = joinPath(baseUrl, 'api', String(apiVersion))
-    const customFetch: FetchFunction = async query => {
+    const customFetch: FetchFunction = async (query, options) => {
         const customResource = customData[query.resource]
         if (!customResource) {
             if (failOnMiss) {
@@ -68,6 +70,7 @@ export const makeCustomContext = (
 
         return await resolveCustomResource(customResource, query, {
             failOnMiss,
+            options,
         })
     }
     const foreverLoadingFetch: FetchFunction = async () => {

services/data/src/hooks/useDataQuery.ts

@@ -1,29 +1,66 @@
-import { useState, useContext, useEffect } from 'react'
+import { useState, useContext, useEffect, useCallback } from 'react'
 import { DataContext } from '../components/DataContext'
-import { QueryState, QueryMap } from '../types/Query'
+import {
+    QueryState,
+    QueryMap,
+    RefetchCallback,
+    QueryRenderInput,
+} from '../types/Query'
+import { ContextType } from '../types/Context'
 
-export const useDataQuery = (query: QueryMap): QueryState => {
+const reduceResponses = (responses: any[], names: string[]) =>
+    responses.reduce(
+        (out, response, idx) => ({
+            ...out,
+            [names[idx]]: response,
+        }),
+        {}
+    )
+
+const fetchData = (
+    context: ContextType,
+    query: QueryMap,
+    signal: AbortSignal
+) => {
     const names = Object.keys(query)
     const requests = names.map(name => query[name])
 
-    const context = useContext(DataContext)
+    const requestPromises = requests.map(q =>
+        context.fetch(q, {
+            signal: signal,
+        })
+    )
+
+    return Promise.all(requestPromises).then(responses =>
+        reduceResponses(responses, names)
+    )
+}
 
+export const useDataQuery = (query: QueryMap): QueryRenderInput => {
+    const context = useContext(DataContext)
     const [state, setState] = useState<QueryState>({ loading: true })
+    const [refetchCount, setRefetchCount] = useState(0)
+    const refetch: RefetchCallback = useCallback(
+        () => setRefetchCount(count => count + 1),
+        []
+    )
 
     useEffect(() => {
-        Promise.all(requests.map(q => context.fetch(q)))
-            .then(responses =>
-                responses.reduce(
-                    (out, response, idx) => ({
-                        ...out,
-                        [names[idx]]: response,
-                    }),
-                    []
-                )
-            )
-            .then(data => setState({ loading: false, data }))
-            .catch(error => setState({ loading: false, error }))
-    }, [])
-
-    return state
+        const controller = new AbortController()
+        const abort = () => controller.abort()
+
+        fetchData(context, query, controller.signal)
+            .then(data => {
+                !controller.signal.aborted && setState({ loading: false, data })
+            })
+            .catch(error => {
+                !controller.signal.aborted &&
+                    setState({ loading: false, error })
+            })
+
+        // Cleanup inflight requests
+        return abort
+    }, [context, query, refetchCount])
+
+    return { refetch, ...state }
 }

services/data/src/types/Query.d.ts

@@ -27,12 +27,18 @@ export interface QueryDefinition extends QueryParameters {
 
 export type QueryMap = Record<string, QueryDefinition>
 
+export type RefetchCallback = () => void
+
 export interface QueryState {
     loading: boolean
     error?: FetchError
     data?: any
 }
 
+export interface QueryRenderInput extends QueryState {
+    refetch: RefetchCallback
+}
+
 /*
 // TODO: Use Union type for better static typeguards in consumer
 export interface QueryStateLoading {
@@ -49,6 +55,4 @@ export interface QueryStateData {
 export type QueryState = QueryStateLoading | QueryStateError | QueryStateData
 */
 
-export type QueryRenderInput = QueryState
-
 export type QueryResult = any