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

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

Closed
danielkcz opened this issue Nov 15, 2018 · 17 comments
Closed
Labels

Comments

@danielkcz
Copy link

danielkcz commented Nov 15, 2018

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
Copy link

OzairP commented Nov 16, 2018

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
Copy link
Author

@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
Copy link
Author

danielkcz commented Nov 16, 2018

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
Copy link
Contributor

Jessidhia commented Nov 16, 2018

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
Copy link
Collaborator

gaearon commented Nov 19, 2018

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 #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.

@gaearon gaearon changed the title [Question] Downward Suspense? Support reporting Suspense loading indicator outside of the suspended tree Nov 19, 2018
@gregberge
Copy link

gregberge commented Feb 15, 2019

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

@D1no
Copy link

D1no commented Apr 28, 2019

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

@thysultan
Copy link

@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
Copy link

crazyll commented May 31, 2019

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
Copy link

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

@belgac
Copy link

belgac commented Jul 16, 2019

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
Copy link

stale bot commented Jan 10, 2020

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 stale bot added the Resolution: Stale Automatically closed due to inactivity label Jan 10, 2020
@stale
Copy link

stale bot commented Jan 17, 2020

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!

@stale stale bot closed this as completed Jan 17, 2020
@iamstarkov
Copy link

@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
Copy link
Collaborator

gaearon commented Apr 1, 2020

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
Copy link
Author

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

@gaearon
Copy link
Collaborator

gaearon commented Apr 2, 2020

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

10 participants