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

[proposal] making hooks return object instead of array for readable typeguard #228

Open
Liooo opened this issue Dec 22, 2021 · 2 comments
Open

Comments

@Liooo
Copy link

Liooo commented Dec 22, 2021

Has there been a discussion about making hooks return object instead of array? The benefits behind that being:

// since typeguard doesn't work with destructuring assignment like this:
const App = () => {
  const [match, params] = useRoute<{id: string}>('/path/:id')
  if (!match) {
    return <div>not present</div>
  }

  return <div>{params.id}</div> // <- compiler think params may be null
}

// we have to write something like this:
const App = () => {
  const route = useRoute<{id: string}>('/path/:id')
  if (!route[0]) {
    return <div>not present</div>
  }

  return <div>{route[1].id}</div> 
  // Not easily understandable what this index accessed variable is, at a glance. Though of course can be avoided
  // by assigning another variable with proper name or something, why not: ↓
}

// make it return object like this:
const App = () => {
  const route = useRoute<{id: string}>('/path/:id')
  if (!route.match) {
    return <div>not present</div>
  }

  return <div>{route.params.id}</div>
}

// by doing the same for useLocation, we don't have to introduce unused variable when using only one of the return values:
const App = () => {
  const [_location, setLocation] = useLocation() // <- unused variable

  return <button onClick={(e)=>setLocation('somepath')}>
}

const App = () => {
  const {setLocation} = useLocation() // <- no unused variable

  return <button onClick={(e)=>setLocation('somepath')}>
}

I'm assuming the upside of the current implementation is

  • closer signature to well known hooks like useState()
  • in useRoute case, some may think receiving array makes it more obvious that the second item params depends on the first item match, especially for non typescript users

Since this will cause a breaking change and even the subtle benefit isn't obvious for everybody, it might not get enough 👍s. But probably it's still worth discussing once for ppl wondering the same thing later.

(our workaround so far is using a wrapper hook that make it return an object, and also redefining most of return types)

@cbbfcd
Copy link
Contributor

cbbfcd commented Dec 27, 2021

Since this will cause a breaking change

Indeed, this will have a big impact.

@Liooo
Copy link
Author

Liooo commented Dec 30, 2021

@cbbfcd

Thanks for the comment, and yea I guess so!
It's a valid decision to not make the change due to ROI, still wanted to make sure if there's any particular intention behind the current design and what other typescript users are feeling about etc.

@molefrog molefrog added the V3 Features that won't be released in v2, but planned for the next major release label Sep 14, 2023
@molefrog molefrog removed the V3 Features that won't be released in v2, but planned for the next major release label Mar 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants