Split Timeout into separate Timeout and Placeholder primitives #12728
Conversation
Adds Timeout component. If a promise is thrown from inside a Timeout component, React will suspend the in-progress render from committing. When the promise resolves, React will retry. If the render is suspended for longer than the maximum threshold, the Timeout switches to a placeholder state. The timeout threshold is defined as the minimum of: - The expiration time of the current render - The `ms` prop given to each Timeout component in the ancestor path of the thrown promise.
Co-authored-by: Andrew Clark <[email protected]>
React should resume rendering regardless of whether it resolves or rejects.
Async is not required for Suspense, but strict mode is.
Some of this was added with "soft expiration" in mind, but now with our revised
model for how soft expiration will work, this isn't necessary.
It would be nice to remove more of this, but I think the list itself is inherent
because we need a way to track the start times, for <Timeout ms={ms} />.
It already worked this way in practice.
I think this separation makes sense, if for no other reason than it helps with the mental model. A Timeout determines the threshold before a suspend "effect" turns into an error. A Placeholder is like an error boundary that pattern matches on those errors.
| : 0xead1; | ||
| export const REACT_TIMEOUT_TYPE = hasSymbol | ||
| ? Symbol.for('react.timeout') | ||
| : 0xead2; |
There was a problem hiding this comment.
These new symbols should be added to ReactIs and getComponentName.
There was a problem hiding this comment.
Probably also need a follow-up PR for DevTools to add the new type in so it doesn't show as "unknown"?
|
React: size: 🔺+1.0%, gzip: 🔺+0.7% ReactDOM: size: 🔺+1.1%, gzip: 🔺+1.2% Details of bundled changes.Comparing: ad7cd68...271aa2f react
react-dom
react-art
react-test-renderer
react-reconciler
react-native-renderer
Generated by 🚫 dangerJS |
|
We've been able to do a lot of refactors and change significantly how React works because nobody relies on subtle differences in practice. The APIs we exposed are really the highest level possible that we could get away with, not the lowest level. The lowest level used to be updateComponent or something on the object oriented API and would be beginWork or something in the new API. Exposing the lowest primitive is on its own not a virtue. What's the use case that can't be described with just a Timeout and do you expect people to actually use these separately in practice? |
|
I think there are many places where you'd use a Placeholder without a Timeout. In fact, I think usually you would use Placeholder without overriding the timeout. There are no cases where it benefits you to use Timeout without a Placeholder. At least that I can think of. So the reason to separate them is to avoid overuse of Timeout. I'm concerned if they're both in the same component, it'll be too tempting to specify a Now that I type this out, a better solution might be to keep the two primitives combined but rename Timeout to Placeholder, like we do with the higher-level API we use in demos. |
|
Yea, I think the only reason we named it Timeout was because we expected the higher-level thing to be called Placeholder and that you'd never use Timeout in practice. Maybe that's just an indication that we do need the higher level thing to be built-in and we just have to solve the cross-renderer issue and what it means to hide something. |
|
Should we link the pr(i.e. Just use PlaceHolder and deprecate Timeout) or add the reason why close this? |
I think this separation makes sense, if for no other reason than it helps with the mental model. A Timeout determines the threshold before a suspend "effect" turns into an error. A Placeholder is like an error boundary that pattern matches on those errors.
Depends on #12279. Opening this separately, since it's not needed for the basic functionality and it's debatable whether these are the right primitives.