ktmud commented 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 possible. `makeApi` naturally supports this, while the composable hooks approach may lead to more fragmentary code. There are also places where a general async function that returns a promise would be preferred, as 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). It we are thinking of getting fancy with deduplication and caching by entity, would something more complete/mature like [REST Hooks](https://resthooks.io/docs/api/README) make more sense? ---------------------------------------------------------------- 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]
