Chore: Clean anti-patterns of early returns in long functions

SBoudrias · Oct 8, 2023 · f6a3c50 · f6a3c50
1 parent ba042f9
commit f6a3c50
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 32 deletions.
diff --git a/packages/checkbox/src/index.mts b/packages/checkbox/src/index.mts
@@ -102,14 +102,18 @@ export default createPrompt(
         setStatus('done');
         done(items.filter(isChecked).map((choice) => choice.value));
       } else if (isUpKey(key) || isDownKey(key)) {
-        if (!loop && active === bounds.first && isUpKey(key)) return;
-        if (!loop && active === bounds.last && isDownKey(key)) return;
-        const offset = isUpKey(key) ? -1 : 1;
-        let next = active;
-        do {
-          next = (next + offset + items.length) % items.length;
-        } while (!isSelectable(items[next]!));
-        setActive(next);
+        if (
+          loop ||
+          (isUpKey(key) && active !== bounds.first) ||
+          (isDownKey(key) && active !== bounds.last)
+        ) {
+          const offset = isUpKey(key) ? -1 : 1;
+          let next = active;
+          do {
+            next = (next + offset + items.length) % items.length;
+          } while (!isSelectable(items[next]!));
+          setActive(next);
+        }
       } else if (isSpaceKey(key)) {
         setShowHelpTip(false);
         setItems(items.map((choice, i) => (i === active ? toggle(choice) : choice)));
@@ -124,9 +128,10 @@ export default createPrompt(
         // Adjust index to start at 1
         const position = Number(key.name) - 1;
         const item = items[position];
-        if (item == null || !isSelectable(item)) return;
-        setActive(position);
-        setItems(items.map((choice, i) => (i === position ? toggle(choice) : choice)));
+        if (item != null && isSelectable(item)) {
+          setActive(position);
+          setItems(items.map((choice, i) => (i === position ? toggle(choice) : choice)));
+        }
       }
     });
 

diff --git a/packages/core/core.test.mts b/packages/core/core.test.mts
@@ -347,16 +347,12 @@ describe('createPrompt()', () => {
       useKeypress((event: KeypressEvent) => {
         if (isEnterKey(event)) {
           done(lastKeypress);
-          return;
-        }
-
-        // Space will just trigger a re-render
-        if (isSpaceKey(event)) {
+        } else if (isSpaceKey(event)) {
+          // Space will just trigger a re-render
           setIndex(index + 1);
-          return;
+        } else {
+          setLastKeypress(event.name);
         }
-
-        setLastKeypress(event.name);
       });
 
       return `${config.message} ${displayKeypress}`;

diff --git a/packages/inquirer/README.md b/packages/inquirer/README.md
@@ -170,10 +170,10 @@ A question object is a `hash` containing question related values:
       if (typeof input !== 'number') {
         // Pass the return value in the done callback
         done('You need to provide a number');
-        return;
+      } else {
+        // Pass the return value in the done callback
+        done(null, true);
       }
-      // Pass the return value in the done callback
-      done(null, true);
     }, 3000);
   }
 }

diff --git a/packages/select/src/index.mts b/packages/select/src/index.mts
@@ -86,19 +86,24 @@ export default createPrompt(
         setStatus('done');
         done(selectedChoice.value);
       } else if (isUpKey(key) || isDownKey(key)) {
-        if (!loop && active === bounds.first && isUpKey(key)) return;
-        if (!loop && active === bounds.last && isDownKey(key)) return;
-        const offset = isUpKey(key) ? -1 : 1;
-        let next = active;
-        do {
-          next = (next + offset + items.length) % items.length;
-        } while (!isSelectable(items[next]!));
-        setActive(next);
+        if (
+          loop ||
+          (isUpKey(key) && active !== bounds.first) ||
+          (isDownKey(key) && active !== bounds.last)
+        ) {
+          const offset = isUpKey(key) ? -1 : 1;
+          let next = active;
+          do {
+            next = (next + offset + items.length) % items.length;
+          } while (!isSelectable(items[next]!));
+          setActive(next);
+        }
       } else if (isNumberKey(key)) {
         const position = Number(key.name) - 1;
         const item = items[position];
-        if (item == null || !isSelectable(item)) return;
-        setActive(position);
+        if (item != null && isSelectable(item)) {
+          setActive(position);
+        }
       }
     });