Export useful developer Typescript types

[aleccaputo]

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

Summary 💡

Ran into an issue where i wanted to use the type ThemeStyle (h1, body2, etc) from createTypography.d.ts to type a prop on a wrapper i made so i didn't have to re-type the long union type in my own project.
Upgraded to the latest version of material and noticed the build broke saying that it couldn't find the type ThemeStyle. Did some digging and found this PR #19269 which renamed the ThemeStyle type to Variant (which makes total sense).

Per a comment in there it looks like nested modules more than one level deep are considered "private".

Would be nice if we could have access to some of those types as TS developers to make typing a little easier/more consistent.

Not sure if there is a way to figure out which types could be exported from the root or it's just a matter of going component to component and making a judgement call.

Motivation 🔦

Types are awesome, let's use them more!

[eps1lon]

Thanks for opening this issue and providing some context. Could you post the code which used the ThemeStyle previously?

[aleccaputo]

@eps1lon

import React, {Fragment, ReactNodeArray} from 'react';
import {
    Grid,
    ListItem,
    ListItemSecondaryAction,
    ListItemText,
    Typography,
    Theme,
    ListItemProps,
    GridJustification
} from '@material-ui/core';
import {makeStyles} from '@material-ui/styles';
import {ThemeStyle} from '@material-ui/core/styles/createTypography';

const useStyles = makeStyles((theme: Theme) => ({
    secondaryText: {
        color: theme.palette.text.secondary
    }
}));

interface Props {
    listItemProps?: ListItemProps;
    actions?: React.ReactNode;
    image?: React.ReactNode | ReactNodeArray;
    justify?: GridJustification;
    primaryText?: React.ReactNode;
    textVariant?: ThemeStyle;
    imageFirst?: boolean;
    textAlignment?: 'left' | 'right' | 'center';
}

textVariant?: ThemeStyle; was the issue (it's now upgraded to use Variant)
Looking now textAlignment is probably also something that could benefit from an exported type

[eps1lon]

We can definitely export those types.

Usually I try not to export too much because most of the time you can pick the appropriate types of deep objects on your own e.g. type SomeDeepType = ThemeOptions['props']['Typography']. The issue here is that the type is somewhat lost since it's used as a key.

Would be really helpful if someone could work on a PR.

[aleccaputo]

@eps1lon @oliviertassinari i'll give it a shot. Should i just focus on Typography for this pull? I don't mind going through and doing a pass on all the components

I want to make sure i don't over-do it...
For example the placement prop on ToolTip is typed as

placement?: | 'bottom-end'
  | 'bottom-start'
  | 'bottom'
  | 'left-end'
  | 'left-start'
  | 'left'
  | 'right-end'
  | 'right-start'
  | 'right'
  | 'top-end'
  | 'top-start'
  | 'top';

any reason i shouldn't make that it's own exported type?

[eps1lon]

any reason i shouldn't make that it's own exported type?

The difference is between key types that are "intersected with" and value types.

For tooltip you can easily get the type with TabProps['placement']. For Record<KeyA, string> & Record<KeyB> I'm not aware of any robust solution to extract KeyA.

[aleccaputo]

@eps1lon hmm okay. I'll start with just the variant if you want to assign this to me

[eps1lon]

@eps1lon hmm okay. I'll start with just the variant if you want to assign this to me

The main benefit here is that we don't have to name it. And it's apparent from this issue that this is hard 😉

[aleccaputo]

@eps1lon good point. Although it looks like there is kind of a convention that is set by the already exported types, for example GridJustification is exported from Grid and is the type for justification

[eps1lon]

Although it looks like there is kind of a convention

I appreciate the good faith but we make mistakes as well 😆

[aleccaputo]

@eps1lon haha fair enough! I like the accidental convention for what it's worth 😃

[oliviertassinari]

If we refactor the Grid.d.ts, should we consider it a breaking change (it seems to be the only of its kind in the codebase)?

[aleccaputo]

@oliviertassinari it would be a breaking change for my codebase. We have a wrapper that evenly spaces items in a grid without having to wrap everything in an item and container that relies heavily on those types (although i think i've seen a few PR's for a component trying to accomplish this same thing)