Skip to content
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

[RFC] Introduce infiniteWidth document #680

Closed
wants to merge 1 commit into from

Conversation

vjeux
Copy link
Contributor

@vjeux vjeux commented Feb 13, 2017

For some reason I don't understand, breakParent doesn't have the intended effect. But having a document that always puts the remaining with to < 0 does.

I'm pretty sure that we should actually fix the issue with breakParent but wanted to show a solution that would work.

Fixes #625
Fixes #616

@jlongster
Copy link
Member

That's strange, I would like to look into this more because it feels like it's working around a bug. Does it involve conditionalGroup? Remember that breaks are not propagated across them, so you need to use willBreak and manually manage breaking.

For some reason I don't understand, breakParent doesn't have the intended effect. But having a document that always puts the remaining with to < 0 does.

I'm pretty sure that we should actually fix the issue with breakParent but wanted to show a solution that would work.

Fixes prettier#625
Fixes prettier#616
@vjeux
Copy link
Contributor Author

vjeux commented Feb 23, 2017

Just rebased it

@jlongster
Copy link
Member

Thanks. I've been very busy recently so sorry I haven't had a chance to look at this yet. Tomorrow is pretty busy as well but I'll try to find time for this. (Usually if I have time I skim through all the other issues/prs :p)

@jlongster
Copy link
Member

jlongster commented Feb 28, 2017

I took a look at this. I don't know the solution yet, but here's what I found:

  • The problem is that we forcibly break objects if they were expanded in the original source. That causes these kinds of problems, because if a source has a collapsed object and then we naturally expand it, running prettier on it again will see that it's expanded and forcibly expand it. In case Add testing #1, the code does not see any hard breaks, so any checks like willBreak will be false, but in case Nuclide run-through #2, it does see hard breaks, so willBreak and friends will be true. This changes how various code interacts and deals with breaks.

I still wish we didn't keep objects expanded. :)

  • Your PR happens to fix it, but not for the reason you think. This simple patch makes the problem go away:
diff --git a/src/printer.js b/src/printer.js
index 3a9e6e8..171670c 100644
--- a/src/printer.js
+++ b/src/printer.js
@@ -711,18 +711,17 @@ function genericPrintNoParens(path, options, print) {
             ifBreak(
               canHaveTrailingComma && shouldPrintComma(options) ? "," : ""
             ),
             indent(
               parentIsUnionTypeAnnotation ? 2 : 0,
               concat([options.bracketSpacing ? line : softline, rightBrace])
             ),
             path.call(print, "typeAnnotation")
-          ]),
-          { shouldBreak }
+          ])
         );
       }
 
     case "PropertyPattern":
       return concat([
         path.call(print, "key"),
         ": ",
         path.call(print, "pattern")

But that removes the feature of breaking an object if anything inside it is broken, so we obviously can't do that. Your code happens to remove this so it "works", but you don't need infiniteWidth at all.

This points to why this is happening: the shouldBreak is false in the first pass, and true in the second pass.

I don't know the right solution yet. I'll come back to this tomorrow!

@jlongster
Copy link
Member

Overtaken by #1032

@jlongster jlongster closed this Mar 17, 2017
@lock lock bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Jan 21, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jan 21, 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.

Braceless arrow function indentation is not stable Braceless if + return + object is not stable
2 participants