General feedback from creating TypeScript type definitions.

[erictaylor]

I've worked on creating type definitions for TypeScript for both styletron-standard (DefinitelyTyped/DefinitelyTyped#35324) and styletron-react (DefinitelyTyped/DefinitelyTyped#35349), and have some general feedback I'd like to provide of things I came across when writing these definitions.

Typing StyleObject

The StyleObject type defined in styletron-standard is an interesting definition to type in TypeScript because of the allowed nesting of media queries, pseudo selections, etc beside the explicitly typed Properties interface. Typing StyleObject to be an intersection of Properties and an object with a index signature that’s value is itself recursively doesn’t work in TypeScript. In TypeScript once you set an index signature the values of the explicit properties must match that of the index signature. This obviously isn’t possible and ultimately lead to StyledObject being typed as:

export type StyleObject = Properties & {
  [key in string]: Properties[keyof Properties] | StyleObject }
};

This isn’t exactly accurate, and could potentially allow consumers to write styled objects with unknown properties without a warning.

Example:

const style: StyleObject = {
  display: 'block',
  unknown: 'this should have been a object',
  ':after' : {
    content: '" "',
  }
}

If StyleObject had a defined property to put your nesting it would guarantee better type safety and would allow runtime checks to be removed in places where StyleObject is parsed.

Example:

type StyleObject = Properties & {
  $extend: StyleObject,
}

Obviously that would be a breaking change to how style objects are defined today, but perhaps it could be considered?

Properties types

When typing Properties I pretty much copied the Flow types over and converted them to TypeScript interfaces where appropriate. I have two observations in regards to the Properties type.

1. There are a numerous properties that take keywords but are typed with their union of accepted string literals, but also include the string type in the union. Adding the string type into the union pretty much eliminates intelligent suggestions in editors. The string literals pretty much get ignored and crushed by the string type. I’d love to see properties that only except keywords have their string type removed.

export type DisplayProperty =
    | Globals
    | DisplayOutside
    | DisplayInside
    | DisplayInternal
    | DisplayLegacy
    | 'contents'
    | 'list-item'
    | 'none'
    | string; <— Remove?

2. There sure appears to have been a lot of work put into typing the Properties type out, and I hope for someone’s sanity it wasn’t manually done 😆. I’d like suggest that the csstype package should be used. This would allow us to simple type Properties out as type Properties = CSS.Properties<string | number>. See: https://github.com/frenic/csstype#readme. Moving to use this library would eliminate the corrections to keyworded properties as I described above.

Stack info typing

The Styletron interface (in styletron-react) has a debug property with stackInfo containing many any types. stackInfo could be represented with a StackInfo type defined as follows:

export interface StackInfo {
    stack: string | undefined;
    stacktrace: any;
    message: string;
}

I would also suggest dropping stacktrace as it’s not used and isn’t even a property that comes from new Error(). See: https://github.com/styletron/styletron/blob/master/packages/styletron-react/src/index.js#L154-L159.

---

Feel free to look over the types definition PRs and comment if you see anything that needs correcting, or use anything you can back in your Flow types. I know there were a number of any types in the Flow code that I was able to eliminate in the TypeScript definitions.

Cheers 🍻

[frenic]

There are a numerous properties that take keywords but are typed with their union of accepted string literals, but also include the string type in the union. Adding the string type into the union pretty much eliminates intelligent suggestions in editors. The string literals pretty much get ignored and crushed by the string type. I’d love to see properties that only except keywords have their string type removed.

string only exists on properties that needs string. Like display, that you bring up as example, where you can have space separated values. Related frenic/csstype#8 (comment)

[erictaylor]

string only exists on properties that needs string. Like display, that you bring up as example, where you can have space separated values. Related frenic/csstype#8 (comment)

👍

[tajo]

cc @jh3y who is working on TypeScript definitions for Styletron now