From ae176f2e790bb61144eb071acc1c22616fe1401e Mon Sep 17 00:00:00 2001 From: Georgii Dolzhykov Date: Mon, 12 Apr 2021 18:15:46 +0300 Subject: [PATCH] Process conditional groups in mapDoc (#10695) --- changelog_unreleased/javascript/10695.md | 40 ++++++++++++ src/document/doc-utils.js | 57 ++++++++++++----- .../__snapshots__/jsfmt.spec.js.snap | 61 +++++++++++++++++++ tests/js/multiparser-html/issue-10691.js | 10 +++ 4 files changed, 153 insertions(+), 15 deletions(-) create mode 100644 changelog_unreleased/javascript/10695.md create mode 100644 tests/js/multiparser-html/issue-10691.js diff --git a/changelog_unreleased/javascript/10695.md b/changelog_unreleased/javascript/10695.md new file mode 100644 index 000000000..8d64d832a --- /dev/null +++ b/changelog_unreleased/javascript/10695.md @@ -0,0 +1,40 @@ +#### Process conditional groups in `mapDoc` (#10695 by @thorn0) + +In particular, this fixes broken substitutions in HTML-in-JS. + + +```jsx +// Input +export default function include_photoswipe( + gallery_selector = ".my-gallery" +): string { + return /* HTML */ ` + `; +} + +// Prettier stable +export default function include_photoswipe( + gallery_selector = ".my-gallery" +): string { + return /* HTML */ ` `; +} + +// Prettier main +export default function include_photoswipe( + gallery_selector = ".my-gallery" +): string { + return /* HTML */ ` `; +} +``` diff --git a/src/document/doc-utils.js b/src/document/doc-utils.js index 084844cf4..a57e0a339 100644 --- a/src/document/doc-utils.js +++ b/src/document/doc-utils.js @@ -71,26 +71,53 @@ function traverseDoc(doc, onEnter, onExit, shouldTraverseConditionalGroups) { } function mapDoc(doc, cb) { - if (Array.isArray(doc)) { - return cb(doc.map((part) => mapDoc(part, cb))); + // Within a doc tree, the same subtrees can be found multiple times. + // E.g., often this happens in conditional groups. + // As an optimization (those subtrees can be huge) and to maintain the + // reference structure of the tree, the mapping results are cached in + // a map and reused. + const mapped = new Map(); + + return rec(doc); + + function rec(doc) { + if (mapped.has(doc)) { + return mapped.get(doc); + } + const result = process(doc); + mapped.set(doc, result); + return result; } - if (doc.type === "concat" || doc.type === "fill") { - const parts = doc.parts.map((part) => mapDoc(part, cb)); - return cb({ ...doc, parts }); - } + function process(doc) { + if (Array.isArray(doc)) { + return cb(doc.map(rec)); + } - if (doc.type === "if-break") { - const breakContents = doc.breakContents && mapDoc(doc.breakContents, cb); - const flatContents = doc.flatContents && mapDoc(doc.flatContents, cb); - return cb({ ...doc, breakContents, flatContents }); - } + if (doc.type === "concat" || doc.type === "fill") { + const parts = doc.parts.map(rec); + return cb({ ...doc, parts }); + } - if (doc.contents) { - const contents = mapDoc(doc.contents, cb); - return cb({ ...doc, contents }); + if (doc.type === "if-break") { + const breakContents = doc.breakContents && rec(doc.breakContents); + const flatContents = doc.flatContents && rec(doc.flatContents); + return cb({ ...doc, breakContents, flatContents }); + } + + if (doc.type === "group" && doc.expandedStates) { + const expandedStates = doc.expandedStates.map(rec); + const contents = expandedStates[0]; + return cb({ ...doc, contents, expandedStates }); + } + + if (doc.contents) { + const contents = rec(doc.contents); + return cb({ ...doc, contents }); + } + + return cb(doc); } - return cb(doc); } function findInDoc(doc, fn, defaultValue) { diff --git a/tests/js/multiparser-html/__snapshots__/jsfmt.spec.js.snap b/tests/js/multiparser-html/__snapshots__/jsfmt.spec.js.snap index 9183dad71..ca455f105 100644 --- a/tests/js/multiparser-html/__snapshots__/jsfmt.spec.js.snap +++ b/tests/js/multiparser-html/__snapshots__/jsfmt.spec.js.snap @@ -167,6 +167,67 @@ setFoo( ================================================================================ `; +exports[`issue-10691.js - {"htmlWhitespaceSensitivity":"ignore"} format 1`] = ` +====================================options===================================== +htmlWhitespaceSensitivity: "ignore" +parsers: ["babel", "flow", "typescript"] +printWidth: 80 + | printWidth +=====================================input====================================== +export default function include_photoswipe( + gallery_selector = ".my-gallery" +) { + return /* HTML */ \` + \`; +} + +=====================================output===================================== +export default function include_photoswipe(gallery_selector = ".my-gallery") { + return /* HTML */ \` + + \`; +} + +================================================================================ +`; + +exports[`issue-10691.js format 1`] = ` +====================================options===================================== +parsers: ["babel", "flow", "typescript"] +printWidth: 80 + | printWidth +=====================================input====================================== +export default function include_photoswipe( + gallery_selector = ".my-gallery" +) { + return /* HTML */ \` + \`; +} + +=====================================output===================================== +export default function include_photoswipe(gallery_selector = ".my-gallery") { + return /* HTML */ \` \`; +} + +================================================================================ +`; + exports[`lit-html.js - {"htmlWhitespaceSensitivity":"ignore"} format 1`] = ` ====================================options===================================== htmlWhitespaceSensitivity: "ignore" diff --git a/tests/js/multiparser-html/issue-10691.js b/tests/js/multiparser-html/issue-10691.js new file mode 100644 index 000000000..9761dcb69 --- /dev/null +++ b/tests/js/multiparser-html/issue-10691.js @@ -0,0 +1,10 @@ +export default function include_photoswipe( + gallery_selector = ".my-gallery" +) { + return /* HTML */ ` + `; +}