liferay / clay

A web implementation of the Lexicon Experience Language
http://clayui.com
Other
208 stars 483 forks source link

DataProvider: Integrate with <Suspense /> #2781

Closed matuzalemsteles closed 3 years ago

matuzalemsteles commented 4 years ago

<Suspense /> is a powerful feature in React that allows you to integrate with data fetching libraries to suspend component rendering and declaratively specify a load state. This is very important because it allows React to look deeper into the tree trying to render other components and parallelize requests.

We need to make some modifications to DataProvider to make this possible, some things noted were introduced in #2778. This can also change the way we implement the way "lazy" for useResource that is being introduced in #2672.

bryceosterhaus commented 4 years ago

One note about Suspense is that we should likely wait to make any changes in Clay until suspense is in a stable release and no longer experimental. I know that they will eventually make it officially stable, but they also highly discourage any production uses for it right now since its so subject to change.

matuzalemsteles commented 4 years ago

@bryceosterhaus Of course, I think the <Suspense /> for data fetching is stable since it has been around since 16.6, probably if seen changes around, it will be in relation to how things are done inside your codebase. But of course we must pay attention to these changes.

bryceosterhaus commented 4 years ago

I am primarily just going off of this in their docs Screen Shot 2019-12-11 at 2 40 52 PM

matuzalemsteles commented 4 years ago

Yeah, I think this is related to the experimental version of the react and react-dom packages. You can see more about it here https://reactjs.org/docs/concurrent-mode-adoption.html#feature-comparison

wincent commented 4 years ago

I know that they will eventually make it officially stable, but they also highly discourage any production uses for it right now since its so subject to change.

This is a super important thing to bear in mind, in general, so thanks for bringing it up, @bryceosterhaus. In this concrete case,React.Suspense itself is stable, AFAIK, and listed on the top-level API page. The disclaimer there is:

Suspense lets components “wait” for something before rendering. Today, Suspense only supports one use case: loading components dynamically with React.lazy. In the future, it will support other use cases like data fetching.

So, they definitely won't break the API when used in conjunction with React.lazy, and the "data fetching" they're talking about here is alluding to the concurrent mode stuff; anecdotally, I don't think they'd change the core detail of the underlying implementation (ie. throwing a promise), but it is possible that something may change about other details of the system before concurrent mode gets promoted from experimental to stable. If we want to be conservative (we probably should), we should move carefully here, just in case.

matuzalemsteles commented 4 years ago

Thanks @wincent, I think we can wait for the breakthrough with Suspense until it goes from experimental to stable. This is likely not to be long 😁

julien commented 3 years ago

Closing due to inactivity - we can revisit this when we actually plan on working on it.