Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat: add sortBy: "value" related-options #469

Merged
merged 10 commits into from
Feb 18, 2025

Conversation

hugop95
Copy link
Contributor

@hugop95 hugop95 commented Feb 13, 2025

Should resolve #68

Description

This PR adds 3 sort by value related-options to sort-object-types and sort-interfaces:

  • customGroups.elementValuePattern to match property patterns.
  • sortBy: 'name' | 'value' (by default name).
  • customGroups.sortBy: 'name' | 'value' to override the sortBy option in a given custom group.

Example

Sort properties by:

  • ObjectId.
  • Date.
  • Primitive types.
  • All the rest, sorted by name alphabetically.

Users have two ways to write this: use sortBy: 'value' by default, and for the last group, use sortBy: 'name', or the opposite.

Details
{
"perfectionist/sort-object-types": [
    "error",
    {
      "groups": [
        "objectIds",
        "dates",
        "primitives"
      ],
      "customGroups": [
        {
          "groupName": "objectIds",
          "elementValuePattern": "^ObjectId$",
        },
        {
          "groupName": "dates",
          "elementValuePattern": "^Date$",
        },
        {
          "groupName": "primitives",
          "elementValuePattern": "string|number|boolean",
          "sortBy": "value"
        }
      ]
    }
  ]
}	

Or

"perfectionist/sort-object-types": [
    "error",
    {
      "sortBy": "value",
      "groups": [
        "objectIds",
        "dates",
        "primitives",
        "allTheRestSortedNormally"
      ],
      "customGroups": [
        {
          "groupName": "objectIds",
          "elementValuePattern": "^ObjectId$"
        },
        {
          "groupName": "dates",
          "elementValuePattern": "^Date$"
        },
        {
          "groupName": "primitives",
          "elementValuePattern": "string|number|boolean"
        },
        {
          "groupName": "allTheRestSortedNormally",
          "sortBy": "name"
        }
      ]
    }
  ],

Result:

type Type = {
  _id: ObjectId;
  groupId: ObjectId;
  createdAt: Date;
  lastLoginAt: Date;
  b: boolean;
  c: number;
  a: string;
  // We are sorted alphabetically!
  anotherObject: AnotherObject;
  someValue: AnObject;
};

Important notes

❓Can users sort by value alphabetically, then by name alphabetically if the values are the same ?

Currently, no. This means that

{
  "perfectionist/sort-object-types": [
    "error",
    {
      "sortBy": "value",
    }
  ],
}

will tolerate both

type Types = {
  a: string
  b: string
}

and

type Types = {
  b: string
  a: string
}

We would need to allow users to write

"perfectionist/sort-object-types": [
    "error",
    {
      "sortBy": "value",
	  "fallbackSort": { "sortBy": "name"}
    }
  ],

This can come in a later PR.

❓How does sortBy: 'value' work with non-properties ?

It depends if it's the base sortBy option, or the customGroups one.

Base sortBy option

Non-properties nodes will not have a sort order enforced.

type Types = {
  z(): void
  a: string
}

and

type Types = {
  b: string
  z(): void
}

are both allowed.

customGroups.sortBy option

Grouping is still enabled, but within the group, the sort order is not enforced. This essentially means that we are enforcing the non-property member (such as functions) matching the group to be in that group, but the user can choose to move that member inside that group wherever they want.

Commit split

  • Commits 1-5 are refactors.
  • Commit 6+ are the actual implementation.

What is the purpose of this pull request?

  • New Feature

@hugop95 hugop95 force-pushed the feat/sort-objects/sort-by-value branch 2 times, most recently from ce67487 to 9f46202 Compare February 13, 2025 23:59
@hugop95 hugop95 marked this pull request as ready for review February 14, 2025 00:01
- `compare` natively handles `type: unsorted` now, so we don't need those custom conditions anymore.
- This is currently not handled in custom groups.
- To `sort-object-types` and `sort-interfaces`.
- To `sort-object-types` and `sort-interfaces`.
- To `sort-object-types` and `sort-interfaces`.
@hugop95 hugop95 force-pushed the feat/sort-objects/sort-by-value branch from 9f46202 to f20e334 Compare February 14, 2025 00:05
@azat-io
Copy link
Owner

azat-io commented Feb 18, 2025

LGTM.

In the beginning, I thought about using Typescript to solve this problem.

To have TypeScript ESLint detect the type of the value and sort them according to the type. For example, if it is a JSX props object. And the types are specified somewhere else.

interface Props {
  isOpen: boolean
  isCompact: boolean
  otherProp: string
  value: string
  onChange: () => {}
}

// Example:
// If booleans should be last in object

export let Component: FC<Props> = ({
  onChange,
  otherProp
  value,
  isCompact,
  isOpen,
}) => {}

P.S. At least I want to try to do it in another plugin:
azat-io/eslint-plugin-de-morgan#3

@hugop95
Copy link
Contributor Author

hugop95 commented Feb 18, 2025

@azat-io I think that we would need to dive deeper into how typescript-eslint infers/detects typings.

I haven't had the time to l look into that, so I don't know if it's going to end up being hard to implement or not. The benefits can be quite big for a lot of rules from perfectionist, and for your new de-morgan plugin as well. It's something I can start looking into as well once the whole sortBy: value milestone is done. 🙂

@azat-io
Copy link
Owner

azat-io commented Feb 18, 2025

The benefits can be quite big, but I'm afraid it could affect the performance.

@hugop95
Copy link
Contributor Author

hugop95 commented Feb 18, 2025

@azat-io

The benefits can be quite big, but I'm afraid it could affect the performance.

It's always something we can put behind an option, similarly to sort-imports and tsconfig parsing!

@azat-io
Copy link
Owner

azat-io commented Feb 18, 2025

Yes 👍

@azat-io azat-io merged commit 94caac8 into azat-io:main Feb 18, 2025
8 checks passed
@hugop95 hugop95 deleted the feat/sort-objects/sort-by-value branch February 18, 2025 17:42
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sort keys by type
2 participants