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

[manager-components]: refactor useIceberg hooks #13543

Open
1 task done
tristanwagner opened this issue Oct 10, 2024 · 3 comments
Open
1 task done

[manager-components]: refactor useIceberg hooks #13543

tristanwagner opened this issue Oct 10, 2024 · 3 comments

Comments

@tristanwagner
Copy link
Contributor

tristanwagner commented Oct 10, 2024

Have you already checked if a similar item is present on manager-components?

  • Yes, I have already checked the existing components/hooks/utils.

What do you expect from this request?

Hooks useIcebergV2, useIcebergV6, useResourcesV6..

Description

Right now I feel like these hooks that extends the useInfiniteQuery calls provide not enough value and also are very restraining for the end user of the hook, I propose that we update the way of exposing this kind of hooks in a more generic and versatile way, that will allow developers to be able to use more of the react-query features for their usecases.

To illustrate by an example, to me an updated version of the hook would look like that:

export function useResourcesIcebergV2<T = unknown>({
  route,
  pageSize = defaultPageSize,
  ...options
}: IcebergFetchParamsV2 & IcebergV2Hook) {
  return useInfiniteQuery({
    initialPageParam: null,
    queryFn: ({ pageParam }) =>
      fetchIcebergV2<T>({ route, pageSize, cursor: pageParam }),
    getNextPageParam: (lastPage) => lastPage.cursorNext,
    select: (data) => { ...data, flattenData},
    ...options,
  });
}

If you really want to flatten the data in there (to me that should not be the place where you should do this but maybe I'm wrong) then you should do it using the select option from useInfiniteQuery and avoid at all cost using a useState in this place.

I want to add that the way the cursor are designed in the APIs, if you use refetch or like refetchInterval feature, when refetching the data, a new cursor will be provided by the API even tho your actual response payload didn't changed so your data object returned will have changed since it contains pageParams that will contain the current cursor, causing unwanted rerenders in your UI, you can avoid this by omitting the cursor in the data.pageParams using select.

To me it also applies to any other hook that will wrap a useQuery or useInfiniteQuery call: you should be able to pass additional options and also retrieve the returns from the call.

Where do you expect to use this?

any

Do you have mock-up?

No response

When do you expect this to be delivered?

ASAP

Additional Information

No response

@tristanwagner tristanwagner changed the title [manager-components]: <TITLE> [manager-components]: refactor useIceberg hooks Oct 10, 2024
@tristanwagner
Copy link
Contributor Author

linked to #13513 #13331

@anooparveti
Copy link
Contributor

The change is no longer needed

From the requester. @pauldkn

@pauldkn
Copy link
Contributor

pauldkn commented Nov 20, 2024

The change is no longer needed

From the requester. @pauldkn

@anooparveti the #13513 is no longer needed because this one exist and cover a more complete scope on this hook.
I closed the smallest one but this one should still be open

@pauldkn pauldkn reopened this Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants