suddjian commented on a change in pull request #13218: URL: https://github.com/apache/superset/pull/13218#discussion_r580711705
########## File path: superset-frontend/src/common/hooks/apiResources/apiResources.ts ########## @@ -0,0 +1,181 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { makeApi } from '@superset-ui/core'; +import { useEffect, useMemo, useRef, useState } from 'react'; + +export enum ResourceStatus { + LOADING = 'loading', + COMPLETE = 'complete', + ERROR = 'error', +} + +/** + * An object containing the data fetched from the API, + * as well as loading and error info + */ +export type Resource<T> = LoadingState | CompleteState<T> | ErrorState; + +// Trying out something a little different: a separate type per status. +// This should let Typescript know whether a Resource has a result or error. +// It's possible that I'm expecting too much from Typescript here. +// If this ends up causing problems, we can change the type to: +// +// export type Resource<T> = { +// status: ResourceStatus; +// result: null | T; +// error: null | Error; +// } + +type LoadingState = { + status: ResourceStatus.LOADING; + result: null; + error: null; +}; + +type CompleteState<T> = { + status: ResourceStatus.COMPLETE; + result: T; + error: null; +}; + +type ErrorState = { + status: ResourceStatus.ERROR; + result: null; + error: Error; +}; + +const initialState: LoadingState = { + status: ResourceStatus.LOADING, + result: null, + error: null, +}; + +/** + * A general-purpose hook to fetch the response from an endpoint. + * Returns the full response body from the API, including metadata. + * + * Note: You likely want {useApiV1Resource} instead of this! + * + * TODO Store the state in redux or something, share state between hook instances. + * + * TODO Include a function in the returned resource object to refresh the data. + * + * A core design decision here is composition > configuration, + * and every hook should only have one job. + * Please address new needs with new hooks if possible, + * rather than adding config options to this hook. + * + * @param endpoint The url where the resource is located. + */ +export function useApiResourceFullBody<RESULT>( + endpoint: string, +): Resource<RESULT> { + const [resource, setResource] = useState<Resource<RESULT>>(initialState); Review comment: @eschutho Moving state into Redux (or any other sort of provider) could be done by only modifying the internal logic of this hook. Specifically, by calling Redux's `useSelector` here instead of `useState`, and using actions instead of calling `setResource`. You know what maybe I'll just do that now instead of talking about it 😁 ---------------------------------------------------------------- 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]
