Skip to content

Enhancement: member-ordering default configuration #5452

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.

Sign up for GitHub

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

Jump to bottom
Closed
4 tasks done
nbouvrette opened this issue Aug 9, 2022 · 6 comments · Fixed by #8248
Closed
4 tasks done

Enhancement: member-ordering default configuration #5452

nbouvrette opened this issue Aug 9, 2022 · 6 comments · Fixed by #8248
Labels
accepting prs Go ahead, send a pull request that resolves this issue enhancement: plugin rule option New rule option for an existing eslint-plugin rule good first issue Good for newcomers package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@nbouvrette
Copy link

Before You File a Proposal Please Confirm You Have Done The Following...

My proposal is suitable for this project

  • I believe my proposal would be useful to the broader TypeScript community (meaning it is not a niche proposal).

Link to the rule's documentation

https://typescript-eslint.io/rules/member-ordering

Description

Based on my experimentations and also on the documentation, to apply the "default configuration" you have to copy/paste this list: https://typescript-eslint.io/rules/member-ordering/#default-configuration which contains 88 lines.

Adding this in the ESLint config will make it less maintainable and having an option to omit this list and use the default would be preferable.

One idea would be to add a new option called memberTypesPreset where you could pick default - or maybe even its default value would be default and you don't have to specify it.

Or another idea would be that if memberTypes: [] (empty array) or, if memberTypes is simply omitted, the default configuration is applied.

Fail

---
# eslintrc.yaml
root: true # Disables inheritance from parent config.
overrides:
  # TypeScript configurations
  - files:
      - '*.ts'
      - '*.mts'
      - '*.cts'
      - '*.tsx'
    parser: '@typescript-eslint/parser'
    parserOptions:
      project:
        - 'tsconfig.json'
    extends:
      - plugin:@typescript-eslint/recommended
      - plugin:@typescript-eslint/recommended-requiring-type-checking
    rules:
      # Checks members (classes, interfaces, types) and applies consistent ordering.
      # @see https://typescript-eslint.io/rules/member-ordering/
      '@typescript-eslint/member-ordering':
        - error
        - default:
            memberTypesPreset: default
            order: alphabetically-case-insensitive

Pass

---
# eslintrc.yaml
root: true # Disables inheritance from parent config.
overrides:
  # TypeScript configurations
  - files:
      - '*.ts'
      - '*.mts'
      - '*.cts'
      - '*.tsx'
    parser: '@typescript-eslint/parser'
    parserOptions:
      project:
        - 'tsconfig.json'
    extends:
      - plugin:@typescript-eslint/recommended
      - plugin:@typescript-eslint/recommended-requiring-type-checking
    rules:
      # Checks members (classes, interfaces, types) and applies consistent ordering.
      # @see https://typescript-eslint.io/rules/member-ordering/
      '@typescript-eslint/member-ordering':
        - error
        - default:
            memberTypes:
              - signature
              - public-static-field
              - protected-static-field
              - private-static-field
              - public-decorated-field
              - protected-decorated-field
              - private-decorated-field
              - public-instance-field
              - protected-instance-field
              - private-instance-field
              - public-abstract-field
              - protected-abstract-field
              - private-abstract-field
              - public-field
              - protected-field
              - private-field
              - static-field
              - instance-field
              - abstract-field
              - decorated-field
              - field
              - static-initialization
              - public-constructor
              - protected-constructor
              - private-constructor
              - constructor
              - public-static-get
              - protected-static-get
              - private-static-get
              - public-decorated-get
              - protected-decorated-get
              - private-decorated-get
              - public-instance-get
              - protected-instance-get
              - private-instance-get
              - public-abstract-get
              - protected-abstract-get
              - private-abstract-get
              - public-get
              - protected-get
              - private-get
              - static-get
              - instance-get
              - abstract-get
              - decorated-get
              - get
              - public-static-set
              - protected-static-set
              - private-static-set
              - public-decorated-set
              - protected-decorated-set
              - private-decorated-set
              - public-instance-set
              - protected-instance-set
              - private-instance-set
              - public-abstract-set
              - protected-abstract-set
              - private-abstract-set
              - public-set
              - protected-set
              - private-set
              - static-set
              - instance-set
              - abstract-set
              - decorated-set
              - set
              - public-static-method
              - protected-static-method
              - private-static-method
              - public-decorated-method
              - protected-decorated-method
              - private-decorated-method
              - public-instance-method
              - protected-instance-method
              - private-instance-method
              - public-abstract-method
              - protected-abstract-method
              - private-abstract-method
              - public-method
              - protected-method
              - private-method
              - static-method
              - instance-method
              - abstract-method
              - decorated-method
              - method
            order: alphabetically-case-insensitive

Additional Info

No response
@nbouvrette nbouvrette added enhancement: plugin rule option New rule option for an existing eslint-plugin rule package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for team members to take a look labels Aug 9, 2022
@bradzacher
Copy link
Member

Based on my experimentations and also on the documentation, to apply the "default configuration" you have to copy/paste this list

this is not the case at all.

The default config us automatically applied if you provide no default key in your config. This means you can keep the default whilst also applying overrides for the non-default cases.

What is the goal you're trying to achieve?

@bradzacher bradzacher added awaiting response Issues waiting for a reply from the OP or another party and removed triage Waiting for team members to take a look labels Aug 9, 2022
@nbouvrette
Copy link
Author

I'm trying to apply order: alphabetically-case-insensitive on the default configuration.

The only way I was able to achieve this was by using the example in the Pass comment.

@nbouvrette
Copy link
Author

FYI I updated the playground and what I can tell:

  • If you omit memberTypes, it is applying the order on never which is not the default configuration
  • If you leave the default configuration empty then it's not applying the order on anything

Unless I missed something the only way to apply default configuration ordering is to copy/paste it in the configuration, which is also what the documentation is mentionning:

You can copy and paste the default order from Default Configuration.

@bradzacher
Copy link
Member

Yes you have describe the config working as intended.

The default is still setup using the array style config which forces case sensitive alphabetical sorting.
(the config listed in the docs is the exact config that's used)

Which means if you specify an object for default, you override the entire default config (as an array and an object cannot be merged together).

I.e. If you want to change the default ordering then you have to specify an object config which wipes away the the default ordering.

We could switch it to an object with explicit alphabetical ordering which would enable you to change the ordering and it shouldn't be a breaking change as it is equivalent.

@bradzacher bradzacher added accepting prs Go ahead, send a pull request that resolves this issue and removed awaiting response Issues waiting for a reply from the OP or another party labels Aug 10, 2022
@nbouvrette
Copy link
Author

From the documentation

By default, the members are not sorted. If you want to sort them alphabetically, you have to provide a custom configuration.

So basically as soon as you want to use sorting, there is no way right now to re-use the default configuration (without copy/pasting it) which in my opinion should be there by default because it's quite good.

I might consider doing a PR on member-ordering but I am not sure what you are proposing exactly.

Do you mean changing this line

oneOf: [arrayConfig(memberTypes), neverConfig],

To this:

oneOf: [arrayConfig(memberTypes), defaultConfig, neverConfig],

@bradzacher
Copy link
Member

Do you mean changing this line

No I mean changing the default config from

{
  default: [ ... ]
}

to

{
  default: {
    memberTypes: [ ... ],
    order: 'alphabetically' 
  }
}

that way you could specify your config like this:

{
  rules: {
    '@typescript-eslint/member-ordering': [
      'error',
      {
        default: {
          order: 'alphabetically-case-insensitive',
        },
      }
    ],
  },
}

and our tooling would merge it in - retaining the defaults whilst changing the ordering config

@bradzacher bradzacher added the good first issue Good for newcomers label Aug 13, 2022
sviat9440 added a commit to MillerSvt/typescript-eslint that referenced this issue Oct 23, 2022
@sviat9440
fix(eslint-plugin): member-ordering default configuration (typescript…
9b77a70 
…-eslint#5452)
@sviat9440 sviat9440 mentioned this issue Oct 23, 2022
Closed
3 tasks
parloti added a commit to parloti/typescript-eslint that referenced this issue Jan 14, 2024
@parloti
fix(eslint-plugin): member-ordering default configuration (typescript…
ea6c303 
…-eslint#5452)
@parloti parloti mentioned this issue Jan 14, 2024
Merged
3 tasks
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
accepting prs Go ahead, send a pull request that resolves this issue enhancement: plugin rule option New rule option for an existing eslint-plugin rule good first issue Good for newcomers package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
Milestone
No milestone
2 participants
@nbouvrette @bradzacher