ktmud edited a comment on pull request #13218: URL: https://github.com/apache/superset/pull/13218#issuecomment-781745672
@suddjian I'm not against having hooks that do extra cool stuff. I think a powerful API resource manager would be very useful. I do, however, believe there's value in hiding API endpoints and pre-defined transformations from end users so a centralized, strongly typed API catalog like [this](https://github.com/apache-superset/superset-ui/blob/d985ba3711363054662317c87fe64924091db5fb/packages/superset-ui-core/src/query/api/v1/index.ts#L27) would be easier to create. `makeApi` naturally supports this, while the composable hooks approach may lead to more fragmentary code. There are also places where a general async function would be preferred, because hooks can only be used in functional components. Re: dedupping requests and caching results, I do have some simple solution that I used in our internal Superset: ```ts const promiseCache: Record< string, { timestamp: number; promise: Promise<any> } > = {}; /** * Cache a promise maker, make sure it doesn't fire multiple times * @param api - the promise generator. * @param expires - cache TTL in seconds. * @param keyPrefix - prefix for the cache key. */ export function cached<T extends (...args: any[]) => Promise<any>>( api: T, { expires = 300, keyPrefix = '', }: { expires?: number; keyPrefix?: string; }, ) { return function wrapped(...args: Parameters<T>): ReturnType<T> { const cacheKey = `${keyPrefix}-${JSON.stringify(args)}`; if ( cacheKey in promiseCache && promiseCache[cacheKey].timestamp + expires * 1000 > Date.now() ) { return promiseCache[cacheKey].promise as ReturnType<T>; } const promise = api(...args); promiseCache[cacheKey] = { promise, timestamp: Date.now(), }; promise.catch(() => { // don't cache failed responses delete promiseCache[cacheKey]; }); return promise as ReturnType<T>; }; } export const getValidMetrics = cached( makeApi<MinervaValidationApiPaylod, ControlValues['metrics']>({ method: 'GET', endpoint: '/minerva/valid_metrics', }), { keyPrefix: 'metrics' }, ); ``` It's a very simple wrapper on the generated API caller and handles both caching and deduplication (making the same API call will not fire a request twice). For now, this is probably good enough for Superset, but if we are thinking of getting fancy with deduplication and caching by entity, I would also like to see us evaluating some existing solutions out there such as [REST Hooks](https://resthooks.io/docs/api/README) before committing to create our own solution for things like shared storage. ---------------------------------------------------------------- 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]
