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

feat(types): clarify SWRResponse type #2175

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

sanding0
Copy link

@sanding0 sanding0 commented Sep 28, 2022

fix #2482

This PR can clarify SWRResponse type
allows you to asser response types in TypeScript

function App () {
  const {data,isLoading,error,isValidating} = useSWR('/api')
  if(error){
      return (....)
   }
  if(isLoading){
    return (....)
   }

  //data will not be undefined
  return <div>{data.xxx}</div>
}

@sanding0 sanding0 closed this Sep 28, 2022
@tobbbe
Copy link

tobbbe commented Jan 26, 2023

@sanding0 do you know why wasnt this merged? it would be great!! Or is there a workaround?

@manudeli
Copy link

manudeli commented Mar 7, 2023

@sanding0 do you know why wasnt this merged? it would be great!! Or is there a workaround?

@tobbbe I think so too. Isn't this change better than now? Why you(@sanding0) closed?

I expected SWRResponse will work like union Response type like @tanstack/react-query but it isn't. I'm so sad about this SWRResponse's data as undefined.🥲

@shuding @huozhi How about this idea?

@sanding0
Copy link
Author

sanding0 commented Mar 7, 2023

cause there are some interface implements usage that I can't rewrite to union types
@tobbbe @manudeli

@manudeli
Copy link

manudeli commented Mar 7, 2023

@shuding @huozhi Then, Do we need to use useSWR like below to type narrowing?

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?

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

@shuding @huozhi, 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?

@tobbbe
Copy link

tobbbe commented Mar 7, 2023

cause there are some interface implements usage that I can't rewrite to union types @tobbbe @manudeli

Which ones? Can we help? 😊

@manudeli
Copy link

manudeli commented Mar 7, 2023

@tobbbe @sanding0 I made new issue #2482 relative with this Pull Request.

@sanding0
Copy link
Author

sanding0 commented Mar 7, 2023

cause there are some interface implements usage that I can't rewrite to union types @tobbbe @manudeli

Which ones? Can we help? 😊
sorry, it seems like not the implements.
it's been some time since this PR closed.
I will check it later. thanks a lot!

@sanding0 sanding0 reopened this Mar 7, 2023
@manudeli
Copy link

manudeli commented Mar 7, 2023

@sanding0 If you want, Could you link this issue to close it after this PR merging?
#2482

You can edit description (adding fix https://github.com/vercel/swr/issues/2482) of Pull Request or link it in Development section
image

@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?

@manudeli
Copy link

manudeli commented Mar 7, 2023

@tobbbe Could we go #2482 . I just don't want to make this conversation duplicately.

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