diff --git a/changelog_unreleased/javascript/10222.md b/changelog_unreleased/javascript/10222.md index d21c50f3f..77a153961 100644 --- a/changelog_unreleased/javascript/10222.md +++ b/changelog_unreleased/javascript/10222.md @@ -1,4 +1,4 @@ -#### Format assignments more consistently (#10222 by @thorn0) +#### Format assignments more consistently (#10222, #10643 by @thorn0) Previously, Prettier had a lot of trouble with figuring out how to break lines in assignments. E.g., long right-hand sides often stayed unbroken. Not anymore. diff --git a/src/language-js/print/assignment.js b/src/language-js/print/assignment.js index a72c952ae..729ce0101 100644 --- a/src/language-js/print/assignment.js +++ b/src/language-js/print/assignment.js @@ -49,6 +49,9 @@ function printAssignment( ]); } + case "break-lhs": + return group([leftDoc, operator, " ", group(rightDoc)]); + // Parts of assignment chains aren't wrapped in groups. // Once one of them breaks, the chain breaks too. case "chain": @@ -96,7 +99,7 @@ function chooseLayout(path, options, leftDoc, rightPropertyName) { const isTail = !isAssignment(rightNode); const shouldUseChainFormatting = path.match( isAssignment, - (node) => isAssignment(node) || node.type === "VariableDeclarator", + isAssignmentOrVariableDeclarator, (node) => !isTail || (node.type !== "ExpressionStatement" && @@ -124,6 +127,10 @@ function chooseLayout(path, options, leftDoc, rightPropertyName) { return "never-break-after-operator"; } + if (isComplexDestructuring(node)) { + return "break-lhs"; + } + // wrapping object properties with very short keys usually doesn't add much value const hasShortKey = isObjectPropertyWithShortKey(node, leftDoc, options); @@ -195,10 +202,34 @@ function shouldNeverBreakAfterOperator(rightNode) { ); } +// prefer to break destructuring assignment +// if it includes default values or non-shorthand properties +function isComplexDestructuring(node) { + if (isAssignmentOrVariableDeclarator(node)) { + const leftNode = node.left || node.id; + return ( + leftNode.type === "ObjectPattern" && + leftNode.properties.length > 2 && + leftNode.properties.some( + (property) => + (property.type === "ObjectProperty" || + property.type === "Property") && + (!property.shorthand || + (property.value && property.value.type === "AssignmentPattern")) + ) + ); + } + return false; +} + function isAssignment(node) { return node.type === "AssignmentExpression"; } +function isAssignmentOrVariableDeclarator(node) { + return isAssignment(node) || node.type === "VariableDeclarator"; +} + function isMemberExpressionChainHead(node) { return node.type === "Identifier" || node.type === "ThisExpression"; } diff --git a/src/language-js/print/object.js b/src/language-js/print/object.js index ac66f587d..475955062 100644 --- a/src/language-js/print/object.js +++ b/src/language-js/print/object.js @@ -225,7 +225,16 @@ function printObject(path, options, print) { name === "this" || name === "rest") && number === 0 - ) + ) || + // Assignment printing logic (printAssignment) is responsible + // for adding a group if needed + (!shouldBreak && + path.match( + (node) => node.type === "ObjectPattern", + (node) => + node.type === "AssignmentExpression" || + node.type === "VariableDeclarator" + )) ) { return content; } diff --git a/tests/js/assignment/__snapshots__/jsfmt.spec.js.snap b/tests/js/assignment/__snapshots__/jsfmt.spec.js.snap index 29d8b5893..7dbc5bdd2 100644 --- a/tests/js/assignment/__snapshots__/jsfmt.spec.js.snap +++ b/tests/js/assignment/__snapshots__/jsfmt.spec.js.snap @@ -193,6 +193,91 @@ const { ================================================================================ `; +exports[`destructuring-array.js format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 + | printWidth +=====================================input====================================== +const [ + width= nextWidth, + height= nextHeight, + baseline= nextBaseline, +] = measureText(nextText, getFontString(element)); + +=====================================output===================================== +const [width = nextWidth, height = nextHeight, baseline = nextBaseline] = + measureText(nextText, getFontString(element)); + +================================================================================ +`; + +exports[`destructuring-heuristic.js format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 + | printWidth +=====================================input====================================== +{{ + const { + id, + static: isStatic, + method: isMethod, + methodId, + getId, + setId, + } = privateNamesMap.get(name); + + const { + id1, + method: isMethod1, + methodId1 + } = privateNamesMap.get(name); + + const { + id2, + method: isMethod2, + methodId2 + } = privateNamesMap.get(bifornCringerMoshedPerplexSawder); + + const { + id3, + method: isMethod3, + methodId3 + } = anodyneCondosMalateOverateRetinol.get(bifornCringerMoshedPerplexSawder); +}} + +=====================================output===================================== +{ + { + const { + id, + static: isStatic, + method: isMethod, + methodId, + getId, + setId, + } = privateNamesMap.get(name); + + const { id1, method: isMethod1, methodId1 } = privateNamesMap.get(name); + + const { + id2, + method: isMethod2, + methodId2, + } = privateNamesMap.get(bifornCringerMoshedPerplexSawder); + + const { + id3, + method: isMethod3, + methodId3, + } = anodyneCondosMalateOverateRetinol.get(bifornCringerMoshedPerplexSawder); + } +} + +================================================================================ +`; + exports[`issue-1419.js format 1`] = ` ====================================options===================================== parsers: ["babel", "flow", "typescript"] diff --git a/tests/js/assignment/destructuring-array.js b/tests/js/assignment/destructuring-array.js new file mode 100644 index 000000000..66490e60c --- /dev/null +++ b/tests/js/assignment/destructuring-array.js @@ -0,0 +1,5 @@ +const [ + width= nextWidth, + height= nextHeight, + baseline= nextBaseline, +] = measureText(nextText, getFontString(element)); diff --git a/tests/js/assignment/destructuring-heuristic.js b/tests/js/assignment/destructuring-heuristic.js new file mode 100644 index 000000000..30c7625f0 --- /dev/null +++ b/tests/js/assignment/destructuring-heuristic.js @@ -0,0 +1,28 @@ +{{ + const { + id, + static: isStatic, + method: isMethod, + methodId, + getId, + setId, + } = privateNamesMap.get(name); + + const { + id1, + method: isMethod1, + methodId1 + } = privateNamesMap.get(name); + + const { + id2, + method: isMethod2, + methodId2 + } = privateNamesMap.get(bifornCringerMoshedPerplexSawder); + + const { + id3, + method: isMethod3, + methodId3 + } = anodyneCondosMalateOverateRetinol.get(bifornCringerMoshedPerplexSawder); +}} diff --git a/tests/js/destructuring/__snapshots__/jsfmt.spec.js.snap b/tests/js/destructuring/__snapshots__/jsfmt.spec.js.snap index 6535aa58f..1659a2d8c 100644 --- a/tests/js/destructuring/__snapshots__/jsfmt.spec.js.snap +++ b/tests/js/destructuring/__snapshots__/jsfmt.spec.js.snap @@ -126,8 +126,11 @@ printWidth: 80 const { foo, bar: bazAndSomething, quxIsLong } = someBigFunctionName("foo")("bar"); =====================================output===================================== -const { foo, bar: bazAndSomething, quxIsLong } = - someBigFunctionName("foo")("bar"); +const { + foo, + bar: bazAndSomething, + quxIsLong, +} = someBigFunctionName("foo")("bar"); ================================================================================ `; diff --git a/tests/js/method-chain/__snapshots__/jsfmt.spec.js.snap b/tests/js/method-chain/__snapshots__/jsfmt.spec.js.snap index a0fa2eb4c..1c275b51b 100644 --- a/tests/js/method-chain/__snapshots__/jsfmt.spec.js.snap +++ b/tests/js/method-chain/__snapshots__/jsfmt.spec.js.snap @@ -214,8 +214,14 @@ expect( .style.paddingRight ).toBe("1000px"); -const { course, conflicts = [], index, scheduleId, studentId, something } = - a.this.props; +const { + course, + conflicts = [], + index, + scheduleId, + studentId, + something, +} = a.this.props; const { course2,