eslint-config-xo-typescript icon indicating copy to clipboard operation
eslint-config-xo-typescript copied to clipboard

Variable Formatting: global constants

Open EdJoPaTo opened this issue 4 years ago • 12 comments

Related to the rule @typescript-eslint/naming-convention.

I regularly use global constants with UPPER_CASE style for constants that wont change in runtime. Not sure how to exactly specify that in the rules but I first wanted to suggest something like that here before checking on how that could be done.

Something like these:

const DIRECTORY = './events'
const DEFAULT_OPTIONS: Options = {
  …
}

If thats something that is not liked here: How would you propose to write constants like this instead?

EdJoPaTo avatar Oct 04 '21 09:10 EdJoPaTo

It's intentional: https://github.com/eslint/eslint/issues/10605

sindresorhus avatar Oct 04 '21 10:10 sindresorhus

@fregante What are your thoughts about this?

sindresorhus avatar Oct 04 '21 10:10 sindresorhus

Perhaps they should be allowed only at the top level. They're pretty common, even though I don't particularly see the need for that.

fregante avatar Oct 04 '21 12:10 fregante

I'm not sure if they are useful in libraries as they objective is to be configurable. But I have a bunch of them in projects that running somewhere doing something.

As a "need" I searched for some examples from my code (rg "^(?:export )?const [A-Z][A-Z_]+" --glob "*.ts"):

  • SOMETHING_REGEX to be used in new RegExp()
  • DEFAULT_… used as default values somewhere
  • EXAMPLE_… used by multiple tests
  • BASE_… used in objects where some are always the same ({...BASE_BLA, …})
  • numeric constants to simplify calculations (const SECONDS_PER_DAY = 60 * 60 * 24) or being used as sleep / interval arguments later on
  • folder or files where stuff is written or read
  • emojis (and other non ascii stuff) which are simpler to use by writing ERROR_EMOJI rather than the exact emoji / unicode thingy.

in general: stuff that wont change at runtime / compile time constants

I can use camelcase here but I am kinda used to seeing constants directly. Also they are conventions on other c derived languages (example). I am relatively sure I am not the only xo user using them. Also my other currently used language (Rust) produces a warning in that case: non-upper-case-globals but in contrast to TypeScript it knows when something is really const or not.

EdJoPaTo avatar Oct 04 '21 15:10 EdJoPaTo

Yeah, I guess upper-case is a good way to make it clear that the value is completely static. PR welcome.

sindresorhus avatar Oct 05 '21 08:10 sindresorhus

Perhaps they should be allowed only at the top level. They're pretty common, even though I don't particularly see the need for that.

Ideally, yes, but there's no way to enforce that with this rule.

sindresorhus avatar Oct 05 '21 08:10 sindresorhus

There is. The first part of the rule description even states that:

such as by enforcing all private properties begin with an _, and all global-level constants are written in UPPER_CASE.

Selectors → modifiers → global

EdJoPaTo avatar Oct 05 '21 14:10 EdJoPaTo

Oh great. I missed that.

sindresorhus avatar Oct 05 '21 15:10 sindresorhus

I tested around with the following but it didnt seem to work… Not sure why…

{
	selector: 'variable',
	format: [
		'strictCamelCase',
		'UPPER_CASE'
	],
	modifiers: [
		'global',
		'const'
	]
},

EdJoPaTo avatar Oct 05 '21 16:10 EdJoPaTo

It's because of how the rule prioritizes the selectors. If you have two selectors for variable, one with the filter option and one without, a variable name will always be checked against the one with the filter first.

So because of that, the very first selector in the XO rule ends up overriding any other variable selectors added later (unless they also have filters). Because it has a filter, a variable name will always be checked against it first, and the name will always match because the only criteria is that it doesn't have a hyphen or space (which variables can't have anyway).

There are a few ways to get around this, but the simplest way is just to remove the filter entirely. Properties with hyphens or spaces will still be exempt from camelCase because of the later requiresQuotes selector; I'm not sure if the filter was intended to do anything else?

Once that's removed, the above code for global constants will mostly work as intended. The only issue is that it does conflict with another selector for booleans. Since that one specifies types, it has a higher priority than selectors that only specify modifiers, so any booleans will get matched there first, global constants or not.

(I don't think that boolean rule actually triggers at all right now, because of the same issue with the filter. I didn't get any errors from it until I removed the filter, and then I suddenly got a whole bunch.)

So if you want global constant booleans to have the required prefixes and potentially be UPPER_CASE, you need to add a special case for that, something like this:

{
  selector: 'variable',
  types: ['boolean'],
  modifiers: ['const', 'global'],
  format: ['UPPER_CASE'],
  prefix: ['IS_', 'HAS_', 'CAN_', 'SHOULD_', 'WILL_', 'DID_'],
  filter: '[A-Z]{2,}'
},

Then adding everything together should get the right output.

kestrelnova avatar Dec 17 '21 01:12 kestrelnova

Any updates on this?

maxpain avatar Jan 13 '22 20:01 maxpain