Decide on typescript syntax

[vnys]

There’s been a discussion on Slack regarding Typescript syntax, adding it here so we have a paper trail in the repo for future generations.

Example of inline types

The consensus was that it looks messy

const StyledDivider = styled.hr(
  ({
    backgroundColor,
    marginTop,
    marginBottom,
    dividerHeight: height,
  }: {
    backgroundColor: string
    marginTop: string
    marginBottom: string
    dividerHeight: string
  }) => ({
    backgroundColor,
    marginTop,
    marginBottom,
    height,
    border: 'none',
  }),
)

Type object

type StyleProps = {
  backgroundColor: string
  marginTop: string
  marginBottom: string
  dividerHeight: string
}
const StyledDivider = styled.hr<StyleProps>`
  border: 'none';
  background-color: ${(props) => props.backgroundColor};
  margin-top: ${(props) => props.marginTop};
  margin-bottom: ${(props) => props.marginBottom};
  height: ${(props) => props.dividerHeight};

Real world example

Some important things to note is that in this example we don’t use destructuring in function arguments or inline type definition.

type Props = {
  color: 'lighter' | 'light' | 'medium'
  variant: 'small' | 'medium'
  className?: string
}
export const Divider = forwardRef<HTMLHRElement, Props>((props, ref) => {
  const { color = 'medium', variant = 'medium', className } = props
  const styleProps = {
    backgroundColor: tokens.color[color],
    marginTop: tokens[variant].spacings.top,
    marginBottom: tokens[variant].spacings.bottom,
    dividerHeight: tokens.height,
  }
  return <StyledDivider {...styleProps} className={className} ref={ref} />
})

This is the proposed solution in the Divider pr. Missing return value, see next point.

Update 31.08.2020

With the above syntax default props has to be repeated both in the js-doc comments and the desctructuring of the props object to be picked up by docgen:

type Props = {
  /** The colour of the thing @default medium */
  color: 'lighter' | 'light' | 'medium'
  variant: 'small' | 'medium'
  className?: string
}

The reason for this is that we don’t actually set the default in the function parameters, but in the destructuring afterwards so in order for docgen to be able to extract the defaults the following syntax must be used instead, even though it might be argued that it’s less legible.

export const Divider = forwardRef<HTMLHRElement, Props>(
  ({ color = 'medium', variant = 'medium', className } , ref) => {
    const styleProps = {
      backgroundColor: tokens.color[color],
      marginTop: tokens[variant].spacings.top,
      marginBottom: tokens[variant].spacings.bottom,
      dividerHeight: tokens.height,
    }
    return <StyledDivider {...styleProps} className={className} ref={ref} />
  }
)

Return value

export const Divider = forwardRef<HTMLHRElement, Props>((props, ref): JSX.Element => {/* here be dragons */}

Pros: The advantage of adding return values is to avoid errors if the developer forgets to return something, and also to avoid void as the return value.

Cons: The drawback of using return value is that prettier inserts a line-break. We can increase the print-width to avoid that to a certain extent – but that would potentially reformat a lot of files. Besides, the return value is inferred anyway so it might not be needed.

Decision: Unclear about this one

Style props

In #700 we use type StyledProps = Pick<Props, 'maxWidth'> to pick one of the props to use in the styled component. This is something that should be investigated further and potentially be used across all components.

Extending props

In some components we use type Props = React.HTMLAttributes<HTMLDivElement>, in others type Props = JSX.IntrinsicElements['nav']. This is something we need to investigate and decide on one of them. There’s a vast number of discussions online about this subject.

Types for tokens

Several components(like Button, Banner, Cards) are updated to use the ComponentToken. This is a mechanism to ensure that the components are as similar as possible.

export const example: ComponentToken = {
   ...
}

ComponentToken

States

States are representation of states in Figma, which are in turn inspired from states in material design.

Entities

If a component has sub parts, add them in a entities object.

export type ExampleToken = ComponentToken & {
  entities(?): {
    icon: ComponentToken
  }
}

export const example: ExampleToken =  {
  ...,
  entities: {
    icon: {
      ...,
    },
  }
}

Color & Variants

If a component has different colors (like info, warning, success or error) or variants (like contained, outlined, ghost or ghost_icon) we export multiple token constants.

export const primary: ComponentToken = {
...
}

export const info: ComponentToken = {
  ...
}

[vnys]

• About extending props -> https://gist.github.com/mfal/88fe927f2061d22425cc78fcec8d260f#gistcomment-3354198

[mimarz]

• About extending props -> https://gist.github.com/mfal/88fe927f2061d22425cc78fcec8d260f#gistcomment-3354198

Good observation, I double checked forwardRef types and it adds the ref props for us (which kinda makes sens). Which explains why it has worked with us doing Props & React.HTMLAttributes<HTMLDivElement>

[mimarz]

Just to make things simpler. You also have these which kinda do the same

• React.HTMLProps
• React.AllHTMLAttributes

[vnys]

An update on extending props, in the <Table /> stories pr, we ended up extending TdHTMLAttributes<HTMLTableDataCellElement> as proposed by @mimarz, this seems to be the way to solve this and should be done similarly in other components.