diff --git a/superset-frontend/src/hooks/apiResources/catalogs.ts b/superset-frontend/src/hooks/apiResources/catalogs.ts index 1e6a97b34463b..52037971563b2 100644 --- a/superset-frontend/src/hooks/apiResources/catalogs.ts +++ b/superset-frontend/src/hooks/apiResources/catalogs.ts @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { useCallback, useEffect, useRef } from 'react'; +import { useCallback, useEffect } from 'react'; import useEffectEvent from 'src/hooks/useEffectEvent'; import { api, JsonResponse } from './queryApi'; @@ -68,7 +68,6 @@ export const { export const EMPTY_CATALOGS = [] as CatalogOption[]; export function useCatalogs(options: Params) { - const isMountedRef = useRef(false); const { dbId, onSuccess, onError } = options || {}; const [trigger] = useLazyCatalogsQuery(); const result = useCatalogsQuery( @@ -78,47 +77,28 @@ export function useCatalogs(options: Params) { }, ); - const handleOnSuccess = useEffectEvent( - (data: CatalogOption[], isRefetched: boolean) => { - onSuccess?.(data, isRefetched); - }, - ); - - const handleOnError = useEffectEvent(() => { - onError?.(); - }); - - const refetch = useCallback(() => { - if (dbId) { - trigger({ dbId, forceRefresh: true }).then( - ({ isSuccess, isError, data }) => { + const fetchData = useEffectEvent( + (dbId: FetchCatalogsQueryParams['dbId'], forceRefresh = false) => { + if (dbId && (!result.currentData || forceRefresh)) { + trigger({ dbId, forceRefresh }).then(({ isSuccess, isError, data }) => { if (isSuccess) { - handleOnSuccess(data || EMPTY_CATALOGS, true); + onSuccess?.(data || EMPTY_CATALOGS, forceRefresh); } if (isError) { - handleOnError(); + onError?.(); } - }, - ); - } - }, [dbId, handleOnError, handleOnSuccess, trigger]); + }); + } + }, + ); + + const refetch = useCallback(() => { + fetchData(dbId, true); + }, [dbId, fetchData]); useEffect(() => { - if (isMountedRef.current) { - const { requestId, isSuccess, isError, isFetching, data, originalArgs } = - result; - if (!originalArgs?.forceRefresh && requestId && !isFetching) { - if (isSuccess) { - handleOnSuccess(data || EMPTY_CATALOGS, false); - } - if (isError) { - handleOnError(); - } - } - } else { - isMountedRef.current = true; - } - }, [result, handleOnSuccess, handleOnError]); + fetchData(dbId, false); + }, [dbId, fetchData]); return { ...result, diff --git a/superset-frontend/src/hooks/apiResources/schemas.test.ts b/superset-frontend/src/hooks/apiResources/schemas.test.ts index 78df4f850e1e0..e63a3b97af96e 100644 --- a/superset-frontend/src/hooks/apiResources/schemas.test.ts +++ b/superset-frontend/src/hooks/apiResources/schemas.test.ts @@ -88,7 +88,7 @@ describe('useSchemas hook', () => { })}`, ).length, ).toBe(1); - expect(onSuccess).toHaveBeenCalledTimes(2); + expect(onSuccess).toHaveBeenCalledTimes(1); act(() => { result.current.refetch(); }); @@ -100,7 +100,7 @@ describe('useSchemas hook', () => { })}`, ).length, ).toBe(1); - expect(onSuccess).toHaveBeenCalledTimes(3); + expect(onSuccess).toHaveBeenCalledTimes(2); expect(result.current.data).toEqual(expectedResult); }); @@ -149,28 +149,36 @@ describe('useSchemas hook', () => { }, ); - await waitFor(() => expect(result.current.data).toEqual(expectedResult)); + await waitFor(() => + expect(result.current.currentData).toEqual(expectedResult), + ); expect(fetchMock.calls(schemaApiRoute).length).toBe(1); - expect(onSuccess).toHaveBeenCalledTimes(2); + expect(onSuccess).toHaveBeenCalledTimes(1); rerender({ dbId: 'db2' }); - await waitFor(() => expect(result.current.data).toEqual(expectedResult2)); + await waitFor(() => + expect(result.current.currentData).toEqual(expectedResult2), + ); expect(fetchMock.calls(schemaApiRoute).length).toBe(2); - expect(onSuccess).toHaveBeenCalledTimes(4); + expect(onSuccess).toHaveBeenCalledTimes(2); rerender({ dbId: expectDbId }); - await waitFor(() => expect(result.current.data).toEqual(expectedResult)); + await waitFor(() => + expect(result.current.currentData).toEqual(expectedResult), + ); expect(fetchMock.calls(schemaApiRoute).length).toBe(2); - expect(onSuccess).toHaveBeenCalledTimes(5); + expect(onSuccess).toHaveBeenCalledTimes(2); // clean up cache act(() => { store.dispatch(api.util.invalidateTags(['Schemas'])); }); - await waitFor(() => expect(fetchMock.calls(schemaApiRoute).length).toBe(3)); + await waitFor(() => expect(fetchMock.calls(schemaApiRoute).length).toBe(4)); expect(fetchMock.calls(schemaApiRoute)[2][0]).toContain(expectDbId); - await waitFor(() => expect(result.current.data).toEqual(expectedResult)); + await waitFor(() => + expect(result.current.currentData).toEqual(expectedResult), + ); }); test('returns correct schema list by a catalog', async () => { @@ -201,7 +209,7 @@ describe('useSchemas hook', () => { await waitFor(() => expect(fetchMock.calls(schemaApiRoute).length).toBe(1)); expect(result.current.data).toEqual(expectedResult3); - expect(onSuccess).toHaveBeenCalledTimes(2); + expect(onSuccess).toHaveBeenCalledTimes(1); rerender({ dbId, catalog: 'catalog2' }); await waitFor(() => expect(fetchMock.calls(schemaApiRoute).length).toBe(2)); diff --git a/superset-frontend/src/hooks/apiResources/schemas.ts b/superset-frontend/src/hooks/apiResources/schemas.ts index 51e8528ce5d33..e60e62a7fbcb1 100644 --- a/superset-frontend/src/hooks/apiResources/schemas.ts +++ b/superset-frontend/src/hooks/apiResources/schemas.ts @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { useCallback, useEffect, useRef } from 'react'; +import { useCallback, useEffect } from 'react'; import useEffectEvent from 'src/hooks/useEffectEvent'; import { api, JsonResponse } from './queryApi'; @@ -72,7 +72,6 @@ export const { export const EMPTY_SCHEMAS = [] as SchemaOption[]; export function useSchemas(options: Params) { - const isMountedRef = useRef(false); const { dbId, catalog, onSuccess, onError } = options || {}; const [trigger] = useLazySchemasQuery(); const result = useSchemasQuery( @@ -82,62 +81,34 @@ export function useSchemas(options: Params) { }, ); - const handleOnSuccess = useEffectEvent( - (data: SchemaOption[], isRefetched: boolean) => { - onSuccess?.(data, isRefetched); + const fetchData = useEffectEvent( + ( + dbId: FetchSchemasQueryParams['dbId'], + catalog: FetchSchemasQueryParams['catalog'], + forceRefresh = false, + ) => { + if (dbId && (!result.currentData || forceRefresh)) { + trigger({ dbId, catalog, forceRefresh }).then( + ({ isSuccess, isError, data }) => { + if (isSuccess) { + onSuccess?.(data || EMPTY_SCHEMAS, forceRefresh); + } + if (isError) { + onError?.(); + } + }, + ); + } }, ); - const handleOnError = useEffectEvent(() => { - onError?.(); - }); - useEffect(() => { - if (dbId) { - trigger({ dbId, catalog, forceRefresh: false }).then( - ({ isSuccess, isError, data }) => { - if (isSuccess) { - handleOnSuccess(data || EMPTY_SCHEMAS, true); - } - if (isError) { - handleOnError(); - } - }, - ); - } - }, [dbId, catalog, handleOnError, handleOnSuccess, trigger]); + fetchData(dbId, catalog, false); + }, [dbId, catalog, fetchData]); const refetch = useCallback(() => { - if (dbId) { - trigger({ dbId, catalog, forceRefresh: true }).then( - ({ isSuccess, isError, data }) => { - if (isSuccess) { - handleOnSuccess(data || EMPTY_SCHEMAS, true); - } - if (isError) { - handleOnError(); - } - }, - ); - } - }, [dbId, catalog, handleOnError, handleOnSuccess, trigger]); - - useEffect(() => { - if (isMountedRef.current) { - const { requestId, isSuccess, isError, isFetching, data, originalArgs } = - result; - if (!originalArgs?.forceRefresh && requestId && !isFetching) { - if (isSuccess) { - handleOnSuccess(data || EMPTY_SCHEMAS, false); - } - if (isError) { - handleOnError(); - } - } - } else { - isMountedRef.current = true; - } - }, [catalog, result, handleOnSuccess, handleOnError]); + fetchData(dbId, catalog, true); + }, [dbId, catalog, fetchData]); return { ...result,