Add useToggle hook to react-hooks

[BPScott]

Description

Standardize some of the layout in the react-hooks package as we were sometimes using named and sometimes using default exports - now we always used named.

Add two new hooks that I've found very useful in polaris-react and have came in handy when writing stuff in web.

useToggle() is a little hook that abstracts away creating a boolean state which crops up a lot when you have to deal with toggling the visibility of modals and popovers and other conditional rendering. It's a pretty light touch but I've found myself having to reach for it quite often as many of these boolean states are needed in meaty components.

See the readme file for a usage demo.

Type of change

• react-hooks Minor: New feature (non-breaking change which adds functionality)

Checklist

• I have added a changelog entry, prefixed by the type of change noted above

---

EDIT: Disregard a

[marutypes]

Looks great, one API suggestion but I'm fine with this as is if it's hated

[marutypes]

I know it's not how we usually structure hook APIs, but the other functions feel so much like nice-to-haves that I wonder if it would be nicer DX to have a single hook which exposes the "bonus" callbacks as a third return value?

const [active, toggle] = useToggle();
const [forceyActive, forceyToggle, {setTrue, setFalse}] = useToggle();

(I think this is also helpful as a naming change since at first I was confused what was "forcible" about the toggle with setters)

[BPScott]

Sometimes you want just the forcing functions and not the toggle though, and then things start to look a little unintuitive:

// weird
const [forceyActive, , {setTrue, setFalse}] = useToggle();
// slightly less weird, but still weird
const [forceyActive, _, {setTrue, setFalse}] = useToggle();

I think this crops up often enough that lots of people will see it and be thrown by that empty slot in the array deconstruction.

I agree having the two hooks is annoying, but I think "use this hook if you just want to toggle, use that hook if you want to force the value to either true or false" is easier to teach and more readable than trying to squeeze both APIs into a single hook.

---

Alternative plan is we could do some renaming to frame the one that returns the object as the "default" hook that people reach for:

• rename useToggle() to useSimpleToggle()
• rename useForcibleToggle() to useToggle()

That might stop people wondering what "forcible" means. Would that help? (Or does that open a different bag of worms with people wondering what is simple and why it wasn't that way in the first place)

[marutypes]

It seems to me that the only reason to have the forcible option in the first place is because the useToggle API takes away the power that useState gives you.

One solution could just be having useToggle be a simple hook that wraps a useState call and gives you the toggle function, and then have useToggleState be the hook that both creates a state and gives you the toggle. Then you aren't being opionated about having "forcible" callbacks, you're just giving them a maximally flexible API, and a slightly more opinionated one built on that.

example

When you don't need regular state setting at all use useToggleState for convenience
(or you can do useToggle(useState(false))

  const [shown, toggleShown] = useToggleState();

  return (
    <Button onClick={toggleShown}>toggle thing be shown</Button>

When you need both toggling and regular setting just wrap your state setter

const [thingShown, setThingShown] = useState(false);
const toggle = useToggle(thingShown, setThingShown);

return (
<>
 <Button onClick={() => setThingShown(true)}>set it open</Button>
 <Button onClick={toggle}>toggle it</Button>

and if you don't need toggling just like, use state

const [modalOpen, setModalOpen] = useState(false);

return (
 <Button onClick={() => setModalOpen(true)}>set it open</Button>

[BPScott]

It's veeery subtle but

const [thingShown, setThingShown] = useState(false);
 <Button onClick={() => setThingShown(true)}>set it open</Button>

Results in a new handler being created every render, which if you pass it into a pure component will break it's pure optimisations, e.g. if Button was pure above, it would always rerender because the onClick prop differs every time. You'd need to define const setThingShownTrue = useCallback(() => setThingShown(state => true), []); and pass that to onClick to not break the pure check.

The utility of these two hooks is that it encapsulates those useCallback wrappings so people don't have to think about forgetting that useCallback, I think that's a key part of this.

[marutypes]

useCallback is usually premature optimization IMO so I'd rather we didn't do it for simple stuff like this by default

[marutypes]

been trying to discourage it in general ;)

[marutypes]

👏🏻 yessss death to default

[marutypes]

so annoying that this is the case :<

tuple literal when?

[BPScott]

I found out that as const as added in TS 3.4 does the same thing but I'm kinda paranoid about support in older versions of TS in consumers

[perrupa]

Have you considered returning an object instead of an array?

const {value, toggle, setTrue, setFalse} = useToggle();

renaming can be done through destructuring, like so

const {
  value: modalIsVisible, 
  toggle: toggleModal, 
  setTrue: openModal,
  setFalse: hideModal,
} = useToggle();

[BPScott]

Have you considered returning an object instead of an array?

I want to evoke a familiarity with useState's API which returns an array whos first item is the value. I don't want people to have to use destructuring renaming for the value as that's going to look clunky and is unfamiliar.

Note that the existing API for forcibleToggle is already an object with the exception of the value:

const [modalIsVisible, {
  toggle: toggleModal, 
  setTrue: openModal,
  setFalse: hideModal,
}] = useForcibleToggle();

Should we push that as the API as the primary way of doing this? i.e. would you prefer it if useToggle() had the above API (renaming useForcibleToggle to useToggle, and finding a new name for the current useToggle)

[perrupa]

Should we push that as the API as the primary way of doing this? i.e. would you prefer it if useToggle() had the above API (renaming useForcibleToggle to useToggle, and finding a new name for the current useToggle)

I think we should pick 1 API to expose, and run with it. Creating two hooks that do the same thing but expose slightly different APIs seems to discourage consistency. I like the idea of using forcibleToggle's API but renaming it to useToggle.

[Flufd]

In the past I used [state, toggleState, setState] = useToggleState(true)
If you only need the state + setState parts then you can just use the regular useState so you don't get the weird missing elements

[BPScott]

The masses have spoken. Two Hooks instead of one is a little confusing and following the "return an array with two things" API stet state is considered clunky if the second item is an object, so now we have a pure object based API.

If you need everything:

const {
  value: isActive,
  toggle: toggleIsActive,
  setTrue: setIsActiveTrue,
  setFalse: setIsActiveFalse
} = useToggle(false)

If you need just the toggle:

const {value: isActive, toggle: toggleIsActive} = useToggle(false)

[AndrewMusgrave]

cc/ @BPScott Slight suggestion on naming before this is released. This hook isn't just for toggling anymore. It provides useful methods for changing a boolean value, useBoolean better represents what the hook does imo