vercel / swr

React Hooks for Data Fetching
https://swr.vercel.app
MIT License
30.5k stars 1.22k forks source link

Support clarifying SWRResponse type to do type narrowing SWRResponse["data"] by flags(isSuccess) or union type or any other ways please #2482

Open manudeli opened 1 year ago

manudeli commented 1 year ago

How can we use useSWR like below to do type narrowing for data?

const SWRComp = () => {
  const swrQuery = useSWR(...)

  if(swrQuery.data !== undefined){
    return <>{swrQuery.data}</>
  }
}

but in this case, if fetcher to put in useSWR return undefined, how can we do guarantee fetcher's returning promise is success? you can check it this edge case. data is never.

image

I'm sad that there is no isSuccess flag in useSWR because of this problem.

Could you make it TypeScript example to guide us avoiding this problem?

In this example(https://swr.vercel.app/examples/basic) in official guide in JavaScript file. I can understand it because of JavaScript will accept it. but in TypeScript is not!🥲

In JavaScript, no type error

import React from "react";
import useSWR from "swr";

const fetcher = (url) => fetch(url).then((res) => res.json());

export default function App() {
  const { data, error, isLoading } = useSWR(
    "https://api.github.com/repos/vercel/swr",
    fetcher
  );

  if (error) return "An error has occurred.";
  if (isLoading) return "Loading...";
  return (
    <div>
      <h1>{data.name}</h1>
      <p>{data.description}</p>
      <strong>👁 {data.subscribers_count}</strong>{" "}
      <strong>✨ {data.stargazers_count}</strong>{" "}
      <strong>🍴 {data.forks_count}</strong>
    </div>
  );
}

In TypeScript, Type error will be occured

import React from 'react'
import useSWR from 'swr'

const fetcher = (
  url: string
) => fetch(url).then((res) => res.json()) as Promise<{
  name: string
  description: string
  subscribers_count: number
  stargazers_count: number
  forks_count: number
}>

export default function App() {
  const { data, error, isLoading } = useSWR(
    'https://api.github.com/repos/vercel/swr',
    fetcher
  )

  if (error) return 'An error has occurred.'
  if (isLoading) return 'Loading...'
  return (
    <div>
      <h1>{data.name}</h1>
      <p>{data.description}</p>
      <strong>👁 {data.subscribers_count}</strong>{' '}
      <strong>✨ {data.stargazers_count}</strong>{' '}
      <strong>🍴 {data.forks_count}</strong>
    </div>
  )
}
image

How do we have to avoid this type error situation?

Originally posted by @manudeli in https://github.com/vercel/swr/issues/2175#issuecomment-1457608985

What I expected: union return type of useSWR containing flag expressing Promise's status

in @tanstack/react-query, they support union return type of useQuery, so we can do type narrowing by isSuccess, isLoading, isError. but in swr, it's not.

type narrowing by isSuccess in @tanstack/react-query

const ReactQueryComp = () => {
  const reactQuery = useQuery(...)

  if (reactQuery.isSuccess){ // even if data is undefined, when Promise fulfilled, we can use it! not like swr
    return <>{reactQuery.data}</>
  }
}

type narrowing by isLoading, isError in @tanstack/react-query

const ReactQueryComp = () => {
  const reactQuery = useQuery(...)

  if (reactQuery.isLoading) return <>laoding...</>
  if (reactQuery.isError) return <>{reactQuery.error}</> 
  return <>{reactQuery.data}</> // data type narrowed by isLoading, isError
}
tobbbe commented 1 year ago

I dont think we need isSuccess? Because if x.error has value or x.isValidating/x.isLoading is true. Then we can assume isSuccess is false right?

Could this do?

export type SWRResponse<Data = any, Error = any, Config = any> = {
  data: undefined
  error: undefined
  mutate: undefined
  isValidating: true
  isLoading: BlockingData<Data, Config> extends true ? false : true
} |
{
  data: undefined
  error: undefined
  mutate: undefined
  isValidating: false
  isLoading: BlockingData<Data, Config> extends true ? false : true
} |
{
  data: undefined
  error: undefined
  mutate: undefined
  isValidating: true
  isLoading: BlockingData<Data, Config> extends true ? false : false
} |
{
  data: undefined
  error: undefined
  mutate: undefined
  isValidating: false
  isLoading: BlockingData<Data, Config> extends true ? false : false
} |
{
  /**
   * The error object thrown by the fetcher function.
  */
  error: Error
  data: undefined
  mutate: undefined
  isValidating: false
  isLoading: false
}
|
{
  /**
   * The returned data of the fetcher function.
   */
  data: BlockingData<Data, Config> extends true ? Data : Data
  error: undefined
  mutate: KeyedMutator<Data>
  isValidating: false
  isLoading: false
}
manudeli commented 1 year ago

I dont think we need isSuccess? Because if x.error has value or x.isValidating/x.isLoading is true. Then we can assume isSuccess is false right?

Then don't we need to provide data type narrowing in this below picture? but why data can be undefined typescriptly? Then you mean you want to make SWRResponse can do type narrowing data by only error, isValidating, isLoading without isSuccess flag? like below picture?

type narrowing by isLoading, error

image

because of cached data by swr, we don't have to type narrowing by isValidating. during isValidating, data can be existed by cache.

I expected data type need to be just "success" without undefined typescriptly. anyway in this case too without isSuccess flag.

tobbbe commented 1 year ago

Yea true! x.data can be both undefined and some value while isValidating.

Im not sure why data can be undefined in that case 🤔 useSWR hook must do something. Ill keep looking!

mnik01 commented 1 year ago

facing a bug

image image

In data is possibly undefined. But fetcher returns a Promise which is object not undefined

UPD: I guess this is because of isRevalidating. Fixed if make my condition to check if user there.

if (!user) return spinner;
marcelocarmona commented 1 year ago

@tobbbe @manudeli I'm interested in this feature I like what I see in https://github.com/vercel/swr/pull/2175 Do you know if there is something missing here or edge cases that are not covered?

Also for more context about this feature, this is a good comparison with react-query https://news.ycombinator.com/item?id=33929553