Disable timeoutMs argument #19703
Merged
Disable timeoutMs argument #19703
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We're removing the `timeoutMs` option, so there's no longer any distinction between "short" and "long" transitions. They're all treated the same. This commit doesn't remove `timeoutMs` yet, only combines the internal priority levels.
tl;dr ----- - We're removing the `timeoutMs` argument from `useTransition`. - Transitions will either immediately switch to a skeleton/placeholder view (when loading new content) or wait indefinitely until the data resolves (when refreshing stale content). - This commit disables the `timeoutMS` so that the API has the desired semantics. It doesn't yet update the types or migrate all the test callers. I'll do those steps in follow-up PRs. Motivation ---------- Currently, transitions initiated by `startTransition` / `useTransition` accept a `timeoutMs` option. You can use this to control the maximum amount of time that a transition is allowed to delay before we give up and show a placeholder. What we've discovered is that, in practice, every transition falls into one of two categories: a **load** or a **refresh**: - **Loading a new screen**: show the next screen as soon as possible, even if the data hasn't finished loading. Use a skeleton/placeholder UI to show progress. - **Refreshing a screen that's already visible**: keep showing the current screen indefinitely, for as long as it takes to load the fresh data, even if the current data is stale. Use a pending state (and maybe a busy indicator) to show progress. In other words, transitions should either *delay indefinitely* (for a refresh) or they should show a placeholder *instantly* (for a load). There's not much use for transitions that are delayed for a small-but-noticeable amount of time. So, the plan is to remove the `timeoutMs` option. Instead, we'll assign an effective timeout of `0` for loads, and `Infinity` for refreshes. The mechanism for distinguishing a load from a refresh already exists in the current model. If a component suspends, and the nearest Suspense boundary hasn't already mounted, we treat that as a load, because there's nothing on the screen. However, if the nearest boundary is mounted, we treat that as a refresh, since it's already showing content. If you need to fix a transition to be treated as a load instead of a refresh, or vice versa, the solution will involve rearranging the location of your Suspense boundaries. It may also involve adding a key. We're still working on proper documentation for these patterns. In the meantime, please reach out to us if you run into problems that you're unsure how to fix. We will remove `timeoutMs` from `useDeferredValue`, too, and apply the same load versus refresh semantics to the update that spawns the deferred value. Note that there are other types of delays that are not related to transitions; for example, we will still throttle the appearance of nested placeholders (we refer to this as the placeholder "train model"), and we may still apply a Just Noticeable Difference heuristic (JND) in some cases. These aren't going anywhere. (Well, the JND heuristic might but for different reasons than those discussed above.)
Details of bundled changes.Comparing: 60ba723...93ca66a react-dom
react-art
react-reconciler
react-test-renderer
react-native-renderer
Size changes (experimental) |
Details of bundled changes.Comparing: 60ba723...93ca66a react-dom
react-art
react-native-renderer
react-reconciler
react-test-renderer
Size changes (stable) |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 93ca66a:
|
acdlite
force-pushed
the
disable-timeoutms
branch
3 times, most recently
from
12efd27
to
93ca66a
Compare
facebook-github-bot
added
CLA Signed
React Core Team
rickhanlonii
approved these changes
Aug 26, 2020
| }); | ||
| // Later we load the data. | ||
| Scheduler.unstable_advanceTime(3000); | ||
| await advanceTimers(3000); | ||
| await resolveText('B'); |
There was a problem hiding this comment.
Nice, I was just thinking we should refactor this so resolving the data doesn't require timers.
| <span hidden={true} prop="B" /> | ||
| <span prop="Loading..." /> | ||
| </>, | ||
| ); |
There was a problem hiding this comment.
Why was this test ending with a placeholder before instead or resolving the values like we do now?
There was a problem hiding this comment.
This is a regression test that, at the time, triggered a very specific bug in the implementation. It's not really possible to trigger the exact scenario anymore because the semantics of placeholders have changed. But I generally like to keep around old regression tests because they cover cases that we often wouldn't think to write tests for otherwise. It's not uncommon for them to catch things during refactors, even when the initial bug is no longer relevant.
There was a problem hiding this comment.
I guess what I'm asking is do you know why it stopped with rendering a fallback?
Was that that regression we were testing?
| @@ -4030,9 +3997,6 @@ describe('ReactSuspenseWithNoopRenderer', () => { | |||
| // Neither is resolved yet. | |||
| 'Pending...', | |||
| '', | |||
| 'Pending...', | |||
| 'Suspend! [a]', | |||
There was a problem hiding this comment.
How was [a] suspending again before and why isn't it now?
There was a problem hiding this comment.
A used to be categorized as TransitionShortPriority and B was TransitionLongPriority. Now they're both just TransitionPriority and so get batched together. This will change again once we implement parallel transitions.
There was a problem hiding this comment.
I see, would it help to update the comment here to say:
// Neither is resolved yet, but they're batched so we only see the B suspend flushed.
|
|
||
| const TransitionLongHydrationLane: Lane = /* */ 0b0000000000000100000000000000000; | ||
| const TransitionLongLanes: Lanes = /* */ 0b0000000001111000000000000000000; | ||
| const TransitionHydrationLane: Lane = /* */ 0b0000000000000000001000000000000; |
There was a problem hiding this comment.
Is the hydration lane used for loads or is it used for something else?
There was a problem hiding this comment.
Hydration lanes are a used for this scenario:
- During progressive hydration, only part of the initial HTML has been hydrated. Let's say a parent is hydrated, but its child has not.
- Then, the parent receives an update (
setState) and attempts to re-render the child. But the child hasn't even hydrated yet; it's still just showing the initial HTML. - We could render a brand new child using the newest props, but that would require discarding the initial HTML and losing its state, like text input.
- So instead we do a clever thing: pause the parent update, then bump the priority of hydrating the child to a slightly higher priority.
- Finish hydrating the child. It has now fully mounted.
- Now try the parent update again and proceed like normal.
In order for the "bump the priority" trick to work, we reserve special lanes that sit in between the ranges used for regular updates. The hydration lanes only ever include this type of work, so that they don't get accidentally batched together with other updates.
There was a problem hiding this comment.
See:
There was a problem hiding this comment.
Awesome, that makes sense. Is it possible (semantically or structurally) for hydration lanes to get entangled with non-hydration lanes?
There was a problem hiding this comment.
I think theoretically it could happen with useMutableSource, which uses entanglement to de-opt and prevent tearing in some cases when it detects a mutation.
Thanks for the invitation 😄 It's very possible I'm missing something, so I apologize if that's the case. My first thought about this is: what about a scenario where the data takes 100ms to resolve (for a refresh). If pending is set to So before this change, the way I solved that was using the (currently undocumented and I only think I know how they work) I guess that if those options are still available then this should work ok, but this PR #19719 makes it sound like all config has been removed. So my main question is, what's the (experimental but current) recommended approach to avoiding a flash of loading state given the changes? In case it's helpful, here's how I explain it when I teach it to others: https://react-suspense.netlify.app/3 (Please, keep in mind that before people pay for or go through this material it is made extremely clear that concurrent mode is experimental and likely to change...). |
My understanding is that there is currently none. In the longer term, this is something we want to support in principle, but with a different API. In the short term, not supporting this seems like already the status quo before Concurrent Mode — so it's something you have to accept (or work around in userland) regardless. |
|
Longer term the hope is to build a first-class API for enter/leave animations. Like ReactTransitionGroup, but way better. So yeah, we're kicking the can down the road a bit. |
|
I can accept that 🙂 thanks! |
|
Drive-by curiosity question: Since “suspending on a user-blocking update” doesn’t seem to be an issue anymore, what’s the practical difference now between wrapping a setState call in a startTransition vs not wrapping it in startTransition, in the case that the state change triggers a not-yet-mounted Suspense boundary (aka effective timeoutMs of 0)? (It’s cool if the answer is just “we’ll talk about that in the docs when we release”) |
9 tasks
facebook-github-bot
pushed a commit
to facebook/react-native
that referenced
this pull request
Oct 27, 2020
Summary: This sync includes the following changes: - **[eaaf4cbce](facebook/react@eaaf4cbce )**: 17.0.1 //<Dan Abramov>// - **[6f62abb58](facebook/react@6f62abb58 )**: Remove usage of Array#fill ([#20071](facebook/react#20071)) //<Dan Abramov>// - **[40cfe1f48](facebook/react@40cfe1f48 )**: Update CHANGELOG.md //<Dan Abramov>// - **[f021a983a](facebook/react@f021a983a )**: Bump versions for 17 ([#20062](facebook/react#20062)) //<Dan Abramov>// - **[d1bb4d851](facebook/react@d1bb4d851 )**: Profiler: Include ref callbacks in onCommit duration ([#20060](facebook/react#20060)) //<Brian Vaughn>// - **[c59c3dfe5](facebook/react@c59c3dfe5 )**: useRef: Warn about reading or writing mutable values during render ([#18545](facebook/react#18545)) //<Brian Vaughn>// - **[7b6cac952](facebook/react@7b6cac952 )**: Improved Profiler commit hooks test ([#20053](facebook/react#20053)) //<Brian Vaughn>// - **[dfb6a4033](facebook/react@dfb6a4033 )**: [Fast Refresh] Fix crashes caused by rogue Proxies ([#20030](facebook/react#20030)) ([#20039](facebook/react#20039)) //<Kai Riemann>// - **[37cb732c5](facebook/react@37cb732c5 )**: Use bitwise OR to define flag masks ([#20044](facebook/react#20044)) //<Andrew Clark>// - **[eb3181e77](facebook/react@eb3181e77 )**: Add Visibility flag for hiding/unhiding trees ([#20043](facebook/react#20043)) //<Andrew Clark>// - **[0dd809bdf](facebook/react@0dd809bdf )**: Remove last schedulePassiveEffectCallback call ([#20042](facebook/react#20042)) //<Andrew Clark>// - **[8df7b7911](facebook/react@8df7b7911 )**: Remove Passive flag from "before mutation" phase ([#20038](facebook/react#20038)) //<Andrew Clark>// - **[c57fe4a2c](facebook/react@c57fe4a2c )**: ReactIs.isValidElementType Unit Test extended with PureComponent case ([#20033](facebook/react#20033)) //<adasq>// - **[02da938fd](facebook/react@02da938fd )**: Don't double-invoke effects in legacy roots ([#20028](facebook/react#20028)) //<Brian Vaughn>// - **[d95c4938d](facebook/react@d95c4938d )**: [EventSystem] Revise onBeforeBlur propagation mechanics ([#20020](facebook/react#20020)) //<Dominic Gannaway>// - **[f46a80ae1](facebook/react@f46a80ae1 )**: Update outdated links and fix two broken links ([#19985](facebook/react#19985)) //<Saikat Guha>// - **[0a4c7c565](facebook/react@0a4c7c565 )**: [Flight] Don't warn for key, but error for ref ([#19986](facebook/react#19986)) //<Sebastian Markbåge>// - **[993ca533b](facebook/react@993ca533b )**: Enable eager listeners statically ([#19983](facebook/react#19983)) //<Dan Abramov>// - **[40c52de96](facebook/react@40c52de96 )**: [Flight] Add Runtime Errors for Non-serializable Values ([#19980](facebook/react#19980)) //<Sebastian Markbåge>// - **[1992d9730](facebook/react@1992d9730 )**: Revert "Temporarily disable Profiler commit hooks flag ([#19900](facebook/react#19900))" ([#19960](facebook/react#19960)) //<Brian Vaughn>// - **[44d39c4d7](facebook/react@44d39c4d7 )**: Removed skip-error-boundaries modifications from old fork ([#19961](facebook/react#19961)) //<Brian Vaughn>// - **[cc77be957](facebook/react@cc77be957 )**: Remove unnecessary error overriding in ([#19949](facebook/react#19949)) //<Paul Doyle>// - **[97625272a](facebook/react@97625272a )**: Debug tracing tests for CPU bound suspense ([#19943](facebook/react#19943)) //<Brian Vaughn>// - **[43363e279](facebook/react@43363e279 )**: Fix codestyle for typeof comparison ([#19928](facebook/react#19928)) //<Eugene Maslovich>// - **[5427b4657](facebook/react@5427b4657 )**: Temporarily disable Profiler commit hooks flag ([#19900](facebook/react#19900)) //<Brian Vaughn>// - **[1faf9e3dd](facebook/react@1faf9e3dd )**: Suspense for CPU-bound trees ([#19936](facebook/react#19936)) //<Andrew Clark>// - **[7f08e908b](facebook/react@7f08e908b )**: Fix missing context to componentDidMount() when double-invoking lifecycles ([#19935](facebook/react#19935)) //<Brian Vaughn>// - **[9198a5cec](facebook/react@9198a5cec )**: Refactor layout effect methods ([#19895](facebook/react#19895)) //<Brian Vaughn>// - **[ba82eea38](facebook/react@ba82eea38 )**: Remove disableSchedulerTimeoutInWorkLoop flag ([#19902](facebook/react#19902)) //<Andrew Clark>// - **[c63741fb3](facebook/react@c63741fb3 )**: offscreen double invoke effects ([#19523](facebook/react#19523)) //<Luna Ruan>// - **[c6917346f](facebook/react@c6917346f )**: Fixed broken Profiler test ([#19894](facebook/react#19894)) //<Brian Vaughn>// - **[87c023b1c](facebook/react@87c023b1c )**: Profiler onRender only called when we do work ([#19885](facebook/react#19885)) //<Brian Vaughn>// - **[81aaee56a](facebook/react@81aaee56a )**: Don't call onCommit et al if there are no effects ([#19863](facebook/react#19863)) //<Andrew Clark>// - **[7355bf575](facebook/react@7355bf575 )**: Consolidate commit phase hook functions ([#19864](facebook/react#19864)) //<Andrew Clark>// - **[bc6b7b6b1](facebook/react@bc6b7b6b1 )**: Don't trigger lazy in DEV during element creation ([#19871](facebook/react#19871)) //<Dan Abramov>// - **[a774502e0](facebook/react@a774502e0 )**: Use single quotes in getComponentName return ([#19873](facebook/react#19873)) //<Gustavo Saiani>// - **[8b2d3783e](facebook/react@8b2d3783e )**: Use Passive flag to schedule onPostCommit ([#19862](facebook/react#19862)) //<Andrew Clark>// - **[50d9451f3](facebook/react@50d9451f3 )**: Improve DevTools editing interface ([#19774](facebook/react#19774)) //<Brian Vaughn>// - **[6fddca27e](facebook/react@6fddca27e )**: Remove passive intervention flag ([#19849](facebook/react#19849)) //<Dan Abramov>// - **[36df9185c](facebook/react@36df9185c )**: chore(docs): Removed outdated comment about fb.me link ([#19830](facebook/react#19830)) //<Adnaan Bheda>// - **[16fb2b6f9](facebook/react@16fb2b6f9 )**: Moved resetChildLanes into complete work ([#19836](facebook/react#19836)) //<Brian Vaughn>// - **[cc581065d](facebook/react@cc581065d )**: eslint-plugin-react-hooks@4.1.2 //<Dan Abramov>// - **[0044805c8](facebook/react@0044805c8 )**: Update CHANGELOG.md //<Dan Abramov>// - **[0f70d4dd6](facebook/react@0f70d4dd6 )**: Consider components in jsx as missing dependencies in typescript-eslint/parser@4.x ([#19815](facebook/react#19815)) //<Sebastian Silbermann>// - **[84558c61b](facebook/react@84558c61b )**: Don't visit passive effects during layout phase ([#19809](facebook/react#19809)) //<Andrew Clark>// - **[ad8a0a8cd](facebook/react@ad8a0a8cd )**: eslint-plugin-react-hooks@4.1.1 //<Dan Abramov>// - **[77544a0d6](facebook/react@77544a0d6 )**: Update CHANGELOG.md //<Dan Abramov>// - **[ed4fdfc73](facebook/react@ed4fdfc73 )**: test(eslint-plugin-react-hooks): Run with TS parsers >= 2.x ([#19792](facebook/react#19792)) //<Sebastian Silbermann>// - **[cd75f93c0](facebook/react@cd75f93c0 )**: eslint-plugin-react-hooks: fix compatibility with typescript-eslint/parser@4.0.0+ ([#19751](facebook/react#19751)) //<Matthias Schiffer>// - **[781212aab](facebook/react@781212aab )**: Remove double space in test name ([#19762](facebook/react#19762)) //<Gustavo Saiani>// - **[e7b255341](facebook/react@e7b255341 )**: Internal `act`: Flush timers at end of scope ([#19788](facebook/react#19788)) //<Andrew Clark>// - **[d17086c7c](facebook/react@d17086c7c )**: Decouple public, internal act implementation ([#19745](facebook/react#19745)) //<Andrew Clark>// - **[d38ec17b1](facebook/react@d38ec17b1 )**: [Flight] Set dispatcher for duration of performWork() ([#19776](facebook/react#19776)) //<Joseph Savona>// - **[4f3f7eeb7](facebook/react@4f3f7eeb7 )**: Bugfix: Effect clean up when deleting suspended tree ([#19752](facebook/react#19752)) //<Andrew Clark>// - **[7baf9d412](facebook/react@7baf9d412 )**: Combine Flags and SubtreeFlags types ([#19775](facebook/react#19775)) //<Andrew Clark>// - **[166544360](facebook/react@166544360 )**: Rename effect fields ([#19755](facebook/react#19755)) //<Andrew Clark>// - **[708fa77a7](facebook/react@708fa77a7 )**: Decrease expiration time of input updates ([#19772](facebook/react#19772)) //<Andrew Clark>// - **[36df483af](facebook/react@36df483af )**: Add feature flag to disable scheduler timeout in work loop ([#19771](facebook/react#19771)) //<Ricky>// - **[bcc0aa463](facebook/react@bcc0aa463 )**: Revert "Revert "Remove onScroll bubbling flag ([#19535](facebook/react#19535))" ([#19655](facebook/react#19655))" ([#19761](facebook/react#19761)) //<Dan Abramov>// - **[99cae887f](facebook/react@99cae887f )**: Add failing test for passive effect cleanup functions and memoized components ([#19750](facebook/react#19750)) //<Brian Vaughn>// - **[2cfd73c4d](facebook/react@2cfd73c4d )**: Fix typo in comment (Noticable→Noticeable) ([#19737](facebook/react#19737)) //<Ikko Ashimine>// - **[53e622ca7](facebook/react@53e622ca7 )**: Fix instances of function declaration after return ([#19733](facebook/react#19733)) //<Bhumij Gupta>// - **[b7d18c4da](facebook/react@b7d18c4da )**: Support Babel's envName option in React Refresh plugin ([#19009](facebook/react#19009)) //<Kevin Weber>// - **[1f38dcff6](facebook/react@1f38dcff6 )**: Remove withSuspenseConfig ([#19724](facebook/react#19724)) //<Andrew Clark>// - **[1396e4a8f](facebook/react@1396e4a8f )**: Fixes eslint warning when node type is ChainExpression ([#19680](facebook/react#19680)) //<Pascal Fong Kye>// - **[a8500be89](facebook/react@a8500be89 )**: Add `startTransition` as a known stable method ([#19720](facebook/react#19720)) //<Andrew Clark>// - **[380dc95de](facebook/react@380dc95de )**: Revert "Append text string to <Text> error message ([#19581](facebook/react#19581))" ([#19723](facebook/react#19723)) //<Timothy Yung>// - **[ddd1faa19](facebook/react@ddd1faa19 )**: Remove config argument from useTransition ([#19719](facebook/react#19719)) //<Andrew Clark>// - **[92fcd46cc](facebook/react@92fcd46cc )**: Replace SuspenseConfig object with an integer ([#19706](facebook/react#19706)) //<Andrew Clark>// - **[b754caaaf](facebook/react@b754caaaf )**: Enable eager listeners in open source ([#19716](facebook/react#19716)) //<Dan Abramov>// - **[c1ac05215](facebook/react@c1ac05215 )**: [Flight] Support more element types and Hooks for Server and Hybrid Components ([#19711](facebook/react#19711)) //<Dan Abramov>// - **[1eaafc9ad](facebook/react@1eaafc9ad )**: Clean up timeoutMs-related implementation details ([#19704](facebook/react#19704)) //<Andrew Clark>// - **[8da0da093](facebook/react@8da0da093 )**: Disable timeoutMs argument ([#19703](facebook/react#19703)) //<Andrew Clark>// - **[60ba723bf](facebook/react@60ba723bf )**: Add SuspenseList to devTools ([#19684](facebook/react#19684)) //<Ben Pernick>// - **[5564f2c95](facebook/react@5564f2c95 )**: Add React.startTransition ([#19696](facebook/react#19696)) //<Ricky>// - **[c4e0768d7](facebook/react@c4e0768d7 )**: Remove unused argument from `finishConcurrentRender` ([#19689](facebook/react#19689)) //<inottn>// - **[848bb2426](facebook/react@848bb2426 )**: Attach Listeners Eagerly to Roots and Portal Containers ([#19659](facebook/react#19659)) //<Dan Abramov>// - **[d2e914ab4](facebook/react@d2e914ab4 )**: Remove remaining references to effect list ([#19673](facebook/react#19673)) //<Andrew Clark>// - **[d6e433899](facebook/react@d6e433899 )**: Use Global Render Timeout for CPU Suspense ([#19643](facebook/react#19643)) //<Sebastian Markbåge>// - **[64ddef44c](facebook/react@64ddef44c )**: Revert "Remove onScroll bubbling flag ([#19535](facebook/react#19535))" ([#19655](facebook/react#19655)) //<Dan Abramov>// - **[dd651df05](facebook/react@dd651df05 )**: Keep onTouchStart, onTouchMove, and onWheel passive ([#19654](facebook/react#19654)) //<Dan Abramov>// - **[b8fa09e9e](facebook/react@b8fa09e9e )**: provide profiling bundle for react-reconciler ([#19559](facebook/react#19559)) //<Julien Gilli>// - **[23595ff59](facebook/react@23595ff59 )**: Add missing param to safelyCallDestroy() ([#19638](facebook/react#19638)) //<Brian Vaughn>// - **[ee409ea3b](facebook/react@ee409ea3b )**: change destroy to safelyCallDestroy ([#19605](facebook/react#19605)) //<Luna Ruan>// - **[bcca5a6ca](facebook/react@bcca5a6ca )**: Always skip unmounted/unmounting error boundaries ([#19627](facebook/react#19627)) //<Brian Vaughn>// - **[1a41a196b](facebook/react@1a41a196b )**: Append text string to <Text> error message ([#19581](facebook/react#19581)) //<Timothy Yung>// - **[e4afb2fdd](facebook/react@e4afb2fdd )**: eslint-plugin-react-hooks@4.1.0 //<Dan Abramov>// - **[ced05c46c](facebook/react@ced05c46c )**: Update CHANGELOG.md //<Dan Abramov>// - **[702fad4b1](facebook/react@702fad4b1 )**: refactor fb.me redirect link to reactjs.org/link ([#19598](facebook/react#19598)) //<CY Lim>// - **[49cd77d24](facebook/react@49cd77d24 )**: fix: leak strict mode with UMD builds ([#19614](facebook/react#19614)) //<Toru Kobayashi>// - **[ffb749c95](facebook/react@ffb749c95 )**: Improve error boundary handling for unmounted subtrees ([#19542](facebook/react#19542)) //<Brian Vaughn>// - **[9b35dd2fc](facebook/react@9b35dd2fc )**: Permanently removed component stacks from scheduling profiler data ([#19615](facebook/react#19615)) //<Brian Vaughn>// - **[3f8115cdd](facebook/react@3f8115cdd )**: Remove `didTimeout` check from work loop //<Andrew Clark>// - **[9abc2785c](facebook/react@9abc2785c )**: Remove wasteful checks from `shouldYield` //<Andrew Clark>// - **[1d5e10f70](facebook/react@1d5e10f70 )**: [eslint-plugin-react-hooks] Report constant constructions ([#19590](facebook/react#19590)) //<Jordan Eldredge>// - **[dab0854c5](facebook/react@dab0854c5 )**: Move commit passive unmount/mount to CommitWork ([#19599](facebook/react#19599)) //<Sebastian Markbåge>// - **[ccb6c3945](facebook/react@ccb6c3945 )**: Remove unused argument ([#19600](facebook/react#19600)) //<inottn>// - **[629125555](facebook/react@629125555 )**: [Scheduler] Re-throw unhandled errors ([#19595](facebook/react#19595)) //<Andrew Clark>// - **[b8ed6a1aa](facebook/react@b8ed6a1aa )**: [Scheduler] Call postTask directly ([#19551](facebook/react#19551)) //<Andrew Clark>// - **[ce37bfad5](facebook/react@ce37bfad5 )**: Remove resolutions from test renderer package.json ([#19577](facebook/react#19577)) //<Dan Abramov>// - **[2704bb537](facebook/react@2704bb537 )**: Add ReactVersion to SchedulingProfiler render scheduled marks ([#19553](facebook/react#19553)) //<Kartik Choudhary>// - **[0c52e24cb](facebook/react@0c52e24cb )**: Support inner component _debugOwner in memo ([#19556](facebook/react#19556)) //<Brian Vaughn>// - **[0cd9a6de5](facebook/react@0cd9a6de5 )**: Parallelize Jest in CI ([#19552](facebook/react#19552)) //<Andrew Clark>// - **[a63893ff3](facebook/react@a63893ff3 )**: Warn about undefined return value for memo and forwardRef ([#19550](facebook/react#19550)) //<Brian Vaughn>// - **[32ff42868](facebook/react@32ff42868 )**: Add feature flag for setting update lane priority ([#19401](facebook/react#19401)) //<Ricky>// - **[5bdd4c8c6](facebook/react@5bdd4c8c6 )**: Remove unused argument from call to jest method ([#19546](facebook/react#19546)) //<Gustavo Saiani>// - **[a5fed98a9](facebook/react@a5fed98a9 )**: Register more node types that are used later as JSXIdentifiers ([#19514](facebook/react#19514)) //<Mateusz Burzyński>// - **[f77c7b9d7](facebook/react@f77c7b9d7 )**: Re-add discrete flushing timeStamp heuristic (behind flag) ([#19540](facebook/react#19540)) //<Dominic Gannaway>// Changelog: [general] [feature] Upgrade to React 17 Reviewed By: cpojer Differential Revision: D24491201 fbshipit-source-id: c947da9dcccbd614e9dc58f3339b63e24829aca7
This was referenced Apr 19, 2021
koto
pushed a commit
to koto/react
that referenced
this pull request
Jun 15, 2021
* Remove distinction between long, short transitions We're removing the `timeoutMs` option, so there's no longer any distinction between "short" and "long" transitions. They're all treated the same. This commit doesn't remove `timeoutMs` yet, only combines the internal priority levels. * Disable `timeoutMs` argument tl;dr ----- - We're removing the `timeoutMs` argument from `useTransition`. - Transitions will either immediately switch to a skeleton/placeholder view (when loading new content) or wait indefinitely until the data resolves (when refreshing stale content). - This commit disables the `timeoutMS` so that the API has the desired semantics. It doesn't yet update the types or migrate all the test callers. I'll do those steps in follow-up PRs. Motivation ---------- Currently, transitions initiated by `startTransition` / `useTransition` accept a `timeoutMs` option. You can use this to control the maximum amount of time that a transition is allowed to delay before we give up and show a placeholder. What we've discovered is that, in practice, every transition falls into one of two categories: a **load** or a **refresh**: - **Loading a new screen**: show the next screen as soon as possible, even if the data hasn't finished loading. Use a skeleton/placeholder UI to show progress. - **Refreshing a screen that's already visible**: keep showing the current screen indefinitely, for as long as it takes to load the fresh data, even if the current data is stale. Use a pending state (and maybe a busy indicator) to show progress. In other words, transitions should either *delay indefinitely* (for a refresh) or they should show a placeholder *instantly* (for a load). There's not much use for transitions that are delayed for a small-but-noticeable amount of time. So, the plan is to remove the `timeoutMs` option. Instead, we'll assign an effective timeout of `0` for loads, and `Infinity` for refreshes. The mechanism for distinguishing a load from a refresh already exists in the current model. If a component suspends, and the nearest Suspense boundary hasn't already mounted, we treat that as a load, because there's nothing on the screen. However, if the nearest boundary is mounted, we treat that as a refresh, since it's already showing content. If you need to fix a transition to be treated as a load instead of a refresh, or vice versa, the solution will involve rearranging the location of your Suspense boundaries. It may also involve adding a key. We're still working on proper documentation for these patterns. In the meantime, please reach out to us if you run into problems that you're unsure how to fix. We will remove `timeoutMs` from `useDeferredValue`, too, and apply the same load versus refresh semantics to the update that spawns the deferred value. Note that there are other types of delays that are not related to transitions; for example, we will still throttle the appearance of nested placeholders (we refer to this as the placeholder "train model"), and we may still apply a Just Noticeable Difference heuristic (JND) in some cases. These aren't going anywhere. (Well, the JND heuristic might but for different reasons than those discussed above.)
This was referenced Jan 25, 2022
Closed
8 tasks
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
tl;dr
timeoutMsargument fromuseTransition.timeoutMSso that the API has the desired semantics. It doesn't yet update the types or migrate all the test callers. I'll do those steps in follow-up PRs.Motivation
Currently, transitions initiated by
startTransition/useTransitionaccept atimeoutMsoption. You can use this to control the maximum amount of time that a transition is allowed to delay before we give up and show a placeholder.What we've discovered is that, in practice, every transition falls into one of two categories: a load or a refresh:
In other words, transitions should either delay indefinitely (for a refresh) or they should show a placeholder instantly (for a load). There's not much use for transitions that are delayed for a small-but-noticeable amount of time.
So, the plan is to remove the
timeoutMsoption. Instead, we'll assign an effective timeout of0for loads, andInfinityfor refreshes.The mechanism for distinguishing a load from a refresh already exists in the current model. If a component suspends, and the nearest Suspense boundary hasn't already mounted, we treat that as a load, because there's nothing on the screen. However, if the nearest boundary is mounted, we treat that as a refresh, since it's already showing content.
If you need to fix a transition to be treated as a load instead of a refresh, or vice versa, the solution will involve rearranging the location of your Suspense boundaries. It may also involve adding a key.
We're still working on proper documentation for these patterns. In the meantime, please reach out to us if you run into problems that you're unsure how to fix.
We will remove
timeoutMsfromuseDeferredValue, too, and apply the same load versus refresh semantics to the update that spawns the deferred value.Note that there are other types of delays that are not related to transitions; for example, we will still throttle the appearance of nested placeholders (we refer to this as the placeholder "train model"), and we may still apply a Just Noticeable Difference heuristic (JND) in some cases. These aren't going anywhere. (Well, the JND heuristic might but for different reasons than those discussed above.)