[GitHub] [superset] ktmud commented on pull request #13218: refactor: Introduce api resource hooks, fetch owners for chart errors

[GitBox <gi...@apache.org>]

ktmud commented on pull request #13218:
URL: https://github.com/apache/superset/pull/13218#issuecomment-781697520


   Hmm, interesting concept. We definitely lack some handy React hooks to fetch API resources. But I'd also like to point out the [`makeApi`](https://github.com/apache-superset/superset-ui/blob/ef823365c06a4b2c33b0f5541cd306b227b2bc0f/packages/superset-ui-core/src/query/api/v1/makeApi.ts#L80) util already supports a `processResponse` option that [allows you to transform the response.](https://github.com/apache-superset/superset-ui/blob/482da217938bcbd70db8b79117abacf00f8037e2/packages/superset-ui-core/test/query/api/v1/makeApi.test.ts#L71-L83)
   
   It even infers the return type for you:
   
   <img src="https://user-images.githubusercontent.com/335541/108431842-4c3fa680-71f8-11eb-9d49-0097afcd2d91.png" width="600">
   
   So we may be able to simplify the hooks a little bit. For example, `useChartOwnerNames` can be rewritten as:
   
   ```ts
   function useAsyncData<T>(promise: Promise<T>) {
     const [result, setResult] = useState<{ data?: T; error": any }>();
     useEffect(() => {
       promise.then(data => setResult({ data }))
         .catch(error => setResult({ error }));
     }, [promise]);
     return { data, error };
   }
   
   function useChartOwnerNames(chartId: number) {
     const fetchChartOwnerNames = makeApi({
       method: 'GET',
       requestType: 'rison',
       endpoint: `/api/v1/chart/${chartId}`,
       processResponse: ({ result } : { result?: Chart }) =>
         results?.owners.map(owner =>
           `${owner.first_name || ''} ${owner.last_name || ''}`.strip() || owner.id) || [],
     );
     return useAsyncData(fetchChartOwnerNames({
       columns: ['owners.first_name', 'owners.last_name', 'owners.id']
     }));
   }
   ```
   
   If we add support for url match params for `endpoint` and default base payload (i.e. make column selection part of the base payload), we may even move `fetchChartOwnerNames` outside of the hook and have it become a general async API caller (and potentially export for other places to use).
   
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org