Variable declarator formatting

[MichaReiser]

Rome has no special formatting for variable declarations but Prettier formats them the same way as assignment expressions.

Examples

A few examples from the latest Prettier comparison report

• https://gist.github.com/MichaReiser/77799fe4363bcc48838a159e581d8229#jsassignmentissue-10218js
• https://gist.github.com/MichaReiser/77799fe4363bcc48838a159e581d8229#jsassignmentissue-1966js
• https://gist.github.com/MichaReiser/77799fe4363bcc48838a159e581d8229#jsassignmentissue-4094js
• https://gist.github.com/MichaReiser/77799fe4363bcc48838a159e581d8229#jsassignmentissue-7091js
• https://gist.github.com/MichaReiser/77799fe4363bcc48838a159e581d8229#jsassignmentissue-8218js
• https://gist.github.com/MichaReiser/77799fe4363bcc48838a159e581d8229#jsassignmentlone-argjs
• https://gist.github.com/MichaReiser/77799fe4363bcc48838a159e581d8229#jsassignmentunaryjs

Expected

Rome's output to match Prettier's

[ematipico]

This would require some understand of Prettier's IR and source code, to understand how/when they decide to break variable declarations.

[denbezrukov]

Hi,
Could I also take this issue?🙏🏽
Because it's relative with object member property.

[MichaReiser]

Sure thing. I assigned it to you

[denbezrukov]

Need to check because Rome and Prettier have different IR for these cases:
const _id1 = data.createTestMessageWithAReallyLongName.someVeryLongProperty.thisIsAlsoALongProperty._id;
const { imStore, showChat, customerServiceAccount } = store[config.reduxStoreName];
const pendingIndicators = shield.alarmGeneratorConfiguration.getPendingVersionColumnValues;
const pendingIndicatorz = shield.alarmGeneratorConfiguration.getPendingVersionColumnValues();
const bifornCringerMoshedPerplexSawderGlyphsHa = someBigFunctionName("foo")( "bar", );

https://gist.github.com/MichaReiser/77799fe4363bcc48838a159e581d8229#jsassignmentunaryjs

• I guess that it's because of isPoorlyBreakableMemberOrCallChain https://github.com/prettier/prettier/blob/be9d63e543b6ac28aa6eee7ab8dc45849cd8f0b3/src/language-js/print/assignment.js#L215

[denbezrukov]

@ematipico @MichaReiser
Could you please help me figure out if Rome has an alternative for some Prettier functions or not? 🙏🏽🙏🏽🙏🏽
I'm looking into isPoorlyBreakableMemberOrCallChain.

• https://github.com/prettier/prettier/blob/be9d63e543b6ac28aa6eee7ab8dc45849cd8f0b3/src/language-js/print/assignment.js#L329

It has if (doc.label === "member-chain") conditional. It seems that prettier can mark elements. Does Rome has anything like this?

• https://github.com/prettier/prettier/blob/be9d63e543b6ac28aa6eee7ab8dc45849cd8f0b3/src/language-js/print/assignment.js#L346

Another conditional is if (isCallExpressionWithComplexTypeArguments(node, print)) . The function uses willBreak function. Does Rome has anything like this?

• https://github.com/prettier/prettier/blob/be9d63e543b6ac28aa6eee7ab8dc45849cd8f0b3/src/language-js/print/assignment.js#L432

• https://github.com/prettier/prettier/blob/be9d63e543b6ac28aa6eee7ab8dc45849cd8f0b3/src/language-js/print/assignment.js#L454

function willBreak(doc) {
  return findInDoc(doc, willBreakFn, false);
}

[MichaReiser]

It has if (doc.label === "member-chain") conditional. It seems that prettier can mark elements. Does Rome has anything like this?

No, Rome doesn't have a similar IR element yet. Adding it should be straightforward. You add a new variant to the FormatElement enum called Label that has a Box<[FormatElement]> content (similar to Group). The printer can ignore the labels, meaning it can just queue the elements from the content.

You then have to add a labelled function to rome_formatter::builders, take a look at how e.g. comment works.

Another conditional is if (isCallExpressionWithComplexTypeArguments(node, print)) . The function uses willBreak function. Does Rome has anything like this?

@ematipico is looking into this as well. There are two ways:

Ideal: We implement a will_break extension method for Buffer that returns a WillBreakBuffer. The WillBreakBuffer works similar to Inspect, it wrapps an inner Buffer but inspects every written element before forwarding it to the inner buffer.

fn write_element(&mut self, element: FormatElement) -> FormatResult<()> {
  self.will_break = self.will_break || element.will_break();
  self.inner.write_element(element);
}

The WillBreakBuffer must have a finish method:

fn finish(self) -> bool {
  self.will_break
}

Other alternatives are to use an ad-hoc inspector (results in a lot of repetition)

let mut will_break = false;
{
	let mut buffer = f.inspect(|element| {
		will_break = will_break || element.will_break();
	});

	write!(buffer, [element])?;
}

// use `will_break

Or write into a VecBuffer and retrieve the element (potentially slower because it may require coping many elements from the temporary buffer to the final buffer + requires an additional allocation)

let mut buffer = VecBuffer::new();
write!(buffer, [content])?;
let element = buffer.into_element();
let will_break = element.will_break();

f.write_element(element)?;

[denbezrukov]

Could I try to add Label variant? 🙏🏽🙏🏽

[denbezrukov]

@MichaReiser
Do you mind telling me how you see the use of this new FormatElement?🙏🏽
For instance, If we want to have a conditional which based on Label identifier, we need to get a FormatElement.
Do I understand correctly that the only way to do it is creating VecBuffer and call into_element?

let mut buffer = VecBuffer::new(f.state_mut());

write!(buffer, [self.some_element.format()])?;

let formatted_element = buffer.into_element();


if let FormatElement::Label(label) = formatted_element {
    if label.identifier() == "some-chain" {
       .. 
    }
} else {
    ..
}

[MichaReiser]

There are two ways, one is using VecBuffer the other is to use inspect but that only works if the some_element.format() only writes exactly one element:

let mut is_chain = false;
{
	  let mut buffer = f.inspect(|element| {
	    if let FormatElement::Label(label) = element {
	     is_chain = labelled.label() == "some-chain";
	    } else {
	     is_chain = false;
	  });
	  
	  write!(buffer, [self.some_element.format()])?;
}

This has the advantage that it avoids allocating a new vector and later copying all elements over into the final buffer.

Something I wanted to explore but didn't get along yet is to provide a way that memoized allows you to inspect the content. Ideally, something like

let inner = self.some_element.format().memoized();

if FormatElement::Label(label) = inner.content() {
  write!(f, [inner])?;
} else {
  // write it in another way
}

I think this would provide the best ergonomics and has the added benefit that it avoids copying the elements because it only needs to write a single element, the interned element.

Let me quickly try if I can write something along these lines

[ematipico]

This is almost completed. The only thing that remains is querying, using will_break some parts of the right hand side.

[denbezrukov]

This is almost completed. The also thing that remains is querying, using will_break some parts of the right hand side.

Yes, there is also one piece.

• https://github.com/prettier/prettier/blob/e5beb5aea451c5d826e4fe840777653dc173e6a3/src/language-js/print/assignment.js#L145

I've implemented it but it didn't change tests.🤷🏻‍♂️

[ematipico]

I've implemented it but it didn't change tests.🤷🏻‍♂️

It might be some weird edge case. Usually I do git blame on the lines that interests me, retrieve the commit and see if there were some tests pushed with that commit. But it's fine, no need to hit our head :)

[ematipico]

@MichaReiser what's your stance here? I believe this issue can be closed.

[MichaReiser]

I'm OK with closing this for now until we are able to reproduce an example where the labelled member expression is relevant (isPoorlyBreakableMember...)