New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Prettier does not respect existing method chain breaks #7884
Comments
Kinda related to #4765 |
Conceptually, Prettier doesn't preserve the existing formatting, it removes it. 🔥🔥🔥 There are a few exceptions to this (object literals, method decorators, and some cases in JSX), but those are cases where a better trade-off couldn't be found at all. I'm not convinced that method chains are such a stalemate case. If the lines Prettier generates are too long and unreadable, the general advice is to reduce
But if we're sure that we want to change the current output, the first thing we should do is try to tweak the heuristic. E.g. we can add a new rule: if a chain contains a call with >= 3 args, always split it onto multiple lines. Only if we completely fail to find a good heuristic should we reach for the last resort, preserving the original formatting. What is a good heuristic though? If it yields good results say in 95% of the cases, is it good enough? Does it matter if its results are subpar when |
Arguably, no, since Prettier is widely used and many people will run into the 5% every day.
👍 |
No, that's not the same thing as "if a chain contains a call with >= 3 args..." |
big +1 on the request. IMO: it's now as annoying as before for a different set of use-cases, notably fluent APIs. As a library author with dozen builder types libs with fluent apis, hundreds of files are now beeing reformated in a non consistent / weird and not readable way. I now need to go and edit all those files to add comments in the middle of each chain to ensure lines are broken correctly. Is there a way to keep using the previous heuristic ? here is an example I'm editing right now : domain.concept('Page').val('title', 'string').vals('widgets', 'Widget')
domain
.concept('Widget')
.val('title', 'string')
.val('color', 'Color')
.val('foo', 'Foo')
.val('bar', 'Bar')
domain.concept('Widget').val('title', 'string').val('color', 'Color') here is what it used to be domain
.concept('Page')
.val('title', 'string')
.vals('widgets', 'Widget')
domain
.concept('Widget')
.val('title', 'string')
.val('color', 'Color')
.val('foo', 'Foo')
.val('bar', 'Bar')
domain // <-- I now manually add a comment here
.concept('Widget')
.val('title', 'string')
.val('color', 'Color') |
I tend to think they do under the same circumstances => in some cases: this is especially true in fluent apis / builder-like apis |
BTW, this sounds like a great source of information for improving the heuristic! What do those weirdly formatted chains have in common structurally? |
Hi @thorn0 , thanks for your answers !
I have no strong opinion on this but tends to disagree.
Prettier found a very nice in-between IMO.
For prettier devs, I believe you it is a PITA.
It may help some cases, but not most fluent APIs / Builder I can think of. I tried the same example as shown above in the playground for your PR and this is the result
Good question;
=> so I really don't see lots of ways to deal with this beside preserving indentation. this being said, I'm used to adding empty comments to force indentation when I really want to, and the benefit of prettier offsets this very minor annoyance by a HGUE margin, so it's not the end of the world for me. It's just slightly worse. I can also say that I've written a fair amount of typescript over the years. Sometimes, Prettier euristics is not perfect, but it's usually great, and I usually don't care about it. When I do care , I always find a way to force things either with comments, or with //prettier-ignore } (for instance, from time to time, Igetters spanning on 3 lines even when they fit in a few chars. For those cases, and when I have lots of them but want to see the whole file at once, I add a few //prettier-ignore at the end of the line like so: but that's about it. It's the first time I felt the need to come report how this change affected me in a negative way. |
For now, I'll just type [ and will just loose 3 keystrokes in the process. The drawbacks are :
|
By doing so, I believe you're destroying a very important piece of information: intent. No (flexible) heuristic will be able to transform s.split('').map(Number).sort() into s.split('').map(Number).sort() (i.e. don't transform at all) while also transforming domain.concept('Page').val('title', 'string').vals('widgets', 'Widget') into domain
.concept('Page')
.val('title', 'string')
.vals('widgets', 'Widget') Both snippets have the same-chain length, and comparable argument-complexity and text-width. |
How can we distinguish between a valuable intentional line break and a random line break that doesn't convey any important information and was inserted, for example, by Prettier's previous run because the line used to be long?
I'm not sure about that. The total amount of arguments is different. 2 vs 5. The first chain includes only calls with 0 or 1 argument. It's definitely something we can work with. |
I agree, it's not an easy problem to solve.
Sure, that's why I included the flexible part. The example could just as well have been: s.split(';').splice.(2, 5).map(Number).sort() and domain
.concept('Page')
.val('title')
.vals() I guess, no matter what combinations of statistics you'll utilize, it will always create a false dichotomy.
That would be pretty difficult. But I would argue that devising a heuristic for solving the command-chain problem would be at least as difficult. |
I really don't understand why it's a problem if domain
.concept('Page')
.val('title')
.vals() gets reformatted into domain.concept('Page').val('title').vals(); I mean I see the problem in the original post in this issue, but here? I expect that the answer is along the lines of "other |
I would really like if anyone here who wants to say something in support of preserving existing new lines first reflected on #2068 (comment) and realized how such situations defeat the whole point of Prettier. Thanks is advance. |
we can't, and that's exactly why I would like Prettier to preserve my line breaks there. To be honest, that's a very bad reason why it's a PITA and I'm totally unconvinced.
Sure, why not adding a flag to remove all whitespace related preserving... but I'd rather solve the bug and leave the features instead if "the majority of people seem to want this level of control" 🤔 IMO: if someone has problem with CI because of some tooling, and decide to worsen the code to please the CI, there is a problem going on.
I do; for the same reason users want the power to choose if objects are multiline or not. this is quite visible here, and easy to read: domain
.concept('Page')
.val('title', 'string')
.vals('widgets', 'Widget')
domain
.concept('Widget')
.val('title', 'string')
.val('color', 'Color')
.val('foo', 'Foo')
.val('bar', 'Bar')
domain // <-- I now manually add a comment here
.concept('Widget')
.val('title', 'string')
.val('color', 'Color') But in the below example, it's way less clear. domain.concept('Page').val('title', 'string').vals('widgets', 'Widget')
domain
.concept('Widget')
.val('title', 'string')
.val('color', 'Color')
.val('foo', 'Foo')
.val('bar', 'Bar')
domain.concept('Widget').val('title', 'string').val('color', 'Color') It's even worse in lots of cases where I have
as @RikJansen81 said, it's about preserving intent (& clear readability) conveyed by formatting, while still not caring about mundane formating mostly unrelated to intent. In my current project (~500 files) where I use several fluent APIs / "builder" classes, I can assure you my code seems globally worse now after applying the formatting. I think I have a pragmatic aproach to line breaks. I only care when it boost readability a lot. In this case, I really feel the current default is globally slightly worse. But again, I can fix it easilly by adding comments, so it's not a major problem. I redirected people from #4765 (comment) here :) Thanks again for your time |
Prettier is well past that phase when a GitHub issue or a Twitter poll can be used to understand what the majority of users wants. As for #2068, preserving new lines is a destructive suggestion because it provokes merge conflicts and preserving at least as much noise as "valuable line breaks". It's not about some CI. Where did you read that? Again, let's try and think of heuristic improvements. How about special-casing expression statements somehow? |
indeed, nothing about CI, I made a shortcut in my mind, sorry, but it doesnt change much IMO. You redirected me to #2068 : in #2068 , I read in the original post
so basically, what I read is :
Again, I don't see at all why it makes line preserving a PITA for end-users.
I understand you want to go back to thinking about heuristic improvements. but I think it misses the point that there are cases where no heuristics can do anything. Focusing on improving heuristics seems to be narrowing the debate right off the bat. IMO, It's a matter of: does presentation has enough semantic information to be worth preserving in some cases. My opinion is yes, and improving heuristics doesn't seem to help me as @RikJansen81 (sorry for the noise if this was clear already. I'll continue watching the thread but won't add more informations unless requested. Thanks for you time) |
Not a bug. The line break preserving behavior. Resolving merge conflicts is not unrelated to code quality.
Prettier has Another problem with object literals is that users have to know to remove the new line before the first property for an object literal to collapse, but of course many of them don't know about that because it's counter-intuitive and because they naively think the formatting is fully automated. Prettier should remain a simple automatic tool, not a semi-automatic one with esoteric tricks the user has to be aware about. |
there are many style guides and linter rules (eslint, tslint,..) that are recommending or even forcing a new line per chained call, so it is obvious that there is a not inconsiderable number of users who want these line breaks to be preserved and obviously no heuristic improvement would help. |
/// yay
str
.split()
.join()
domain
.concept('Page')
.val('title', 'string')
.vals('widgets', 'Widget')
domain
.concept('Widget')
.val('title', 'string')
.val('color', 'Color')
.val('foo', 'Foo')
.val('bar', 'Bar')
domain
.concept('Widget')
.val('title', 'string')
.val('color', 'Color') /// meh
str.split().join()
domain.concept('Page').val('title', 'string').vals('widgets', 'Widget')
domain
.concept('Widget')
.val('title', 'string')
.val('color', 'Color')
.val('foo', 'Foo')
.val('bar', 'Bar')
domain.concept('Widget').val('title', 'string').val('color', 'Color') The reason why I personally think it's unconvinient it's because it's not stable. And also from perspective "one line - one instruction" and in our case "one function call" sounds sane to me. So for for me
|
Just as @rvion , I fail to see the relevance of that issue to ours. Firstly because I have a hard time understanding the issue described there as the problem is nowhere clearly stated in a reproducible fashion. The best I can make of it is that the commenter is stating that the same input produces different output.
Either way, why is this an illustration that preserving line breaks is a PITA?
If the intent of the code changed or it the new formatting conveys the semantics of the code in a different/clearer way, I would say that should provoke a merge conflict. Code is written for coders and there is a good reason we are not directly coding in ASTs. Of course, I do understand this can open up a whole new can of worms, because discussions will arise of what formatting will convey intent in the best manner, which seems like a type of issue Prettier tries to solve, but I feel these types of discussions (regarding intent) are one abstraction level higher than the ones Prettier should try to solve. That issue did lead me to this section in the rationale:
It seems that our request aligns very will this rationale.
I would also like to come back on this quote, because I fail to see why Prettier would need to solve that problem at all:
What is exactly meant with expression statement, I see a lot of different definition of this term online. |
Sorry, I really have no time for the discussion. If anyone has interesting counter-examples for the heuristic proposed in #7889, please write them here or there. |
This comment has been minimized.
This comment has been minimized.
I see the reasoning behind what prompted the heuristics change, but on a daily basis the new heuristics are pretty annoying to use. I want my formatting to be consistent, not all over the place, which is the result of the current heuristics. // 79 characters
string
.replace(regex, '')
.replace(regex2, ',')
.replace(regex3, '%')
.split(',')
// 80+ characters
string
.replace(regex, '')
.replace(regex2, ',')
.replace(regex3, '%')
.split(',')
.map(mapFunc) the code above annoyingly turns into // 79 characters
string.replace(regex, '').replace(regex2, ',').replace(regex3, '%').split(',')
// 80+ characters
string
.replace(regex, '')
.replace(regex2, ',')
.replace(regex3, '%')
.split(',')
.map(mapFunc) They do pretty much the same thing, except the 80+ example has one more method, I want to be able to easily read what they are doing and understand that they are both similar with one small change, but since they aren't formatted the same it is harder to understand it at a glance. I believe for method chain breaks manual multi-line should be kept. |
It would help if I could have a comment at the end of the line that I want to force wrapping, and Prettier would leave it alone, but it doesn't: Original:
Prettier says:
Isn't this a bug? I should be able to comment on the line and not have Prettier insert another line's code there that may not pertain to the comment. Apparently this was possible in the past but has gotten even less user-friendly. I feel like there should be a post-processor specifically for handling chaining. |
A project I'm keeping an eye on: https://dprint.dev/. It's got some way to go but it does support this use case. There's a playground at https://dprint.dev/playground/#code/Q/language/typescript. With these settings it breaks chained method calls if you include a new line at any point in the chain: {
"preferSingleLine": false,
"memberExpression.linePerExpression": true
} |
Another thought from my previous example is some degree of optimization heuristics. If I change the printWidth to 80, I get const FROM_NATIVES = Cases.when( // My Comment about this line
(x): x is boolean => typeof x === "boolean",
bool
)
.when((x): x is number => typeof x === "number", number)
.when((x): x is string => typeof x === "string", string); To me, if moving the chained member to the preceding line results in breaking the parameters over multiple lines -- resulting in more total lines of code -- than moving the chained member to the next line, then I prefer to move it to the next line. To do that, the formatter has to use some degree of backtracking. The formatter keeps the printWidth constraint fixed, and computes the LOC for the current member on the current line having to be broken over lines due to that constraint. Then it also considers the LOC for the current member starting on a new line. The option resulting in the least LOC wins. It may be sufficient to do that in a greedy way considering only the current chained member. You could constrain the backtrack depth to limit backtracking in nested member chains. From there, you get into longer backtracking breadth to determine whether the chain meets sufficient criteria to be split into one member per line. Such as, how many members were forced onto a new line by the greedy compact algorithm I just described, and then breaking the entire chain onto newlines if it exceeds the threshold. You can set that to 0 to always break, or to say -1 to disable and keep the greedy compact algorithm result. Lastly, for the edge case where you only want to break member chains onto newlines in certain cases, it would be nice to have a simple heuristic (assuming the trailing line comment bug is fixed) like if the chain is forced onto a newline by a trailing line comment immediately before the first member access, then the entire chain is broken into newlines. |
Just ran into this myself when trying to use trpc and zod together. All that fluency is now inconsistently indented. There appear to be some people who want to control chaining line breaks and others that don't. Using The fact of the matter is that not one size fits all as code formatting is now based on semantics. There are so many declarative APIs that beg for different indentation. |
@akutruff That has been the main question for... well, years now. |
Chaining in different lines makes it easier for debugging, why isn't this an option? |
Is this still not an option? |
Is there no way to add an option to ignore method chaining below the configured printWidth? |
|
If I only have 2 chained functions, but I will want to force a wrap on each chained function, I just add a "no-op" call to map:
|
It seems like Prettier stopped doing this (?). I just did a crazy Promise chain example and it did not put all the .then() on one line like I would have expected. |
@currentcreative That's probably because it didn't fit in one line. Try making a shorter one that would, better yet, do a different indentation on the fluent interface to see if prettier still knows better. |
4 years and still no config setting to simply ignore method chain linebreaks i love prettier but this makes me big sad would they even accept a PR for this? or should i not waste my time |
+1 on big sad |
They seem really committed to ignoring this problem. Luckily, alternatives are popping up and gaining mindshare. Imagine bleeding your userbase because you don't want to add a single option that is both sensible and popular. |
It’s because there is no good automatic rule here. It requires a setting and that breaks dogma. On Apr 10, 2024, at 9:15 PM, Avaranze ***@***.***> wrote:
They seem really committed to ignoring this problem. Luckily, alternatives are popping up and gaining mindshare.
Imagine bleeding your userbase because you don't want to add a single option that is both sensible and popular.
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>
|
End-of-line "//" implemented, albeit a different meaning: I could implement preservation of existing method chains, e.g.
Output:
Preserve and align the chained dot method calls on separate lines. Would that help? |
Implemented '--preserve-dot-chain' option for preserving method chain breaks:
https://www.npmjs.com/package/@yikes2000/prettier-plugin-merge-preserve |
@Yikes2000 : do you think the plugin architecture could support fixing this? #2724 |
@Yikes2000 I've yet to try this but hats off already for the "rolling up your sleeves" attitude towards solving this! |
That issue already has a fork solution: https://github.com/qpwo/prettier-no-jsx-parens Seems reliable. Did it not work? |
@Yikes2000 : it does work, but I'd love a plugin form so that Prettier can be kept up to date instead of having to merge changes downstream. |
@silviogutierrez : The methodology would likely work for #2724. Hurdles:
|
referencing: Improved method chain breaking heuristic
I understand the motivation behind the recent change, and fwiw, I agree with your solution in 2.0 with one exception. I don't think that code expressing chained methods across multiple lines should be automatically collapsed by
prettier
.Through each issue linked (#3197, #4765, #1565 and #4125) I didn't interpret that the community wanted chained methods collapsed; they simply wanted inline chained methods to remain inline.
In the highest rated comment from #4125, one of @pvdlg's suggestions is
If the user write them on multiple lines => keep on multiple line
. In this comment, I think @pvdlg nailed it, and that's exactly what I'm looking for too.Prettier 2.0.2
Playground link
# Options (if any): --print-width=120
Input:
Output:
Expected behavior:
For method chaining, manually inserted newlines should not be erased.
The text was updated successfully, but these errors were encountered: