@@ -76,46 +76,13 @@ export const Spinner: FC<SpinnerProps> = ({
7676 unmountChildrenWhileLoading = false ,
7777 ...delegatedProps
7878} ) => {
79- // Disallow negative timeout values and fractional values, but also round
80- // the delay down if it's small enough that it might as well be immediate
81- // from a user perspective
82- let safeDelay = Math . trunc ( spinnerDelayMs ) ;
83- if ( safeDelay < 100 ) {
84- safeDelay = 0 ;
85- }
86-
87- // Doing a bunch of mid-render state syncs to minimize risks of
88- // contradictory states during re-renders. It's ugly, but it's what the
89- // React team officially recommends. Be very careful with this logic; React
90- // only bails out of redundant state updates if they happen outside of a
91- // render. Inside a render, if you keep calling a state dispatch, you will
92- // get an infinite render loop, no matter what the state value is.
93- const [ cachedDelay , setCachedDelay ] = useState ( safeDelay ) ;
94- const [ delayLapsed , setDelayLapsed ] = useState ( safeDelay === 0 ) ;
95- if ( delayLapsed && ! loading ) {
96- setDelayLapsed ( false ) ;
97- }
98- const delayWasRemovedOnRerender =
99- ! delayLapsed && safeDelay === 0 && cachedDelay !== safeDelay ;
100- if ( delayWasRemovedOnRerender ) {
101- setDelayLapsed ( true ) ;
102- setCachedDelay ( safeDelay ) ;
103- }
104- useEffect ( ( ) => {
105- if ( safeDelay === 0 ) {
106- return ;
107- }
108- const id = window . setTimeout ( ( ) => setDelayLapsed ( true ) , safeDelay ) ;
109- return ( ) => window . clearTimeout ( id ) ;
110- } , [ safeDelay ] ) ;
111-
11279 /**
11380 * @todo Figure out if this conditional logic can ever cause a component to
11481 * lose state when showSpinner flips from false to true while
11582 * unmountedWhileLoading is false. I would hope not, since the children prop
11683 * is the same in both cases, but I need to test this out
11784 */
118- const showSpinner = loading && delayLapsed ;
85+ const showSpinner = useShowSpinner ( loading , spinnerDelayMs ) ;
11986 if ( ! showSpinner ) {
12087 return children ;
12188 }
@@ -159,3 +126,44 @@ export const Spinner: FC<SpinnerProps> = ({
159126 </ >
160127 ) ;
161128} ;
129+
130+ // Split off logic into custom hook so that the main component is insulated from
131+ // some of the necessary chaos from handling re-render logic. A lot of browsers
132+ // should be able to inline this function call anyway
133+ function useShowSpinner ( loading : boolean , spinnerDelayMs : number ) : boolean {
134+ // Disallow negative timeout values and fractional values, but also round
135+ // the delay down if it's small enough that it might as well be immediate
136+ // from a user perspective
137+ let safeDelay = Math . trunc ( spinnerDelayMs ) ;
138+ if ( safeDelay < 100 ) {
139+ safeDelay = 0 ;
140+ }
141+
142+ // Doing a bunch of mid-render state syncs to minimize risks of
143+ // contradictory states during re-renders. It's ugly, but it's what the
144+ // React team officially recommends. Be very careful with this logic; React
145+ // only bails out of redundant state updates if they happen outside of a
146+ // render. Inside a render, if you keep calling a state dispatch, you will
147+ // get an infinite render loop, no matter what the state value is. There
148+ // must be a "base case" that causes re-renders to stabilize/stop
149+ const [ cachedDelay , setCachedDelay ] = useState ( safeDelay ) ;
150+ const [ delayLapsed , setDelayLapsed ] = useState ( safeDelay === 0 ) ;
151+ if ( delayLapsed && ! loading ) {
152+ setDelayLapsed ( false ) ;
153+ }
154+ const delayWasRemovedOnRerender =
155+ ! delayLapsed && safeDelay === 0 && cachedDelay !== safeDelay ;
156+ if ( delayWasRemovedOnRerender ) {
157+ setDelayLapsed ( true ) ;
158+ setCachedDelay ( safeDelay ) ;
159+ }
160+ useEffect ( ( ) => {
161+ if ( safeDelay === 0 ) {
162+ return ;
163+ }
164+ const id = window . setTimeout ( ( ) => setDelayLapsed ( true ) , safeDelay ) ;
165+ return ( ) => window . clearTimeout ( id ) ;
166+ } , [ safeDelay ] ) ;
167+
168+ return delayLapsed && loading ;
169+ }
0 commit comments