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

Feature: Support for case-insensitive elementNamePattern RegExp #462

Closed
2 tasks done
letelete opened this issue Feb 7, 2025 · 8 comments · Fixed by #464
Closed
2 tasks done

Feature: Support for case-insensitive elementNamePattern RegExp #462

letelete opened this issue Feb 7, 2025 · 8 comments · Fixed by #464
Labels
enhancement New feature or request

Comments

@letelete
Copy link

letelete commented Feb 7, 2025

What rule do you want to change?

sort-objects

Describe the problem

I'm trying to define a linear order for size definitions commonly used in my codebase.

I generate groups and customGroups dynamically using a flat ESLint config.

My custom groups work as intended for lowercase keys. However, to support all possible cases, I need to explicitly define all the variations (for example: ^(?:.*?[0-9]+xl)$ -> ^(?:.*?[0-9]+[xX][lL])$). As you can imagine, this becomes a bit of a hassle when working with multiple groups, and it also makes my patterns harder to understand.

It would be great to have a nice API for handling case-insensitive pattern matching using the elementNamePattern option. Any ideas? Perhaps it’s already supported, and I’m just missing something.

Code example

const sizesOrder = [
  { name: 'size-larger-than-xl', pattern: '^(?:.*?[0-9]+xl)$', order: 'desc' },
  { name: 'size-xl', pattern: '^(?:[a-z-_]*xl)$' },
  { name: 'size-lg', pattern: '^(?:.*lg)$' },
  { name: 'size-md', pattern: '^(?:.*md)$' },
  { name: 'size-base', pattern: '^(?:.*(?:base|regular))$' },
  { name: 'size-sm', pattern: '^(?:.*sm)$' },
  { name: 'size-xs', pattern: '^(?:[a-z-_]*xs)$' },
  { name: 'size-smaller-than-xs', pattern: '^(?:.*?[0-9]+xs)$' },
];
const perfectionistConfig = {
  sizes: {
    groups: sizesOrder.map(({ name }) => name),
    customGroups: sizesOrder.map(
      ({ name: groupName, pattern: elementNamePattern, order = 'asc' }) => ({
        type: 'natural',
        groupName,
        selector: 'property',
        elementNamePattern,
        order,
      })
    ),
  },
};```

Then, I apply it to desired rule:

``` js
[
  'perfectionist/sort-objects': [
    'error',
    {
      type: 'natural',
      partitionByComment: true,
      partitionByNewLine: true,
      groups: [
        <other groups>
        ...perfectionistConfig.sizes.groups,
        <other groups>
      ],
      customGroups: [
        <other custom groups>
        ...perfectionistConfig.sizes.customGroups,
        <other custom groups>
      ]
    }
  ]
]

Additional comments

Proposal for the new API

  1. Allow elementNamePattern to be a RegExp object.
    Allowing users to define elementNamePattern as a RegExp object would enable case-insensitive matching out of the box:

    elementNamePattern: /^(?:.*?[0-9]+xl)$/i,
  2. Add an ignoreCase option to elementNamePattern.
    Another solution would be to allow users to specify an ignoreCase option that applies to elementNamePattern, similar to the existing API for the sort-objects rule.

This is slightly related to issue #419.

Let me know what you think!

Validations

  • Read the docs.
  • Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
@letelete letelete added the enhancement New feature or request label Feb 7, 2025
@letelete
Copy link
Author

letelete commented Feb 7, 2025

This is how I support case-insensitive pattern matching now:

const sizesOrder = [
  {
    name: 'size-larger-than-xl',
    pattern: '^(?:.*?[0-9]+[xX][lL])$',
    order: 'desc',
  },
  { name: 'size-xl', pattern: '^(?:[a-zA-Z-_]*[xX][lL])$' },
  { name: 'size-lg', pattern: '^(?:.*[lL][gG])$' },
  { name: 'size-md', pattern: '^(?:.*[mM][dD])$' },
  {
    name: 'size-base',
    pattern: '^(?:.*(?:[bB][aA][sS][eE]|[rR][eE][gG][uU][lL][aA][rR]))$',
  },
  { name: 'size-sm', pattern: '^(?:.*[sS][mM])$' },
  { name: 'size-xs', pattern: '^(?:[a-zA-Z-_]*[xX][sS])$' },
  { name: 'size-smaller-than-xs', pattern: '^(?:.*?[0-9]+[xX][sS])$' },
];

@hugop95
Copy link
Contributor

hugop95 commented Feb 7, 2025

Hi @letelete

I had this issue in mind when implementing the RegExp matcher, but thought that there wasn't a need yet to support more advanced cases. 😅

Allow elementNamePattern to be a RegExp object.

ESLint configurations need to be serializable (in theory at least), which is why we handle RegExp strings without their options.

Allowing non-serializable options (even if they would work in practice today) is risky as this can potentially break at any moment depending on what the ESLint Team decides on doing.

Add an ignoreCase option to elementNamePattern

In my opinion, I think it's better to go toward a more generic approach: see how users can pass RegExp options (i, u, whatever other options is supported), which is similar to your first proposal in the end.

@hugop95
Copy link
Contributor

hugop95 commented Feb 7, 2025

I think that the following API can be suitable (for any other option that currently supports RegExp strings as well):
elementNamePattern: string | { regex: string; regexOptions: 'iu' }

@letelete
Copy link
Author

letelete commented Feb 7, 2025

Hi @hugop95,
thanks for such an immediate response!

Considering the need to support serialization, regexOptions make sense. I had something like flags in mind (because regexOptions suggests more preferences to be set). So: elementNamePattern: string | { regex: string; regexOptions: { flags: 'iu' } }, but yeah, it's a minor detail.

@letelete
Copy link
Author

letelete commented Feb 7, 2025

So there's currently no other way to handle my problem other than what I've defined here, right?

@hugop95
Copy link
Contributor

hugop95 commented Feb 7, 2025

@letelete

So there's currently no other way to handle my problem other than what I've defined #462 (comment), right?

At this very moment, no: lowercase and uppercase characters need to appear in the expression, but the feature shouldn't take long to be implemented.

@letelete
Copy link
Author

letelete commented Feb 7, 2025

Awesome! I'm looking forward to seeing it live :)

@azat-io
Copy link
Owner

azat-io commented Feb 13, 2025

Added in 00a8080.
Released in v4.9.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants