Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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 this issue Mar 7, 2023 · 5 comments · May be fixed by #2175

Comments

@manudeli
Copy link

manudeli commented Mar 7, 2023

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 #2175 (comment)

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
}
@manudeli manudeli linked a pull request Mar 7, 2023 that will close this issue
@manudeli manudeli changed the title Support clarifying SWRResponse type to do type narrowing Support clarifying SWRResponse type to do type narrowing please Mar 7, 2023
@manudeli manudeli changed the title Support clarifying SWRResponse type to do type narrowing please Support isSuccess flag or other way clarifying SWRResponse type to do type narrowing please Mar 7, 2023
@manudeli manudeli changed the title Support isSuccess flag or other way clarifying SWRResponse type to do type narrowing please Support clarifying SWRResponse type to do type narrowing please Mar 7, 2023
@manudeli manudeli changed the title Support clarifying SWRResponse type to do type narrowing please Support clarifying SWRResponse type to do type narrowing SWRResponse["data"] please Mar 7, 2023
@manudeli manudeli changed the title Support clarifying SWRResponse type to do type narrowing SWRResponse["data"] please Support clarifying SWRResponse type to do type narrowing SWRResponse["data"] by flags(isSuccess) or union type or any other ways please Mar 7, 2023
@tobbbe
Copy link

tobbbe commented Mar 7, 2023

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
Copy link
Author

manudeli commented Mar 7, 2023

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
Copy link

tobbbe commented Mar 7, 2023

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
Copy link

mnik01 commented Apr 2, 2023

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
Copy link

@tobbbe @manudeli I'm interested in this feature I like what I see in #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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants