Use stronger types instead of String in ts type definitions.

[seivan]

• I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior

I am expecting strong types for easier discovery through autocomplete when passing configurations to e.g <Grid>

<Grid container direction={GridDirection.Row}>

Current Behavior

Passing String as configurations to <Grid>

<Grid container direction={'row'}>

Context

From a developer UX point of view, discoverability is the key to making people comfortable and familiar with the large API. Thus offering a decent API is vital.

export enum GridDirection {
    Row = 'row', 
    RowReverse = 'row-reverse',
    Column = 'column',
    Columnreverse = 'column-reverse'
}

Would make things more stronger typed and easier to get help from autocomplete when filling in values at call site.

Tech	Version
Material-UI	v1.0.0-beta.22

[espenh]

Looks like direction is defined using a union of string literal types:

export type GridDirection = 'row' | 'row-reverse' | 'column' | 'column-reverse';

That means that the language service can provide suggestions, and it can complain if you specify an invalid direction.

Here it is in action:

That's VS Code 1.18.1 with TypeScript version 2.6.1, but I think TypeScript introduced string literal types in 1.8.

[seivan]

@espenh It is correct, but only after you've defined your initial string.
What I am suggesting, is making an actual type that can be referenced, filter and searched on .
GridDirection as a type is far more specific than String and allows further help with the editor and can be reused as e.g GridDirection.Column whenever needed.

[pelotom]

It is correct, but only after you've defined your initial string.

I don't understand what you mean.

What I am suggesting, is making an actual type that can be referenced, filter and searched on .

Isn't that true of the existing GridDirection?

GridDirection as a type is far more specific than String and allows further help with the editor and can be reused as e.g GridDirection.Column whenever needed.

The current GridDirection type is already more specific than string, completely type safe and the code editor (at least those using the language service) will help you complete it. I don't think anything is gained by using an enum except extra verbosity. To me enums represent a legacy feature from before the introduction of unions and literal types, and I almost never have occasion to use them in modern TypeScript.

[seivan]

That's not exactly true, from what I can gather string based enums are new in TypeScript, and heavily used where types are needed to be represented under a rawValue. Mostly seen in Swift.
What you gain are better editor support, easier refactoring in case you need to target a certain enum to refactor instead targeting raw strings.
You haven't used them in modern TypeScript because I assume they're new, made because of request from people who wanted improved type correctness.

Sometimes you need a collection of strings collected under a common key. Prior to TypeScript 2.4, TypeScript only supported number-based enums. If using versions prior to 2.4, a work-around is to use string literal types to create string based enums by combining with union types.

Readability and discoverability improves tremendously as it's an indication that you can find more options under GridDirection at a glance while a string doesn't indicate what's being used at the call site. This is my main reason for making the change as well.

e.g

<Grid container direction={'row'}>

Doesn't tell me it's using a specific type, it tells me it's a String.
It doesn't tell me that I have options to chose from unless I explicitly go check the internals of it.
By using an Enum you're indicating choices to be made directly at the call site and not just internally.

Unfortunately I made the poor choice of using Flexbox as the main entry point into the discussion as they meaning in strings.
But my convoluted example, Enums make better APIs than String. Existing types aren't sufficient if api discoverability and design is considered, but goes without saying, that's my opinion.

[pelotom]

That's not exactly true, from what I can gather string based enums are new in TypeScript, and heavily used where types are needed to be represented under a rawValue. Mostly seen in Swift.
What you gain are better editor support, easier refactoring in case you need to target a certain enum to refactor instead targeting raw strings.

I don't know which editor you're using, but any editor using the TS language service has fine support for autocompletion and refactoring of string literal types. (Refactoring support isn't really relevant to this discussion though, because you as a user shouldn't be refactoring the names provided by material-ui...)

You haven't used them in modern TypeScript because I assume they're new, made because of request from people who wanted improved type correctness.

Sometimes you need a collection of strings collected under a common key. Prior to TypeScript 2.4, TypeScript only supported number-based enums. If using versions prior to 2.4, a work-around is to use string literal types to create string based enums by combining with union types.

I'm well aware of string-based enums. I don't use them because they don't actually improve type correctness. What they do increase is verbosity, both at the definition and usage sites. I'd be interested to see an example where using an enum instead of a literal type would prevent a bug.

Readability and discoverability improves tremendously as it's an indication that you can find more options under GridDirection at a glance while a string doesn't indicate what's being used at the call site. This is my main reason for making the change as well.
e.g

<Grid container direction={'row'}>

Doesn't tell me it's using a specific type, it tells me it's a String. It doesn't tell me that I have options to chose from unless I explicitly go check the internals of it.
By using an Enum you're indicating choices to be made directly at the call site and not just internally.

If you type

<Grid container direction="

and hit control-space in VSCode, it will give you dropdown of the possible completions. Again, I don't know what editor you're using, but you might want to look into alternatives. There are plenty that use the language service and understand that direction is not a just a simple string.

And why would you want to write

<Grid container direction={GridDirection.Row} />

instead of just

<Grid container direction="row" />

The latter is simple, clean, readable, free of redundancy and perfectly type safe.

Unfortunately I made the poor choice of using Flexbox as the main entry point into the discussion as they meaning in strings.
But my convoluted example, Enums make better APIs than String. Existing types aren't sufficient if api discoverability and design is considered, but goes without saying, that's my opinion.

Again, I'd really like to see a concrete example where enums offer some real advantage. So far I'm not convinced.

[pelotom]

One other important thing to consider is that enums are not merely types, but require a runtime component as well. Since material-ui is actually written in JS, and typings are provided after the fact as declaration files, every enum one wants to add requires writing corresponding JS code to support it. This is laborious and error-prone, so there had better be a clear case for doing it. And one would hope it was something that would benefit more than just TypeScript users. I frankly can't imagine any JS users wanting to write GridDirection.Row instead of 'row'. But then I can't even imagine why TS users would want to write it 😛

[seivan]

I am using VSCode.

Verbosity sounds like a moot argument, since it's not really more verbose, then again coming from Obj-C, maybe I'm lenient on such things but I also read more than I write :)

Again, when reading code that you or others wrote.

<Grid container direction={GridDirection.Row} />

gives a stronger indication of values of a certain type allowed than

<Grid container direction={'row'} />

I can also see what other approaches I have under GridDirection, last time I checked (I could be wrong here) there is no way for me to see what other values I can use, unless I check what union type Grid is expecting on its direction props.

It seems like I have to convince you why enums are better than union types and that's beyond what I was intending with this. From my point of view, using union types is wrong as it's was only a (temporary) hack because TypeScript was incomplete.
Union types seem more fitting if you'd allow more than one type as the raw value, otherwise nail it down with an enum.

I'd like to leave the hack behind and use the proper tools and types as I'd love better TypeScript support, for me its irrelevant wether or not the implementation layer is in JS.

If it's not convincing enough, then I am sorry, I've explained as far as I can. You can read more about the reasoning here microsoft/TypeScript#3192 as much more involved people than me have figured out why it's necessary.

Thank you for the discussion, feel free to close this.

[pelotom]

As far as readability, it's perfectly clear to me reading

<Grid container direction="row" />

that "row" represents a direction of a Grid; all those words are present in the above syntax, and adding GridDirection only duplicates words that are already present. Redundancy only hinders readability.

If you command-click on direction in the above you will be taken to its definition, and you can easily see the type there.

As for union types being a "hack", I don't know what to tell you except that that's simply not accurate. They are a superior solution to the almost all the problems that enums were originally created to address, and thus it's no surprise that they are far more commonly used in modern TS codebases. The only thing that enums offer above and beyond unions is that they are enumerable, but I haven't seen a use case for that here and the cost of being enumerable is that there has to be a runtime component.