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

Thu, 18 Feb 2021 15:15:32 -0800 


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:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to