Kibana: Style guide section for constants

[lukasolson]

We currently have no documentation in our style guide for how to handle constants.

We forked our JS style guide from node, which has a section about constants, which suggests using all uppercase (with underscores for spaces).

Please add your opinion below on whether we should have a section in our style guide for constants, and what it should be. We can then take a vote and decide if/what we want to add.

[kimjoar]

Can we define "constants"? Is it all JavaScript primitives? Only when you export them or also within files?

Well, either way I'm mostly :-1: to it. I think it's noisy and don't give much value. Also more difficult to read.

[stacey-gammon]

I'm for, but I think it should be restricted to primitives.

[lukasolson]

I think Wikipedia has a decent definition of "constant": https://en.wikipedia.org/wiki/Constant_(computer_programming)

[lukasolson]

I guess that definition isn't really helpful though, as it applies to computer programming in general and not specifically to JavaScript. So I'm with @kjbekkelund, can we provide our definition of a "constant" in the context of how we write JavaScript, and how it relates to other immutable objects?

[lukasolson]

To me, it has to do with knowing what the value of something is, regardless of the scope of program execution. If something will only ever have one value, and it isn't derived from something else, it is a constant.

Now that I mention this, I'm actually :-1: to having special handling for constants, because there are so many things that could potentially fit this definition, and it adds extra mental overhead to think about whether or not something should be treated as a constant. In addition, I don't particularly see the value in knowing whether or not something fits this definition.

However, I'd be :+1: to adding a section that explains that we treat the naming of constants the same way we treat other variables.

[kimjoar]

We use the airbnb styleguide a whole lot when discussing JS styleguides, but they don't mention "constants", and also all their consts with primitives are normal camelCased names.

(This is not to say I always agree with them on other things, I was just wondering what their discussion of it was)

[cjcenizal]

I think there are two concepts we need to disambiguate: const and "constants".

A const in JavaScript is a variable which has a one-time assignment of a value. You can't reassign a value to it. From a technical perspective, this is similar to the comp sci definition of a constant variable, per @lukasolson's link above.

AirBnB's prefer-const rule enforces the use of const to communicate to the reader that the value is never reassigned. This improves code readability because it encourages functional manipulation of data over mutation. The result is we use const almost everywhere.

There's also the concept of "constant" as a convention, or a classification. All constants will be const but not all const will be constants. @Bargs mentioned a possible definition as:

"this is a singleton across the entire application", like an enum value would be in Java
so you know you can === on those suckers

Another reason to create a convention for constants is to identify magic numbers. For example, if you defined a variable in your code to represent how many seconds there are in a year, you might create:

export const SECONDS_PER_YEAR = 31557600;

The way the value is written tells you that this is a globally-defined value, and not dependent on any context. For example, secondsPerYear could be a useful variable name inside of a function:

function calculateSecondsAsleepPerYear(minutesPerDay) {
  const minutesPerYear = minutesPerDay * 365;
  const secondsPerYear = minutesPerYear * 60;
  return secondsPerYear;
}

Treating one differently than the other helps people tell them apart.

[jimgoodwin]

I'm pretty sure that depending on the type of the const it's value can change since I guess this is just a constant binding, not in any way a guarantee of immutability. So naming them specially would highlight cases where someone is erroneously changing for example the contents of an const object.

[cjcenizal]

I like @jimgoodwin's point, though as a counterpoint I suppose we could also use Object.freeze to gain true immutability.

[Bargs]

I think most people's conception of a "constant" in this context comes from class based languages like Java, where a constant is generally a static, immutable field accessible on a class, not on class instances. The CAPS_AND_UNDERSCORES naming convention conveys 1. the variable's scope and immutability and 2. the value's importance as an integral part of the abstraction provided by the class.

I can get behind this naming convention if we use it in the same way (replace the word class with module above). So the rules for all caps and underscores variables would be as follows:

• They're always const and the values are immutable
• They're defined at the beginning of a module and accessible throughout the entire module
• They're often exported, though they don't have to be
• They're NOT simple local variables that happen to be declared with const

[w33ble]

There are already places in code where we use have constant values and they generally follow the CAPS_AND_UNDERSCORES pattern. I think it's pretty well established in javascript land and wouldn't expect a lot of pushback on a rule like this.

One place where people seem to disagree is on whether or not these values should be flat or nested. For example, say you had a number of time constants, like so:

export const timeConstants = {
  SECONDS_PER_MINUTE: 60,
  SECONDS_PER_HOUR: 3600,
  SECONDS_PER_DAY: 86400,
};

Here, all values are flat, so you might import them like so:

import { SECONDS_PER_MINUTE, SECONDS_PER_HOUR, SECONDS_PER_DAY } from '/path/to/secondsConstants';

But as the app grows, you have a lot of constants, and it's handy to group them up, probably into separate files and expose them using an index.js file and different "collections" of constants. For example, say there are 2 files, secondsConstants.js and userConstants.js, they could be exposed like so:

// index.js exposing 2 different constants file
export { SECONDS_PER_MINUTE, SECONDS_PER_HOUR, SECONDS_PER_DAY } from './secondsConstants';
export { USER_SESSION_TIMEOUT, USER_DEFAULT_ROLE } from './userConstants';

All values are still available when importing: import { USER_SESSION_TIMEOUT, SECONDS_PER_MINUTE } from /path/to/constants. This is generally my preference. The downside is that all constant names need to be unique across the entire application. This can usually be handled by prefixing them somehow, which in this example, was done with a leading SECONDS_ and USER_ in the constants' names.

Alternatively, they could be exposed as a top-level constant "namespaces". In this example, we have a collection of "seconds" constants and "user" constants, so we could have:

// index.js exposing 2 different constants file
import { PER_MINUTE, PER_HOUR, PER_DAY } from './secondsConstants';
import { SESSION_TIMEOUT, DEFAULT_ROLE } from './userConstants';

export const constants = {
  SECONDS: { PER_MINUTE, PER_HOUR, PER_DAY },
  USER: { SESSION_TIMEOUT, DEFAULT_ROLE },
};

And then we'd use them like import { USER } from '/path/to/constants, and get the value via USER.SESSION_TIMEOUT. We have existing code that follows this convention, and other people seem to prefer it. So this is one place we need to reach consensus: flat constant naming, or nested constants.

[cjcenizal]

I'd prefer your first "flat import" option @w33ble, where you have one module per concern. That seems scalable, and it avoids a situation where you end up with long capitalized paths, e.g. DATE.TIME.SECONDS.PER_MINUTE, which is confusing and unreadable to me.

[stacey-gammon]

@w33ble, in your flat example -

export const timeConstants = {
  SECONDS_PER_MINUTE: 60,
  SECONDS_PER_HOUR: 3600,
  SECONDS_PER_DAY: 86400,
};

you suggest importing them like this:

import { SECONDS_PER_MINUTE, SECONDS_PER_HOUR, SECONDS_PER_DAY } from '/path/to/secondsConstants';

Maybe I'm doing something wrong but that gives me a syntax error. Don't you have to import like this:
import { timeConstants} from './path/to/consts'?

I think the only way to do the import like you say is to export like this:

export const SECONDS_PER_MINUTE = 60;
export const SECONDS_PER_HOUR = 3600;
export const SECONDS_PER_DAY = 86400;

I don't like the long capitalized paths, but I like being able to namespace (e.g. timeConstants.SECONDS_PER_MINUTE).

fwiw I'd be okay with the individual exports, but agree with you that they will often have to be prefixed, with more specific naming. For instance, DashboardConstants and VisualizeConstants have a few that are the same name. Now they are namespaced but if they were converted to be exported individually, they would probably benefit from a type prefix .

[kobelb]

@Stacey-Gammon @w33ble this is one of those situations that being able to import * as constants would allow both use-cases.

[w33ble]

Maybe I'm doing something wrong but that gives me a syntax error. Don't you have to import like this: import { timeConstants} from './path/to/consts'?

@Stacey-Gammon yup, you're right, that's a typo. You'd have to use export default for that to work correctly, like so I guess:

// timeConstants.js
export default {
  SECONDS_PER_MINUTE: 60,
  SECONDS_PER_HOUR: 3600,
  SECONDS_PER_DAY: 86400,
};

// index.js
export { SECONDS_PER_MINUTE, SECONDS_PER_HOUR, SECONDS_PER_DAY } from './timeConstants';

// elsewhere
import { SECONDS_PER_MINUTE, SECONDS_PER_HOUR, SECONDS_PER_DAY } from '/path/to/constants';

// or, use the "* as"
import * as constants from '/path/to/constants';

The question still stands though, do we import a single constants file and destructure off the values we want, or the types of values we want? Or, maybe, do we deal with multiple constants files, which would kind of break the "index.js defines the public API" rule.

[cjcenizal]

Or, maybe, do we deal with multiple constants files, which would kind of break the "index.js defines the public API" rule.

I think we'll be better able to scale our codebase if we design modules to represent both layers within our architecture and concerns within each layer. I think "time", "user", and "visualize sidebar" are good examples of concerns. I don't think "constants" is a concern any more than "singleton", "event bus", or "factory". I think of those more as pattern names.

So if we have a time_constants directory, we could have data_constants.js and time_constants.js inside of that, and index.js could just re-export the constants exported by those files.

[lukasolson]

Okay, let's take a vote, first on whether or not we do CAPS_AND_UNDERSCORES for constant naming, and then if it passes, we can vote on the specifics (for nesting, etc.).

Add a 👍 if you prefer to add CAPS_AND_UNDERSCORES for constants to the style guide, and 👎 if you prefer not to.

[thomasneirynck]

Closing due to inactivity