facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
225.54k stars 45.98k forks source link

Feature Idea: React.useConditional(key, () => { ... }) for conditional and dynamic hooks #22286

Open jedwards1211 opened 2 years ago

jedwards1211 commented 2 years ago

I use @apollo/react-hooks' useQuery a lot, and recently I've had some cases where I needed to call useQuery for each item in an array.

In most idiomatic use cases, each item would map to a separate element UI, so I could have a custom component that performs a single useQuery and renders that list item.

But I have a use case where I'm fetching data for a dynamic list of historical data channels and then rendering data for all of them onto a single HTML canvas. So I have to render a dynamic number of child elements that call useQuery once, stash the data in a ref from the parent element, and return null.

I've also had users of my material-ui-popup-state custom hook asking me how to map an array to hook calls. Fortunately I was able to find a cleaner solution for them.

But, I'm starting to believe that the need for conditional/dynamic hooks is inevitable. There would be ways to avoid multiple useQuery calls in my use case, but they wouldn't be ideal (passing a list of tags to a single query, for example, could end up refetching a bunch of tags that are already cached when just one new tag is added).

Would an API like React.useConditional in the following be possible?

import * as React from 'react'
import { useQuery } from '@apollo/react-hooks'

function ConnectedPlot({ tags }) {
  const results = tags.map(tag => React.useConditional(
    tag,
    () => useQuery(metadataQuery, { variables: { tag } })
  ))
  if (results.some(r => r.loading)) return <LoadingBanner />
  const error = results.find(r => r.error)
  if (error) return <ErrorBanner error={error} />

  const metadataByTag = Object.fromEntries(results.map(r => [r.data.MetadataItem.tag, r.data.MetadataItem]))

  return <Plot tags={tags} metadataByTag={metadataByTag} />
}

The first argument to React.useConditional would be a unique key, just like keys on React elements.

The second argument would be a function that may call any builtin or custom hooks the user likes. Within that function, the same old rules of hooks would apply: all hook calls must be unconditional and in the same order as last time (for the particular key). Only calls to React.useConditional could be conditional.

I'm assuming it would be possible for React to do bookkeeping before and after it calls the callback to keep track of which hooks it called, and compare the keys from the previous render to the next, performing mounting effects for hooks in keys that were added and unmounting effects for hooks in keys that weren't reused.

jedwards1211 commented 2 years ago

Here's an example of how there's a desire for dynamic hooks and one workaround: https://unsplash.com/blog/calling-react-hooks-conditionally-dynamically-using-render-props/

Below is my userland workaround for dynamic hooks. Here are its drawbacks compared to what a builtin React.useConditional would theoretically be able to provide:

/**
 * @prettier
 * @flow
 */

import * as React from 'react'

type UseConditionalFn<K: string, T> = ((key: K, fn: () => T) => ?T) &
  ((key: K, fn: () => T, defaultValue: T) => T)

type UseConditionalProps<K: string, T> = {|
  _key: K,
  fn: () => T,
  setState: (updater: (state: { [K]: T }) => { [K]: T }) => any,
|}

function UseConditional<K: string, T>({
  _key,
  fn,
  setState,
}: UseConditionalProps<K, T>): null {
  const value = fn()
  React.useLayoutEffect(() => {
    setState((state) => ({ ...state, [_key]: value }))
  }, [value])
  React.useEffect(
    () => () => {
      setState((state: { [K]: T }): { [K]: T } => {
        const newState = { ...state }
        delete newState[_key]
        return newState
      })
    },
    []
  )

  return null
}

export default function useConditionals<K: string, T>(): [
  React.Node,
  UseConditionalFn<K, T>
] {
  const [state, setState] = React.useState<{ [K]: T }>({})
  const elements = []

  const useConditional: UseConditionalFn<K, T> = (function (
    key: K,
    fn: () => T,
    defaultValue?: T
  ): ?T {
    elements.push(
      <UseConditional key={key} _key={key} fn={fn} setState={setState} />
    )
    return key in state ? state[key] : defaultValue
  }: any)

  return [elements, useConditional]
}
kmercer1 commented 2 years ago

I'm running into a similar situation, but specifically with Redux Toolkit. At the moment, we have components that will only call one endpoint depending on the view. If the view is X, it'll call A. If the view is Y, it'll call B.

I can support both hooks side by side and trick RTK by passing in skipToken in the non-selected-view hook, which prevents it from triggering a query. But regardless, both hooks are still executing and dispatching actions.

Ended up writing a wrapper hook that uses a loop (violating a rule) that calls all passed in useQuery hooks (so the order is always the same and each hook is always called) and it handles the skipToken logic for me. But this starts to feel a bit more complicated than what it should be.

jedwards1211 commented 2 years ago

@kmercer1 yeah skip options are another good argument for this, Apollo's useQuery has a skip option, I recently added a disable option to useMeasure hook I made, etc. It's a code smell. If there were a useConditional API like I propose, no hooks would need to support a skip option.

pauldraper commented 4 months ago

Yep, something like https://github.com/facebook/react/issues/22286#issuecomment-916602977 is possible today, but the disadvantages -- especially not accessing the values synchronously -- are big.

github-actions[bot] commented 1 month ago

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

pauldraper commented 1 month ago

still relevant