Fix unstable embedded template literals#13532
Conversation
| foo(/* HTML */ `<!-- bar1 --> | ||
| bar | ||
| <!-- bar2 -->`); |
There was a problem hiding this comment.
For this specific case hugging actually doesn't look good to me.
Probably, it'd make sense to distinguish between these two cases and do hugging only for the 2nd:
// 1
foo(
/* HTML */ `<!-- bar1 -->
bar
<!-- bar2 -->`
);
// 2
foo(/* HTML */ `
<!-- bar1 -->
bar
<!-- bar2 -->
`);Thoughts on whether this makes sense and on how to implement this (I guess we'd need to somehow check if there is a line in front of the closing `)?
upd: I did this for HTML embeds
|
Using substrings in label checks instead of what clearly should be a list looks clumsy. It'd be more clear to extend the type of label docs (see #11205), but I decided not to do that in this PR. |
| ] | ||
| ); | ||
|
|
||
| return linebreak ? resultDoc : label("[no-hug]", resultDoc); |
There was a problem hiding this comment.
label seems not limited to string, how about
return label({hug: false}, ...)
|
|
||
| function label(label, contents) { | ||
| return { type: DOC_TYPE_LABEL, label, contents }; | ||
| return label ? { type: DOC_TYPE_LABEL, label, contents } : contents; |
There was a problem hiding this comment.
For building code to be more compact. E.g.
label(
typeof doc.label === "object" && { commented: true, ...doc.label },
[leading, doc, trailing]
)No need to create an intermediate variable for [leading, doc, trailing].
There was a problem hiding this comment.
If the doc.label is null, you can still spread in object expression, they result the same, 0, false, undefined, "" won't pass typeof check
If the doc.label is null, 0, false, undefined, "" , you can still spread, they result the same.
There was a problem hiding this comment.
doc.label can't be null because the label builder doesn't accept falsy values. So this code adds a type: "label" doc wrapper only if doc is a label itself and its .label is an object. This way things like .label.embed and .label.hug are propagated. Otherwise there is nothing to propagate, so it returns [leading, doc, trailing] without wrapping.
There was a problem hiding this comment.
I'm confusing myself.... you want to use the code above? Then the deleted comment seems more correct.
There was a problem hiding this comment.
That's right, but the concat builder had more explicit intention than arrays too. I expect we're going to have more code like this (relying on propagated labels). It's good to be explicit in one-off scenarios, but not in something repeated.
There was a problem hiding this comment.
As a trade-off for explicitness, a conditionalLabel builder could be added, but I'm not convinced it's really needed.
There was a problem hiding this comment.
Won't block this, just thought it's unnecessary limitation.
There was a problem hiding this comment.
concat is a removed thing
Right. But it was explicit. It's just another instance of the typical "DRY vs explicit" tradeoff.
Won't block this, just thought it's unnecessary limitation.
I saw a reusable pattern. I used it at least 3 times only in this PR and expect it to be used more. So I encapsulated it into the function.
|
Ouch! Sometimes it's not wrong to have more than one changelog file in 1 PR. |
My idea is to prevent accidentally "copy file" instead of "move file". You can remove that check. |
|
Filename of those files shouldn't used when generating blog post. You can also update prettier/scripts/lint-changelog.mjs Line 65 in 822f354 13532-2.md
|
This is supposed to fix unstable embedded template literals. Ref prettier/prettier#13532
2 participants
Description
Fixes #6150
Fixes #11205
Template literals with embedded languages are hugged now if they're the only argument and have leading and trailing whitespace. The idempotence issue has been fixed.
Checklist
docs/directory).changelog_unreleased/*/XXXX.mdfile followingchangelog_unreleased/TEMPLATE.md.✨Try the playground for this PR✨