Skip to content

Add support for React useEffect hook#5608

Merged
j-f1 merged 7 commits intoprettier:masterfrom
j-f1:react-hook-improved-formatting
Dec 10, 2018
Merged

Add support for React useEffect hook#5608
j-f1 merged 7 commits intoprettier:masterfrom
j-f1:react-hook-improved-formatting

Conversation

@j-f1
Copy link
Copy Markdown
Member

@j-f1 j-f1 commented Dec 7, 2018

Fixes #5376.

Try the playground for this PR

Comment thread tests/break-calls/__snapshots__/jsfmt.spec.js.snap
j-f1 added 2 commits December 8, 2018 07:37
`canHaveTrailingComma` is defined as `I(lastElem && ...)`, which will always be true when `lastElem === null`.
@j-f1 j-f1 requested a review from ikatyang December 8, 2018 23:27
Comment thread src/language-js/printer-estree.js
Comment thread tests/break-calls/__snapshots__/jsfmt.spec.js.snap
Comment thread tests/break-calls/react.js
@j-f1 j-f1 merged commit fa7eafa into prettier:master Dec 10, 2018
@j-f1 j-f1 deleted the react-hook-improved-formatting branch December 10, 2018 11:09
@ikatyang ikatyang added this to the 1.16 milestone Jan 9, 2019

// useEffect(() => { ... }, [foo, bar, baz])
if (
args.length === 2 &&
Copy link
Copy Markdown

@Jessidhia Jessidhia Jan 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit late with this but, this misses useImperativeHandle which also takes a deps array and has a length of 3 when the array is present.

I'm not sure if it'd be that much better to support it, though; it's used rarely and useImperativeHandle specifically has a very short first argument (it's just ref, almost always) which wouldn't look too bad; but what if it catches other custom hooks?

Maybe a heuristic could be that, if the length is 3 and the first argument is very short (3 characters or less) then keep it in one line, otherwise split?

@SimenB
Copy link
Copy Markdown
Contributor

SimenB commented Jan 20, 2019

A case caught by this is array.reduce(func, []) which looks worse, IMO, especially if the callback breaks its args onto multiple lines. Could the detection be expanded to exclude reduce functions?

I especially dislike

      const coveredFilesSortedIntoThresholdGroup = coveredFiles.reduce(
        (files, file) => {
          const pathOrGlobMatches = thresholdGroups.reduce(
            (agg, thresholdGroup) => {

turning into

      const coveredFilesSortedIntoThresholdGroup = coveredFiles.reduce((
        files,
        file,
      ) => {
        const pathOrGlobMatches = thresholdGroups.reduce((
          agg,
          thresholdGroup,
        ) => {

I'll paste the full relevant diff in here:

Details
diff --git c/packages/jest-cli/src/reporters/coverage_reporter.js w/packages/jest-cli/src/reporters/coverage_reporter.js
index 81e45ec08..3523fda1c 100644
--- c/packages/jest-cli/src/reporters/coverage_reporter.js
+++ w/packages/jest-cli/src/reporters/coverage_reporter.js
@@ -212,30 +212,30 @@ export default class CoverageReporter extends BaseReporter {
   _checkThreshold(globalConfig: GlobalConfig, map: CoverageMap) {
     if (globalConfig.coverageThreshold) {
       function check(name, thresholds, actuals) {
-        return ['statements', 'branches', 'lines', 'functions'].reduce(
-          (errors, key) => {
-            const actual = actuals[key].pct;
-            const actualUncovered = actuals[key].total - actuals[key].covered;
-            const threshold = thresholds[key];
-
-            if (threshold != null) {
-              if (threshold < 0) {
-                if (threshold * -1 < actualUncovered) {
-                  errors.push(
-                    `Jest: Uncovered count for ${key} (${actualUncovered})` +
-                      `exceeds ${name} threshold (${-1 * threshold})`,
-                  );
-                }
-              } else if (actual < threshold) {
+        return ['statements', 'branches', 'lines', 'functions'].reduce((
+          errors,
+          key,
+        ) => {
+          const actual = actuals[key].pct;
+          const actualUncovered = actuals[key].total - actuals[key].covered;
+          const threshold = thresholds[key];
+
+          if (threshold != null) {
+            if (threshold < 0) {
+              if (threshold * -1 < actualUncovered) {
                 errors.push(
-                  `Jest: "${name}" coverage threshold for ${key} (${threshold}%) not met: ${actual}%`,
+                  `Jest: Uncovered count for ${key} (${actualUncovered})` +
+                    `exceeds ${name} threshold (${-1 * threshold})`,
                 );
               }
+            } else if (actual < threshold) {
+              errors.push(
+                `Jest: "${name}" coverage threshold for ${key} (${threshold}%) not met: ${actual}%`,
+              );
             }
-            return errors;
-          },
-          [],
-        );
+          }
+          return errors;
+        }, []);
       }
 
       const THRESHOLD_GROUP_TYPES = {
@@ -248,58 +248,58 @@ export default class CoverageReporter extends BaseReporter {
       const groupTypeByThresholdGroup = {};
       const filesByGlob = {};
 
-      const coveredFilesSortedIntoThresholdGroup = coveredFiles.reduce(
-        (files, file) => {
-          const pathOrGlobMatches = thresholdGroups.reduce(
-            (agg, thresholdGroup) => {
-              const absoluteThresholdGroup = path.resolve(thresholdGroup);
-
-              // The threshold group might be a path:
-
-              if (file.indexOf(absoluteThresholdGroup) === 0) {
-                groupTypeByThresholdGroup[thresholdGroup] =
-                  THRESHOLD_GROUP_TYPES.PATH;
-                return agg.concat([[file, thresholdGroup]]);
-              }
+      const coveredFilesSortedIntoThresholdGroup = coveredFiles.reduce((
+        files,
+        file,
+      ) => {
+        const pathOrGlobMatches = thresholdGroups.reduce((
+          agg,
+          thresholdGroup,
+        ) => {
+          const absoluteThresholdGroup = path.resolve(thresholdGroup);
+
+          // The threshold group might be a path:
+
+          if (file.indexOf(absoluteThresholdGroup) === 0) {
+            groupTypeByThresholdGroup[thresholdGroup] =
+              THRESHOLD_GROUP_TYPES.PATH;
+            return agg.concat([[file, thresholdGroup]]);
+          }
 
-              // If the threshold group is not a path it might be a glob:
+          // If the threshold group is not a path it might be a glob:
 
-              // Note: glob.sync is slow. By memoizing the files matching each glob
-              // (rather than recalculating it for each covered file) we save a tonne
-              // of execution time.
-              if (filesByGlob[absoluteThresholdGroup] === undefined) {
-                filesByGlob[absoluteThresholdGroup] = glob
-                  .sync(absoluteThresholdGroup)
-                  .map(filePath => path.resolve(filePath));
-              }
+          // Note: glob.sync is slow. By memoizing the files matching each glob
+          // (rather than recalculating it for each covered file) we save a tonne
+          // of execution time.
+          if (filesByGlob[absoluteThresholdGroup] === undefined) {
+            filesByGlob[absoluteThresholdGroup] = glob
+              .sync(absoluteThresholdGroup)
+              .map(filePath => path.resolve(filePath));
+          }
 
-              if (filesByGlob[absoluteThresholdGroup].indexOf(file) > -1) {
-                groupTypeByThresholdGroup[thresholdGroup] =
-                  THRESHOLD_GROUP_TYPES.GLOB;
-                return agg.concat([[file, thresholdGroup]]);
-              }
+          if (filesByGlob[absoluteThresholdGroup].indexOf(file) > -1) {
+            groupTypeByThresholdGroup[thresholdGroup] =
+              THRESHOLD_GROUP_TYPES.GLOB;
+            return agg.concat([[file, thresholdGroup]]);
+          }
 
-              return agg;
-            },
-            [],
-          );
+          return agg;
+        }, []);
 
-          if (pathOrGlobMatches.length > 0) {
-            return files.concat(pathOrGlobMatches);
-          }
+        if (pathOrGlobMatches.length > 0) {
+          return files.concat(pathOrGlobMatches);
+        }
 
-          // Neither a glob or a path? Toss it in global if there's a global threshold:
-          if (thresholdGroups.indexOf(THRESHOLD_GROUP_TYPES.GLOBAL) > -1) {
-            groupTypeByThresholdGroup[THRESHOLD_GROUP_TYPES.GLOBAL] =
-              THRESHOLD_GROUP_TYPES.GLOBAL;
-            return files.concat([[file, THRESHOLD_GROUP_TYPES.GLOBAL]]);
-          }
+        // Neither a glob or a path? Toss it in global if there's a global threshold:
+        if (thresholdGroups.indexOf(THRESHOLD_GROUP_TYPES.GLOBAL) > -1) {
+          groupTypeByThresholdGroup[THRESHOLD_GROUP_TYPES.GLOBAL] =
+            THRESHOLD_GROUP_TYPES.GLOBAL;
+          return files.concat([[file, THRESHOLD_GROUP_TYPES.GLOBAL]]);
+        }
 
-          // A covered file that doesn't have a threshold:
-          return files.concat([[file, undefined]]);
-        },
-        [],
-      );
+        // A covered file that doesn't have a threshold:
+        return files.concat([[file, undefined]]);
+      }, []);
 
       const getFilesInThresholdGroup = thresholdGroup =>
         coveredFilesSortedIntoThresholdGroup
diff --git c/packages/jest-cli/src/reporters/utils.js w/packages/jest-cli/src/reporters/utils.js
index 997ca3a0a..84d035488 100644
--- c/packages/jest-cli/src/reporters/utils.js
+++ w/packages/jest-cli/src/reporters/utils.js
@@ -241,35 +241,32 @@ export const wrapAnsiString = (string: string, terminalWidth: number) => {
   let lastLineLength = 0;
 
   return tokens
-    .reduce(
-      (lines, [kind, token]) => {
-        if (kind === 'string') {
-          if (lastLineLength + token.length > terminalWidth) {
-            while (token.length) {
-              const chunk = token.slice(0, terminalWidth - lastLineLength);
-              const remaining = token.slice(
-                terminalWidth - lastLineLength,
-                token.length,
-              );
-              lines[lines.length - 1] += chunk;
-              lastLineLength += chunk.length;
-              token = remaining;
-              if (token.length) {
-                lines.push('');
-                lastLineLength = 0;
-              }
+    .reduce((lines, [kind, token]) => {
+      if (kind === 'string') {
+        if (lastLineLength + token.length > terminalWidth) {
+          while (token.length) {
+            const chunk = token.slice(0, terminalWidth - lastLineLength);
+            const remaining = token.slice(
+              terminalWidth - lastLineLength,
+              token.length,
+            );
+            lines[lines.length - 1] += chunk;
+            lastLineLength += chunk.length;
+            token = remaining;
+            if (token.length) {
+              lines.push('');
+              lastLineLength = 0;
             }
-          } else {
-            lines[lines.length - 1] += token;
-            lastLineLength += token.length;
           }
         } else {
           lines[lines.length - 1] += token;
+          lastLineLength += token.length;
         }
+      } else {
+        lines[lines.length - 1] += token;
+      }
 
-        return lines;
-      },
-      [''],
-    )
+      return lines;
+    }, [''])
     .join('\n');
 };

@j-f1
Copy link
Copy Markdown
Member Author

j-f1 commented Jan 20, 2019

How about only detecting () => style functions?

@SimenB
Copy link
Copy Markdown
Contributor

SimenB commented Jan 20, 2019

Yeah, that'd work! 🙂

@SimenB
Copy link
Copy Markdown
Contributor

SimenB commented Jan 20, 2019

Opened up #5778 for it 🙂

@lock lock Bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Apr 20, 2019
@lock lock Bot locked as resolved and limited conversation to collaborators Apr 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Formatting: inline function with multiple arguments

8 participants