sort-prop-types fix to account for comments

[alexzherdev]

I know comments on prop types are not in favour here, but for those who do have them, would it be acceptable to support sorting comments together with their prop types? i.e. currently the rule would fix

{
  /** z */
  z: PropTypes.string,
  /** a */
  a: PropTypes.any
}

to

propTypes: {
  /** z */
  a: PropTypes.any,
  /** a */
  z: PropTypes.string
}

[ljharb]

How would you determine whether comments attach to the thing before, or the thing after?

[alexzherdev]

Hmm... can you give an example where the comment goes after the thing you're commenting on? I'm having a difficulty coming up with a real use case.
AST-wise, there is a leadingComments property on the node that contains the comment before the node.

[ljharb]

propTypes: {
  baz: PropTypes.any,
  foo: PropTypes.any,
  // ^ this is a foo, but the others don't need explanation
  bar: PropTypes.any,
}

[alexzherdev]

Ok. Haven't really seen code like that. But if it's something people would do in your experience (as opposed to putting this is a foo above the foo), then yeah.

[ljharb]

However, I’d say the current behavior is a bug, so your plan is probably a strict improvement.

Perhaps a plugin setting, that defaults to “above”, that determines comment attachment?

[alexzherdev]

comments: “above” | “below”

[ljharb]

(At plugin level tho, not rule)

[alexzherdev]

Why? Are there other rules that would use it?

[ljharb]

Any rule that moves code around :-) I’ve found over time that many rule configs belong instead as settings, and it’s easier to plan ahead.

[alexzherdev]

I'm able to use sourceCode.getCommentsBefore(node) to process comments when attachment is "above". But finding this tricky to implement for the "below" attachment.

propTypes: {
  foo: PropTypes.any,
  // ^ this is a foo, but the others don't need explanation
  bar: PropTypes.any
}

If node is the entire foo property, then the comment will not be available in sourceCode.getCommentsAfter(node) because it's after the comma. So I need to either get down to the token level functions, or try using getCommentsBefore on the property that goes next (in this case, bar).

Also, here

propTypes: {
  foo: PropTypes.any, // <- this is a foo, but the others don't need explanation
  bar: PropTypes.any
}

the comment needs to be attached to foo as well (whether "above" or "below"), but it only differs from the previous example in a new line I think.
I'll continue working on this and see if I can make it work. We could use this rule internally.

[jmtoung]

Has there been any progress on this? FWIW the sort-keys-fix eslint rule doesn't properly deal with comments after the node either

[ljharb]

jsx-sort-props got a fix for this in #3358; perhaps someone could handle it the same way in sort-prop-types? (cc @ROSSROSALES)

[ROSSROSALES]

@ljharb Thanks for pinging me! I can try and take a look at it. I also noticed in the lib/rules/sort-prop-types.js file, all the invalid test cases are commented out

eslint-plugin-react/tests/lib/rules/sort-prop-types.js

Line 489 in 210865e

// output: [

Currently when running test cases, it passes. However, once I comment them back in, test cases don't pass.
I am going to try and read into the history of the code, but if you had any insight please let me know! 👍

[ljharb]

nah that sounds like it’s probably an oversight. It’d be great to get a PR that uncomments everything passing; and then a second PR that uncomments the rest and fixes the bugs :-)

[ROSSROSALES]

@ljharb
I think we can close this issue now since #3471 fixes this.