Skip to content

[naming-convention] enumMembers should be default PascalCase #7879

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

Open
2 tasks done
Mister-Hope opened this issue Nov 4, 2023 · 6 comments · May be fixed by #11127
Open
2 tasks done

[naming-convention] enumMembers should be default PascalCase #7879

Mister-Hope opened this issue Nov 4, 2023 · 6 comments · May be fixed by #11127
Labels
accepting prs Go ahead, send a pull request that resolves this issue package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin preset config change Proposal for an addition, removal, or general change to a preset config

Comments

@Mister-Hope
Copy link
Contributor

Mister-Hope commented Nov 4, 2023

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

Description

Code like this fails with the default option:

const enum SearchItemType {
  Page = 0,
  ID = 1,
}

const enum SearchIndexType {
  Title = 1,
  Heading = 2,
  Text = 3,
  Image = 4,
  Card = 5,
  Doc = 6,
}

However, I think using PascalCase for enum member should be the default option, as this is the default syntax in TypeScript official docs, blogs and examples.

E.g.:


Other things to report and could be a feature request

The problem is affecting me because we are not providing a keyword to enable the default naming convention and "make some tweaks over it". I.E.: In this case, I can not simply add the green line below, instead I have to redeclare the whole "option". That is not convinient for me accross projects.

Impacted Configurations

// the default config is similar to ESLint's camelcase rule but more strict
const defaultOptions: Options = [
  {
    selector: 'default',
    format: ['camelCase'],
    leadingUnderscore: 'allow',
    trailingUnderscore: 'allow',
  },

  {
    selector: 'import',
    format: ['camelCase', 'PascalCase'],
  },

  {
    selector: 'variable',
    format: ['camelCase', 'UPPER_CASE'],
    leadingUnderscore: 'allow',
    trailingUnderscore: 'allow',
  },

  {
    selector: 'typeLike',
    format: ['PascalCase'],
  },

+  {
+    selector: 'enumMember',
+    format: ['PascalCase'],
+  },
];

Additional Info

No response

@Mister-Hope Mister-Hope added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin preset config change Proposal for an addition, removal, or general change to a preset config triage Waiting for team members to take a look labels Nov 4, 2023
@JoshuaKGoldberg JoshuaKGoldberg added accepting prs Go ahead, send a pull request that resolves this issue and removed triage Waiting for team members to take a look labels Nov 4, 2023
@JoshuaKGoldberg
Copy link
Member

Some codebases use camelCase for enums, some PascalCase. I suspect we should allow both for the default convention.

@bradzacher
Copy link
Member

I'm not sure about this, tbh.
This has been the default since the rule was released (so it's been this way for a few years now) and this is the first time we've had this asked.

Either that means one of:

  • not many people use the rule
  • not many people use the rule with the defaults
  • people use the rule with defaults and are happy with camelCase for enum names

Could do a sourcegraph query of public githubs to roughly determine which it is. If it's the 3rd then making this change would be bad as it would allow inconsistency in codebases - IMO you want one or the other in a codebase - not both.

@JoshuaKGoldberg JoshuaKGoldberg added evaluating community engagement we're looking for community engagement on this issue to show that this problem is widely important and removed accepting prs Go ahead, send a pull request that resolves this issue labels Nov 4, 2023
@Mister-Hope
Copy link
Contributor Author

Mister-Hope commented Nov 5, 2023

I agree with @JoshuaKGoldberg that camelCase should be added for backward compatibility reasons.

And for the casing, I searched npm homepage Popular libraries and fronend packages sorted by popularity

I omit any package which does not have built-in typescript (even it as @types/xxx package), also a lot of package does not use enum such as React.

  1. Vue: using PascalCase and small ammount of UPPERCASE (shims them to single char in prod) https://github.com/search?q=repo%3Avuejs%2Fcore%20enum&type=code
  2. axios: using PascalCase https://github.com/axios/axios/blob/f7adacdbaa569281253c8cfc623ad3f4dc909c60/index.d.cts#L138
  3. Preact: camelCase https://github.com/preactjs/preact/blob/9a9967d4edab22581e4f9245741e62452d4b43d2/src/internal.d.ts#L4
  4. Vite: both camelCase and PascalCase https://github.com/search?q=repo%3Avitejs%2Fvite%20enum%20language%3ATypeScript%20&type=code
  5. Webpack: camelCase https://github.com/search?q=repo%3Awebpack%2Fwebpack%20enum%20language%3ATypeScript%20&type=code
  6. Nest.js: UPPERCASE https://github.com/search?q=repo%3Anestjs%2Fnest%20enum%20language%3ATypeScript%20&type=code
  7. Grape.js: UPPERCASE camelCase and PascalCase https://github.com/search?q=repo%3AGrapesJS%2Fgrapesjs%20enum%20language%3ATypeScript%20&type=code
  8. Cheerio: PascalCase https://github.com/search?q=repo%3Acheeriojs%2Fcheerio%20enum%20language%3ATypeScript%20&type=code
  9. Babel: UPPERCASE camelCase and PascalCase https://github.com/search?q=repo%3Ababel%2Fbabel+enum+language%3ATypeScript+&type=code
  10. TypeScript: UPPERCASE camelCase and PascalCase, but >80% PascalCase https://github.com/search?q=repo%3Amicrosoft%2FTypeScript+enum+language%3ATypeScript+&type=code
  11. Angular: PascalCase mostly, only a small ammount of camelCase https://github.com/search?q=repo%3Aangular%2Fangular%20enum%20language%3ATypeScript%20&type=code
  12. VSCode: PascalCase mostly https://github.com/search?q=repo%3Amicrosoft%2Fvscode%20enum%20language%3ATypeScript%20&type=code

For the above repo, only Webpack and Preact is using camelCase enum. Others are all using PascalCase mostly.

A lot of repo includes camelCase (or lowercase) to represent the same keyword strings, e.g.:

const enum Log {
  debug = 'debug',
  info = 'info',
  // ...
}

And some libraries like Vue use UPPERCASE for internal usage of enum to slim package size:

// this is internal and only used to slim pro size
export const enum LifecycleHooks {
  BEFORE_CREATE = 'bc',
  CREATED = 'c',
  BEFORE_MOUNT = 'bm',
  MOUNTED = 'm',
  BEFORE_UPDATE = 'bu',
  UPDATED = 'u',
  BEFORE_UNMOUNT = 'bum',
  UNMOUNTED = 'um',
  DEACTIVATED = 'da',
  ACTIVATED = 'a',
  RENDER_TRIGGERED = 'rtg',
  RENDER_TRACKED = 'rtc',
  ERROR_CAPTURED = 'ec',
  SERVER_PREFETCH = 'sp'
}

enum for other usage are mostly PascalCase, and follows TypeScript docs/blogs syntax.

So I still insist my point @bradzacher, at least PascalCase should be added to defaults. Nobody would expect that codes having the same syntax as TypeScript docs example are "incorrect" with this linter.

@Mister-Hope
Copy link
Contributor Author

Another reason according to my usage, using PascalCase for enum makes it easier to recognize them through codes:

// Probably a class property
Abc.def
// TypeScript enum / const enum
Abc.Def
// object property
abc.def

@Josh-Cena
Copy link
Member

Josh-Cena commented Nov 5, 2023

I also agree to add PascalCase by default. The reason we don't get these requests often is because (a) people don't bother reading the default options and figuring out if they will work well; they just hand-configure them anyway (b) they don't know about conventions and/or they don't read the TS docs, so they trust the default options and they just use camelCase all along since they created the project (c) they don't use enums at all (d) they don't find it meaningful to argue about configurable default options. But I think it's a nice improvement both for developer education and alignment with conventions.

@JoshuaKGoldberg JoshuaKGoldberg added accepting prs Go ahead, send a pull request that resolves this issue and removed evaluating community engagement we're looking for community engagement on this issue to show that this problem is widely important labels Jan 31, 2024
@JoshuaKGoldberg
Copy link
Member

In theory, per #8792, we're feature freezing this rule. It's reached the point of complexity where we're having a hard time justifying working on it.

In practice, I think this is a nice and straightforward enough improvement for users that we can allow it regardless. cc @Josh-Cena

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepting prs Go ahead, send a pull request that resolves this issue package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin preset config change Proposal for an addition, removal, or general change to a preset config
Projects
None yet
4 participants