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

Why object instead of array as return value? #9

Closed
sergiodxa opened this issue Oct 28, 2019 · 9 comments
Closed

Why object instead of array as return value? #9

sergiodxa opened this issue Oct 28, 2019 · 9 comments
Labels
discussion Discussion around current or proposed behavior

Comments

@sergiodxa
Copy link
Contributor

The return value of the React state hook is an array, used as:

const [state, setState] = React.useState(false);

This way you could use any name for the state and the setter.

In useSWR is an object:

const { data, error } = useSWR(URL, fetch)

Is there a special reason why it's not an array to? Even one of the examples in the website needs to rename data.

const { data: user } = useSWR('/api/user')
const { data: projects } = useSWR(
  () => '/api/projects?uid=' + user.id
);

This could have be

const [ user ] = useSWR('/api/user')
const [ projects ] = useSWR(
  () => '/api/projects?uid=' + user.id
);

And if you need other values:

const [data, error, isValidating, revalidate] = useSWR(URL, fetch)

I understand this could be a annoying if you want to use revalidate without isValidating or error but I think is more common to need multiple data with different names than reading those other values in isolation.

@pacocoursey pacocoursey added the discussion Discussion around current or proposed behavior label Oct 28, 2019
@jamesb3ll
Copy link

jamesb3ll commented Oct 29, 2019

I understand this could be a annoying if you want to use revalidate without isValidating or error but I think is more common to need multiple data with different names than reading those other values in isolation.

Exactly, const [users, _, _unnecessary, _variables, revalidate] = useSWR('/users') isn't a good API.

I think this would be a better solution:

const [users, { error, isValidating, revalidate }] = useSWR('/users', fetch)

As you can deconstruct error, isValidating and revalidate as needed.

Although I think the current API isn't bad at all,const { data: users } = useSWR('/users') is very readable...

@sergiodxa
Copy link
Contributor Author

You don’t need to name ignored values, this should work:

const [data, , , revalidate ] = useSWR(URL, fetch)

@pacocoursey
Copy link
Contributor

pacocoursey commented Oct 29, 2019

This is really a matter of preference. Neither of the worst case of both approaches:

const [data, , , revalidate] = useSWR()
const { data: newName, revalidate } = useSWR()

feel particularly difficult to use.

An array approach feels more like a "hook". But as we develop the API and potentially add more information to the response object, the array approach may become unmanageable.

I personally don't find it easy to have to remember how many commas to type, or which position in the array a variable is in.

@cryptiklemur
Copy link
Contributor

cryptiklemur commented Oct 29, 2019

Not that I'm advocating for it, as I don't care for the pattern, but there's also the possibility of returning both.

@paul-vd
Copy link

paul-vd commented Oct 29, 2019

You don’t need to name ignored values, this should work:

const [data, , , revalidate ] = useSWR(URL, fetch)

that looks horrible though and uncomprehensive for someone seeing the code for the first time.

@sztadii
Copy link
Contributor

sztadii commented Oct 31, 2019

React team implements react hooks in this way, cause always we will need value and its setter and naming will be not important. The problem is coming when we have many optional params like here. So I think your decision was good 👍

@pierreburel
Copy link

I like the way some libs like react-i18next do by returning an array with properties. This way you can decide which one to use depending of your usage : const [users] = useSWR() or const { data, revalidate } = useSWR().

See https://github.com/i18next/react-i18next/blob/master/src/useTranslation.js#L81-L84

@conradreuter
Copy link

With @apollo/react-hooks I don't bother with destructuring the return value:

cost fooQuery = useQuery(...)

// extract what we need, but be aware that `query.data` might not exist
const foo = query.data?.foo

if (fooQuery.error) {
  return <Error>{...}</Error>
}
if (fooQuery.loading) {
  return <Loader>{...}</Loader>
}
return <h1>Hello, {foo}</h1>

That being said, I would much more prefer an object as the return value.

@pacocoursey
Copy link
Contributor

pacocoursey commented Dec 5, 2019

Closing as I think we've reached a happy conclusion :)

Thanks everyone for the discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Discussion around current or proposed behavior
Projects
None yet
Development

No branches or pull requests

8 participants