facebook / react

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

Support reporting Suspense loading indicator outside of the suspended tree #14248

Closed danielkcz closed 4 years ago

danielkcz commented 6 years ago

Cryptic title I can imagine, but I am not aware that something like this would have been mentioned anywhere so far.

I have a page showing some statistics and it's split into two even panels. The left panel is showing some numbers and contains a form to set necessary filters. The right panel is showing some other details about filtered data. Initially, only filter form is visible, nothing else.

The user sets the filter and hits the "filter" button to send out a request. There is a requirement to show a text loader in the left panel and the right panel should be showing content loader animation. Too many loaders perhaps? Well, it kinda makes sense in this context :)

Now my confusion is how to achieve that. Obviously, I don't want each panel to run the same query on its own. I would like to do that once in the upper level. I can surely pass down the isLoading prop to both panels. However, I am not too happy about it, because once there will be a fully fledged data fetching relying on the Suspense, it would mean that for such scenarios I will need to fall back to a regular solution. Am I misunderstanding something in here?

OzairP commented 6 years ago

I think this is more of application specific issue which seems out of scope for React. Why don't you implement Redux or a local Context to apply the loader to both?

danielkcz commented 6 years ago

@OzairP That's not the point really. I am essentially trying to figure out how universal can Suspense be. Considering it kinda requires a specific approach (throwing a promise), does it mean we will need a regular approach (what we have been doing till now) as well?

I am bad at describing stuff like this. I hope someone will be able to understand me :)

danielkcz commented 6 years ago

I am going to elaborate more on this. It actually does not matter if I want to display two or more loaders. What matters is that these loaders are down the tree and Suspense is being looked up in upward direction.

Let's go with some contrived example. It's totally possible that I am missing something obvious here and the Suspense concept hasn't clicked yet in my brains.

const DataContext = createContext()
function DataLoader({ children }) {
  const [result, setResult] = useState(null)
  const onSubmit = async (formValues) => {
    // execute data fetching, eg. with apollo client
   setResult(await query(formValues))
  }
  return <Form onSubmit={onSubmit}>
    <DataContext.Provider value={result}>
      {children}
    </DataContext.Provider>
  </Form>
}

The main idea here is that this sits on upper level of the tree. Beauty of this is that I don't need to care how contents of the form looks like or how is it submit. It will just execute query whenever use clicks the button and collects current values from form fields (I am using react-form here). And thanks to the context I can access data anywhere without passing them through props.

Now if I would wrap this loader inside the Suspense, it won't have a desired effect because form itself would disappear as well and would be replaced by fallback. And if I am not mistaken, since the data fetching is happening in response to an event not as the part of tree rendering, the Suspense would not even notice it. So if I am not mistaken, the Suspense is not answer to everything.

Jessidhia commented 6 years ago

It is possible, but you'd have to use the state to keep a Promise.

The trouble is, though, that hooks are thrown out if a component suspends, so you can't keep that loading state completely within React. You'd need to also make use of a unstable_createResource or an otherwise external communication channel.


Perhaps it is safe if the suspension itself never reaches the component holding the state.

const DataContext = createContext()

function promiseToState(promise, setResult) {
  setResult({
    value: promise.then(
      value => {
        setResult({
          value,
          state: 'resolved'
        })
      },
      err => {
        setResult({
          value: err,
          state: 'rejected'
        })
      }
    ),
    state: 'pending'
  })
}

// DataLoader must not suspend
function DataLoader({ children }) {
  const [result, setResult] = useState(undefined)
  const onSubmit = async formValues => {
    // execute data fetching, eg. with apollo client
    promiseToState(query(formValues), setResult)
  }
  return (
    <Form onSubmit={onSubmit}>
      <DataContext.Provider value={result}>
        <React.Suspense fallback={null}>{children}</React.Suspense>
      </DataContext.Provider>
    </Form>
  )
}

function useDataContext() {
  const promise = useContext(DataContext)
  if (promise === undefined) {
    return undefined
  }
  if (promise.state === 'resolved') {
    return promise.value
  } else {
    throw promise.value
  }
}
gaearon commented 5 years ago

Suspense will support this eventually but not right now. Right now it's not supported to use Suspense for data fetching at all. The only use case that's officially supported today is lazy().

This feature is mentioned in https://github.com/facebook/react/issues/13206 as "soft expiration". (Yeah this name doesn't make any sense, it's an internal-only concept though).

Let's use this issue to track its implementation then.

gregberge commented 5 years ago

Not Suspense, but using hooks + context we can render a progress outside of the tree, a working example: https://codesandbox.io/s/6l65nq9k3n

D1no commented 5 years ago

@neoziro thats a pretty cool example! Thanks! I'll shamelessly use that :)

thysultan commented 5 years ago

@FredyC Instead of the submit event handler you can delegate the "query" to descendent with an effect that is invoked whenever the submitted value changes.

const Context = createContext()

function Loader ({ children }) {
  const [state, dispatch] = useState({})

  return (
    <form onSubmit={event => dispatch(event)}>
      <Suspense><Provider value={state}>{children}</Provider></Suspense>
    </form>
  ) 
}

function Provider ({ children, value }) {
  const [state, dispatch] = useState(value)

  useEffect(() => {
    query(value)
  }, [value])

  return (
    <Context.Provider>
      {children}
    </Context.Provider>
  )
}

This assuming throwing a promise within an effect triggers the suspense mechanism, where "query" will do the throw/await/cache dance.

crazyll commented 5 years ago

Not Suspense, but using hooks + context we can render a progress outside of the tree, a working example: https://codesandbox.io/s/6l65nq9k3n

this demo is no effect when we need show progress/loading with async data request before component render.

gregberge commented 5 years ago

@crazyll yeah you are right, Loadable Components does not handle data fetching, you have to do it by your own.

belgac commented 5 years ago

Not Suspense, but using hooks + context we can render a progress outside of the tree, a working example: https://codesandbox.io/s/6l65nq9k3n

this demo is no effect when we need show progress/loading with async data request before component render.

For people wanting to use it with loading components and explore possibilities for data loading i modified the demo to use suspense https://codesandbox.io/s/suspense-progress-bar-mu9lv

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution.

stale[bot] commented 4 years ago

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please create a new issue with up-to-date information. Thank you!

iamstarkov commented 4 years ago

@gaearon

This feature is mentioned in #13206 as "soft expiration". (Yeah this name doesn't make any sense, it's an internal-only concept though).

Let's use this issue to track its implementation then.

Should the issue be reopened?

gaearon commented 4 years ago

Broadly speaking, React.useTransition in experimental releases serves this use case. The exact specifics are slightly different (and also subject to change) but the feature exists.

danielkcz commented 4 years ago

@gaearon Personally, I did not understand useTransition just yet. Would you mind elaborating and applying to this issue relevant code sample?

gaearon commented 4 years ago

My elaboration wouldn’t be any different from what’s already in the docs. A more pointed question might help.