Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Map and EDA: retrofit react-query's useQuery in place of usePromise #1103

Closed
bobular opened this issue May 31, 2024 · 1 comment · Fixed by #1228
Closed

Map and EDA: retrofit react-query's useQuery in place of usePromise #1103

bobular opened this issue May 31, 2024 · 1 comment · Fixed by #1228

Comments

@bobular
Copy link
Member

bobular commented May 31, 2024

I think I have a plan to do this. It's related to the timeslider stepping forward/backward functionality we might still introduce (if the user has a floating plot active). Would appreciate feedback.

Overview

In all our XxxVisualization plugins, replace

  const data = usePromise(useCallback(() => getData(dep1, dep2, dep3), [ dep1, dep2, dep3 ]));

with

  const data = useCachedPromise(() => getData(dep1, dep2, dep3), [ dep1, dep2, dep3 ]);

where the return value is exactly the same as before ({ value, error, pending });

Implementation

useCachedPromise would be implemented like this (not tested)

import { useMemo } from 'react';
import { v4 as uuidv4 } from 'uuid';
import { useQuery } from 'react-query';
import { PromiseHookState } from './promise';

export function useCachedPromise<T>(task: () => Promise<T>, deps: any[]): PromiseHookState<T> {
  // generate a serialisable key for react-query from a mix of data and class/function dependencies
  const uniqueKey = useMemo(() => uuidv4(), deps);
  // Using useQuery from react-query with the unique key
  const { data, error, isLoading } = useQuery({
    queryKey: ['useCachedPromise', uniqueKey],
    queryFn: task,
  });

  // Mapping the state from useQuery to PromiseHookState<T>
  const state: PromiseHookState<T> = {
    value: data,
    pending: isLoading,
    error: error,
  };

  return state;
}

Details

We may want to use isFetching instead of isLoading or a combination of it and isLoading. Or maybe status === 'loading'. See here. We would need to look into the nitty-gritty of react-query to make sure we get this right, and consider turning off background refetching? See below.

QueryClientProvider

EDA will need this wrapped too (see packages/libs/eda/src/lib/map/MapVeuContainer.tsx) - at least the VisualizationsContainer

@dmfalke
Copy link
Member

dmfalke commented Jun 4, 2024

I think we should just have usePromise use useQuery. We would have to include QueryClientProvider in a more centralized place, such as https://github.com/VEuPathDB/web-monorepo/blob/main/packages/libs/wdk-client/src/Core/Root.tsx.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants