chore: Code refactoring (#1482)

* rename shouldRevalidate to revalidate * bind event listeners * refactor shouldDedupe condition * fix conditions
vercel · Sep 22, 2021 · fb3d8b4 · fb3d8b4
1 parent 4f66f5a
commit fb3d8b4
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 35 deletions.
diff --git a/src/use-swr.ts b/src/use-swr.ts
@@ -125,7 +125,11 @@ export const useSWRHandler = <Data = any, Error = any>(
       let startAt: number
       let loading = true
       const opts = revalidateOpts || {}
-      const shouldDedupe = !isUndefined(CONCURRENT_PROMISES[key]) && opts.dedupe
+
+      // If there is no ongoing concurrent request, or `dedupe` is not set, a
+      // new request should be initiated.
+      const shouldStartNewRequest =
+        isUndefined(CONCURRENT_PROMISES[key]) || !opts.dedupe
 
       // Do unmount check for callbacks:
       // If key has changed during the revalidation, or the component has been
@@ -141,29 +145,23 @@ export const useSWRHandler = <Data = any, Error = any>(
         delete CONCURRENT_PROMISES_TS[key]
       }
 
-      // start fetching
+      // Start fetching. Change the `isValidating` state, update the cache.
+      cache.set(keyValidating, true)
+      setState({
+        isValidating: true
+      })
+
       try {
-        cache.set(keyValidating, true)
-        setState({
-          isValidating: true
-        })
-        if (!shouldDedupe) {
-          // also update other hooks
+        if (shouldStartNewRequest) {
+          // Tell all other hooks to change the `isValidating` state.
           broadcastState(
             cache,
             key,
             stateRef.current.data,
             stateRef.current.error,
             true
           )
-        }
 
-        if (shouldDedupe) {
-          // There's already an ongoing request, this one needs to be
-          // deduplicated.
-          startAt = CONCURRENT_PROMISES_TS[key]
-          newData = await CONCURRENT_PROMISES[key]
-        } else {
           // If no cache being rendered currently (it shows a blank page),
           // we trigger the loading slow event.
           if (config.loadingTimeout && !cache.get(key)) {
@@ -174,18 +172,26 @@ export const useSWRHandler = <Data = any, Error = any>(
           }
 
           // Start the request and keep the timestamp.
-          CONCURRENT_PROMISES_TS[key] = startAt = getTimestamp()
-          newData = await (CONCURRENT_PROMISES[key] = fn.apply(fn, fnArgs))
+          CONCURRENT_PROMISES_TS[key] = getTimestamp()
+          CONCURRENT_PROMISES[key] = fn.apply(fn, fnArgs)
+        }
+
+        // Wait until the ongoing request is done. Deduplication is also
+        // considered here.
+        startAt = CONCURRENT_PROMISES_TS[key]
+        newData = await CONCURRENT_PROMISES[key]
 
+        if (shouldStartNewRequest) {
+          // If the request isn't interrupted, clean it up after the
+          // deduplication interval.
           setTimeout(() => {
-            // CONCURRENT_PROMISES_TS[key] maybe be `undefined` or a number.
+            // CONCURRENT_PROMISES_TS[key] maybe be `undefined`.
             if (CONCURRENT_PROMISES_TS[key] === startAt) {
               cleanupState()
             }
           }, config.dedupingInterval)
 
-          // trigger the success event,
-          // only do this for the original request.
+          // Trigger the successful callback.
           if (isCallbackSafe()) {
             getConfig().onSuccess(newData, key, config)
           }
@@ -251,7 +257,7 @@ export const useSWRHandler = <Data = any, Error = any>(
         // merge the new state
         setState(newState)
 
-        if (!shouldDedupe) {
+        if (shouldStartNewRequest) {
           // also update other hooks
           broadcastState(cache, key, newData, newState.error, false)
         }
@@ -273,7 +279,7 @@ export const useSWRHandler = <Data = any, Error = any>(
             isValidating: false,
             error: err as Error
           })
-          if (!shouldDedupe) {
+          if (shouldStartNewRequest) {
             // Broadcast to update the states of other hooks.
             broadcastState(cache, key, UNDEFINED, err, false)
           }

diff --git a/src/utils/broadcast-state.ts b/src/utils/broadcast-state.ts
@@ -8,7 +8,7 @@ export const broadcastState: Broadcaster = (
   data,
   error,
   isValidating,
-  shouldRevalidate = false
+  revalidate
 ) => {
   const [EVENT_REVALIDATORS, STATE_UPDATERS] = SWRGlobalState.get(
     cache
@@ -24,7 +24,7 @@ export const broadcastState: Broadcaster = (
   }
 
   // If we also need to revalidate, only do it for the first hook.
-  if (shouldRevalidate && revalidators && revalidators[0]) {
+  if (revalidate && revalidators && revalidators[0]) {
     return revalidators[0](revalidateEvents.MUTATE_EVENT).then(() =>
       cache.get(key)
     )

diff --git a/src/utils/mutate.ts b/src/utils/mutate.ts
@@ -10,7 +10,7 @@ export const internalMutate = async <Data>(
   cache: Cache,
   _key: Key,
   _data?: Data | Promise<Data | undefined> | MutatorCallback<Data>,
-  shouldRevalidate = true
+  revalidate = true
 ) => {
   const [key, , keyErr] = serialize(_key)
   if (!key) return
@@ -27,7 +27,7 @@ export const internalMutate = async <Data>(
       cache.get(key),
       cache.get(keyErr),
       UNDEFINED,
-      shouldRevalidate
+      revalidate
     )
   }
 
@@ -86,7 +86,7 @@ export const internalMutate = async <Data>(
     data,
     error,
     UNDEFINED,
-    shouldRevalidate
+    revalidate
   )
 
   // Throw error or return data

diff --git a/src/utils/web-preset.ts b/src/utils/web-preset.ts
@@ -12,18 +12,13 @@ let online = true
 const isOnline = () => online
 
 // For node and React Native, `add/removeEventListener` doesn't exist on window.
-const onWindowEvent = hasWindow
-  ? (...args: Parameters<typeof window.addEventListener>) =>
-      window.addEventListener(...args)
-  : noop
+const onWindowEvent = hasWindow ? window.addEventListener.bind(window) : noop
 const onDocumentEvent = hasDocument
-  ? (...args: Parameters<typeof document.addEventListener>) =>
-      document.addEventListener(...args)
+  ? document.addEventListener.bind(document)
   : noop
 const offWindowEvent = (hasWindow && removeEventListener) || noop
 const offDocumentEvent = hasDocument
-  ? (...args: Parameters<typeof document.removeEventListener>) =>
-      document.removeEventListener(...args)
+  ? document.removeEventListener.bind(document)
   : noop
 
 const isVisible = () => {