feat: enable eraseable syntax

[nazarhussain]

Motivation

For future adoption of Typescript support in NodeJS, update the types.

Description

• Enable eraseable syntax only
• Update all the enums
• Update all constructors.

Steps to test or reproduce

• Run all tests

[nazarhussain]

To enable eraseable syntax we have to get rid of enum from Typescript files. Which requires to split those enums into types and values respectively.

So we need to stick with some pattern for consistency as an alternate.

Look at the example enum below.

enum OutlierSensitivity = {
  Mild = 1.5,
  Strict = 3.0,
}

type EnumLike<T> = T[keyof T];

So now we have split this information into type and values.

Pattern 1 - capital case enum + camelCase values

const outlierSensitivity = {
  Mild: 1.5,
  Strict: 3.0,
} as const;

type OutlierSensitivity = EnumLike<typeof outlierSensitivity>;

1. More related pattern to our code
2. Most time may conflict with local variables

Pattern 2 - Capital case values + Options suffix type

const OutlierSensitivity = {
  Mild: 1.5,
  Strict: 3.0,
} as const;

type OutlierSensitivityOptions = EnumLike<typeof outlierSensitivity>;

Pattern 3 - Capital case values + E prefix type

const OutlierSensitivity = {
  Mild: 1.5,
  Strict: 3.0,
} as const;

type EOutlierSensitivity = EnumLike<typeof outlierSensitivity>;

Please suggest which one you prefer or suggest any other pattern you like.

[github-actions]

Performance Report

✔️ no performance regression detected

Full benchmark results

Benchmark suite	Current: 034ecf0	Previous: null	Ratio
sum array with raw for loop	933.67 us/op
sum array with reduce	9.9933 ms/op
sum array with reduce beforeEach	81.795 us/op
sum array with reduce before beforeEach	81.450 us/op
sum array with reduce high threshold	10.006 ms/op
sum array with reduce no threshold	10.250 ms/op

by benchmarkbot/action

[wemeetagain]

imo something like pattern 2 is good with me. maybe with the type having Option appended instead of Options.

then we have code like:

const OutlierSensitivity = {
  Mild: 1.5,
  Strict: 3.0,
} as const;

type OutlierSensitivityOption = EnumLike<typeof outlierSensitivity>;


function average(data: number[], sensitivity: OutlierSensitivityOption): boolean {
  ...
}

const a = average([...], OutlierSensitivity.Mild);

[matthewkeil]

imo something like pattern 2 is good with me. maybe with the type having Option appended instead of Options.

I tend to agree with this pattern but would suggest appending Type instead of Option

[nazarhussain]

The option 2 is good, the only problem is naming convention. We stick with the camelCase for variable names. If we use option 2 we have to allow the PascalCase as well then over the code will have mixed notation. One workaround could be to have a fix convention that all enum values variables will start with E then we can configure the linter for it.

[wemeetagain]

The reason we'd keep pascal case for "enum" variable names is because they're treated like a namespace (or rather, like an enum lol). They aren't treated as normal variables. Similar reason we uppercase constants.
The casing is a cue that the data has a different meaning from the other casing.

const Sensitivity = {Mild: 1, Strict: 3} as const;
type SensitivityOption = EnumOption<typeof Sensitivity>;

const sensitivity = Sensitivity.Mild;

[KatyaRyazantseva]

EOutlierSensitivity associates with errors in my experience. Pattern 2 seems to work best.

We already have random Type combos in the code. A couple of examples:

export type TypeJson<T> = {
  toJson: (data: T) => unknown; // server
  fromJson: (data: unknown) => T; // client
};

export type LodestarThreadType = "main" | "network" | "discv5";

How about Enum postfix or prefix? I'd prefer prefix.

type OutlierSensitivityEnum = EnumLike<typeof outlierSensitivity>;
// or
type EnumOutlierSensitivity = EnumLike<typeof outlierSensitivity>;

[ensi321]

Might be irrelevant but I want to mention this example in Google Style Guide https://google.github.io/styleguide/tsguide.html#identifiers-constants

const UNIT_SUFFIXES = {
  'milliseconds': 'ms',
  'seconds': 's',
};

However the purpose in this example is to define this as constant, not for replacing an enum. Among the three options, I prefer 2, since we already have a convention of having PascalCase of const denoting namespace that is widely used in Lodestar.

My 2 cents will be to rename EnumLike to EnumValue and for the suffix, no strong opinion could be Option, Value, Type.

Strong opinion against using single character to denote things eg. E, I

[nazarhussain]

Discussion for enum naming is summarized here ChainSafe/lodestar#7574

[matthewkeil]

Just one thing and the question but looks good overall

[matthewkeil]

Just to check this does not alter the output it just enforces that the syntax meets the erasable syntax specs?

[nazarhussain]

The output will just be changed for the code that we changed. There will be no implicit change in the compiled JS.

[nflaig]

just noting here that while adding a suffix to the type is ok, I don't think using Type as suffix is ideal since we use that for ssz types already, so maybe Options / Opts might be better?

[nazarhussain]

I am fine with either, it's just that we had a consensus on this pattern earlier ChainSafe/lodestar#7574

[nflaig]

the pattern would still be the same, just different suffix, see comment from NC, he voted for option 4 but voiced something similar here ChainSafe/lodestar#7574 (comment)

we are probably fine with using Type as suffix, just wanna point this out so we consider it at least

[nazarhussain]

I am fine with either suffix, as long as everyone is aligned with it.

[matthewkeil]

Please no Options.... its a "type"....

[nflaig]

LGTM

[matthewkeil]

🙈 LGTM!!! 🥸

[wemeetagain]

I see you just did the minimal diff here. But consider removing Type from the const and type

[nazarhussain]

I was afraid to change that considering that may trigger another discussion. :)

[wemeetagain]

lgtm