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] Always expand objects if contained in another object or array #475

Closed
wants to merge 1 commit into from

Conversation

vjeux
Copy link
Contributor

@vjeux vjeux commented Jan 26, 2017

The flow tests look really bad with it but the more real world tests feel much better. I think we'll have to run that on a real codebase and see how it feels.

Fixes #74

The flow tests look really bad with it but the more real world tests feel much better. I think we'll have to run that on a real codebase and see how it feels.

Fixes prettier#74
@jlongster
Copy link
Member

I wonder if we added one more condition to the heuristic: there are 2 or more properties in the object. I realized that we would break cases like { foo: { bar: { baz: 5 } } } which look nice collapsed, but once you get more than one it makes sense to expand them. What do you think?

@yamafaktory
Copy link
Contributor

@vjeux this doesn't have any effect with the Flow parser, e.g.:

const a = {b: 5,c: 4,d: {e,f: {}}};

outputs (Babylon):

const a = {
  b: 5,
  c: 4,
  d: {
    e,
    f: {}
  }
};

with Flow:

const a = { b: 5, c: 4, d: { e, f: {} } };

@vjeux
Copy link
Contributor Author

vjeux commented Jan 26, 2017

After thinking more about it, I think that the key property we want is consistency between all the objects of an object/array. So, if there's one object in an object that would be split, then we need to split all of them.

I don't think that the current IR supports this kind of behavior but I think that it would solve a lot of the problems with objects expansion.

@jlongster
Copy link
Member

Yeah, that's too complicated for the printer. That would require something like browser's "reflow" capability where it sees something doesn't fit and can arbitrarily reflow any part of the output. It's not only complicated but could worsen performance.

It's going to be easier to have a heuristic that we can apply statically at IR-generation time.

@vjeux
Copy link
Contributor Author

vjeux commented Jan 27, 2017

We need to think more about this problem. I don't think that this idea is going to work out well.

@vjeux vjeux closed this Jan 27, 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.

None yet

3 participants