autofix for jsx-sort-props doesn't respect comments

[7rulnik]

Autofix for jsx-sort-props doesn't respect comments.

For example:

<foo
  b={1}
  // comment for a
  a={0}
/>

will be fixed like this:

<foo
  a={0}
  // comment for a
  b={1}
/>

But I think it should be like this:

<foo
  // comment for a
  a={0}
  b={1}
/>

[ljharb]

Comment attachment is a very hard problem; some people put comments above, and some below, and many don’t comment at all.

We may be able to handle something here, but we’d be constrained by how Babel implemented comment attachment.

[7rulnik]

Yeah, I agree.

About Babel AST:

<foo
  a={0}
  // comment for a
  b={1}
/>

For this example Babel will keep comment in ast for a prop as trailing comment and for b same comment as leading. I think we can configure it via options. Something like preserveComment:trailing|leading. But seems that it will be not so trivial.

But maybe we can implement option just for disabling autofix?

[ljharb]

It seems prudent to disable autofixes entirely for props that have a leading or a trailing comment, no option needed.

[7rulnik]

Maybe we should disable it entirely for component's that contains props with comments? Otherwise result of autofix will be confusing.

For example group of props before comment sorted in alphabetical order and after comment too. But it doesn't make sense, because it's still not valid.

<input
  a
  b
  c
  // fofof
  f
  a
  b
  c
/>

If so, I can submit PR

[ljharb]

In that example I’d expect everything to be sorted except the ones surrounding the comment, and the dev being forced to manually fix those.

[7rulnik]

So, something like this? Ok, I will take a look

<input
  a
  a
  b
  b
  c
  c
  // fofof
  f
/>

[Vages]

Have you had a look yet, @7rulnik ? No pressure; I'm just evaluating whether to have a go at this myself, and I'd like to hear if you're stuck at some bump which I'm also likely to hit.

[Vages]

And by the way, in your example: Has the c directly above the f prop been moved because it is connected to the comment, or has it just been moved because that's the correct place to put it when sorting alphabetically?

[ljharb]

@Vages if the original was:

<input
  m
  n // this is n
  o
  c // this is c
  // fofof
  f // this is f
  a
  b
  c
/>

it should autocorrect to:

<input
  a
  b
  c
  m
  n // this is n
  o
  c // this is c
  // fofof
  f // this is f
/>

In other words, any line touching a standalone comment on either side should be left alone / sorted last, as a single group.

[jsphstls]

I think there are two distinct issues here: Not moving the beside comment is a bug. Behavior for other comment locations is a feature.

[ljharb]

@jsphstls i agree that a same-line comment should be moved along with it, and should not interfere with sorting.

[ljharb]

For single-line props:

<input f e d /* comment */ c b a />

should autocorrect to:

<input a b d /* comment */ c e f />

or

<input a b e f d /* comment */ c />

as long as the d+comment+c group was kept atomic.

[ROSSROSALES]

@ljharb I am having trouble with my approach to the problem and wanted to clarify if it is appropriate. As per the examples you have given above, I believe grouping to the AST nodes and comments before sorting would work. I have been able to identify the attribute nodes and comments. However, I tried using AST explorer to find comments as nodes, but I was unable to.

Question: Am I heading in the right direction? If so, How should attributes and comments be identified and linked?

Any advice or guidance would be greatly appreciated!

[ljharb]

@ROSSROSALES I think the comments will be attached as a property on an adjacent node of another kind.