From ed00c84f2b6ab625cc6afa80cf1a496f9ca67f2f Mon Sep 17 00:00:00 2001 From: Tom Crockett Date: Sun, 27 May 2018 13:44:07 -0700 Subject: [PATCH 01/11] Add a createStyles function --- packages/material-ui/src/index.d.ts | 1 + packages/material-ui/src/index.js | 1 + packages/material-ui/src/styles/createStyles.d.ts | 11 +++++++++++ packages/material-ui/src/styles/createStyles.js | 5 +++++ .../material-ui/src/styles/createStyles.test.js | 9 +++++++++ packages/material-ui/src/styles/index.d.ts | 1 + packages/material-ui/src/styles/index.js | 1 + .../material-ui/test/typescript/styles.spec.tsx | 13 +++++++------ 8 files changed, 36 insertions(+), 6 deletions(-) create mode 100644 packages/material-ui/src/styles/createStyles.d.ts create mode 100644 packages/material-ui/src/styles/createStyles.js create mode 100644 packages/material-ui/src/styles/createStyles.test.js diff --git a/packages/material-ui/src/index.d.ts b/packages/material-ui/src/index.d.ts index 83871dbabc2ce4..f0070f7e9ce39b 100644 --- a/packages/material-ui/src/index.d.ts +++ b/packages/material-ui/src/index.d.ts @@ -1,6 +1,7 @@ import * as React from 'react'; import { StyledComponentProps } from './styles'; export { StyledComponentProps }; +export { createStyles } from './styles'; /** * All standard components exposed by `material-ui` are `StyledComponents` with diff --git a/packages/material-ui/src/index.js b/packages/material-ui/src/index.js index e2a02e5c535cbf..cfe9da4d20e39a 100644 --- a/packages/material-ui/src/index.js +++ b/packages/material-ui/src/index.js @@ -6,6 +6,7 @@ export { colors }; export { createGenerateClassName, createMuiTheme, + createStyles, jssPreset, MuiThemeProvider, withStyles, diff --git a/packages/material-ui/src/styles/createStyles.d.ts b/packages/material-ui/src/styles/createStyles.d.ts new file mode 100644 index 00000000000000..983fbfe5569fd5 --- /dev/null +++ b/packages/material-ui/src/styles/createStyles.d.ts @@ -0,0 +1,11 @@ +import { CSSProperties, StyleRules } from './withStyles'; + +/** + * This function doesn't really "do anything" at runtime, it's just the identity + * function. Its only purpose is to defeat TypeScript's type widening when providing + * style rules to `withStyles` which are a function of the `Theme`. + * + * @param styles a set of style mappings + * @returns the same styles that were passed in + */ +export default function createStyles(styles: S): S; diff --git a/packages/material-ui/src/styles/createStyles.js b/packages/material-ui/src/styles/createStyles.js new file mode 100644 index 00000000000000..adbb7b02637606 --- /dev/null +++ b/packages/material-ui/src/styles/createStyles.js @@ -0,0 +1,5 @@ +// @flow + +export default function createStyles(s: Object) { + return s; +} diff --git a/packages/material-ui/src/styles/createStyles.test.js b/packages/material-ui/src/styles/createStyles.test.js new file mode 100644 index 00000000000000..d91cd017e1888c --- /dev/null +++ b/packages/material-ui/src/styles/createStyles.test.js @@ -0,0 +1,9 @@ +import { assert } from 'chai'; +import { createStyles } from '.'; + +describe('createStyles', () => { + it('is the identity function', () => { + const styles = {}; + assert.strictEqual(createStyles(styles), styles); + }); +}); diff --git a/packages/material-ui/src/styles/index.d.ts b/packages/material-ui/src/styles/index.d.ts index 18053e94055c7e..9e9de75d02f0cd 100644 --- a/packages/material-ui/src/styles/index.d.ts +++ b/packages/material-ui/src/styles/index.d.ts @@ -2,6 +2,7 @@ export { default as createGenerateClassName } from './createGenerateClassName'; export { default as createMuiTheme, Theme, Direction } from './createMuiTheme'; export { default as jssPreset } from './jssPreset'; export { default as MuiThemeProvider } from './MuiThemeProvider'; +export { default as createStyles } from './createStyles'; export { default as withStyles, WithStyles, diff --git a/packages/material-ui/src/styles/index.js b/packages/material-ui/src/styles/index.js index c3822d9932c485..4f795f608250dd 100644 --- a/packages/material-ui/src/styles/index.js +++ b/packages/material-ui/src/styles/index.js @@ -2,5 +2,6 @@ export { default as createGenerateClassName } from './createGenerateClassName'; export { default as createMuiTheme } from './createMuiTheme'; export { default as jssPreset } from './jssPreset'; export { default as MuiThemeProvider } from './MuiThemeProvider'; +export { default as createStyles } from './createStyles'; export { default as withStyles } from './withStyles'; export { default as withTheme } from './withTheme'; diff --git a/packages/material-ui/test/typescript/styles.spec.tsx b/packages/material-ui/test/typescript/styles.spec.tsx index bc9957ff14dad6..3c1565c240afa7 100644 --- a/packages/material-ui/test/typescript/styles.spec.tsx +++ b/packages/material-ui/test/typescript/styles.spec.tsx @@ -1,5 +1,6 @@ import * as React from 'react'; import { + createStyles, withStyles, WithStyles, createMuiTheme, @@ -22,7 +23,7 @@ interface ComponentProps { } // Example 1 -const styles: StyleRulesCallback<'root'> = ({ palette, spacing }) => ({ +const styles = ({ palette, spacing }: Theme) => ({ root: { padding: spacing.unit, backgroundColor: palette.background.default, @@ -192,7 +193,7 @@ const DecoratedComponent = withStyles(styles)( ; // Allow nested pseudo selectors -withStyles<'listItem' | 'guttered'>(theme => ({ +withStyles(theme => createStyles({ guttered: theme.mixins.gutters({ '&:hover': { textDecoration: 'none', @@ -207,8 +208,8 @@ withStyles<'listItem' | 'guttered'>(theme => ({ { type ListItemContentClassKey = 'root' | 'iiiinset' | 'row'; - const styles = withStyles( - theme => ({ + const decorate = withStyles( + theme => createStyles({ // Styled similar to ListItemText root: { '&:first-child': { @@ -237,7 +238,7 @@ withStyles<'listItem' | 'guttered'>(theme => ({ row?: boolean; } - const ListItemContent = styles(props => { + const ListItemContent = decorate(props => { const { children, classes, inset, row } = props; return (
@@ -298,7 +299,7 @@ withStyles<'listItem' | 'guttered'>(theme => ({ { // https://github.com/mui-org/material-ui/issues/11191 - const decorate = withStyles(theme => ({ + const decorate = withStyles(theme => ({ main: {}, })); From ea88ab8a292146d985b678e54c3a21b5fd23690a Mon Sep 17 00:00:00 2001 From: Tom Crockett Date: Sun, 27 May 2018 14:59:47 -0700 Subject: [PATCH 02/11] Add createStyles documentation to TypeScript guide --- .../src/pages/guides/typescript/typescript.md | 89 +++++++++++++++---- 1 file changed, 73 insertions(+), 16 deletions(-) diff --git a/docs/src/pages/guides/typescript/typescript.md b/docs/src/pages/guides/typescript/typescript.md index 46e6c592418376..ebfa5cc56741c3 100644 --- a/docs/src/pages/guides/typescript/typescript.md +++ b/docs/src/pages/guides/typescript/typescript.md @@ -5,21 +5,78 @@ Have a look at the [Create React App with TypeScript](https://github.com/mui-org ## Usage of `withStyles` -The usage of `withStyles` in TypeScript can be a little tricky, so it's worth showing some examples. You can first call `withStyles()` to create a decorator function, like so: +The usage of `withStyles` in TypeScript can be a little tricky, so it's worth showing some examples. -```js -const decorate = withStyles(({ palette, spacing }) => ({ +### Style rules and type widening + +First off, a frequent source of confusion is TypeScript's [type widening](https://blog.mariusschulz.com/2017/02/04/typescript-2-1-literal-type-widening), which causes this example not to work: + +```ts +const styles = { + root: { + display: 'flex', + flexDirection: 'column', + } +}; + +withStyles(styles); // Error! +``` + +The problem is that the type of the `flexDirection` property here is inferred as `string`, but arbitrary strings are not valid. To fix this, you can pass the styles object directly to `withStyles`: + +```ts +withStyles({ + root: { + display: 'flex', + flexDirection: 'column', + }, +}); +``` + +However type widening rears its ugly head once more if you try to make the styles depend on the theme: + +```ts +withStyles(({ palette, spacing }) => ({ root: { + display: 'flex', + flexDirection: 'column', padding: spacing.unit, backgroundColor: palette.background.default, - color: palette.primary.main + color: palette.primary.main, }, })); ``` -This can then subsequently be used to decorate either a stateless functional component or a class component. Suppose we have in either case the following props: +A more general solution is to use the `createStyles` helper function to construct your style rules object: -```js +```ts +// Non-dependent styles +const styles = createStyles({ + root: { + display: 'flex', + flexDirection: 'column', + }, +}); + +// Theme-dependent styles +const styles = ({ palette, spacing }: Theme) => createStyles({ + root: { + display: 'flex', + flexDirection: 'column', + padding: spacing.unit, + backgroundColor: palette.background.default, + color: palette.primary.main, + }, +}); +``` + +`createStyles` is just the identity function; it doesn't "do anything" at runtime, just helps guide type inference at compile time. + +### Styled props + +Once you've obtained a decorator from `withStyles(styles)`, it can be used to decorate either a stateless functional component or a class component. Suppose we have in either case the following props: + +```ts interface Props { text: string; type: TypographyProps['type']; @@ -29,8 +86,8 @@ interface Props { Functional components are straightforward: -```jsx -const DecoratedSFC = decorate(({ text, type, color, classes }) => ( +```tsx +const DecoratedSFC = withStyles(styles)(({ text, type, color, classes }) => ( {text} @@ -39,10 +96,10 @@ const DecoratedSFC = decorate(({ text, type, color, classes }) => ( Class components are a little more cumbersome. Due to a [current limitation in TypeScript's decorator support](https://github.com/Microsoft/TypeScript/issues/4881), `withStyles` can't be used as a class decorator. Instead, we decorate a class component like so: -```jsx +```tsx import { WithStyles } from '@material-ui/core/styles'; -const DecoratedClass = decorate( +const DecoratedClass = withStyles(styles)( class extends React.Component> { render() { const { text, type, color, classes } = this.props @@ -58,7 +115,7 @@ const DecoratedClass = decorate( When your `props` are a union, Typescript needs you to explicitly tell it the type, by providing a generic `` parameter to `decorate`: -```jsx +```tsx import { WithStyles } from '@material-ui/core/styles'; interface Book { @@ -92,7 +149,7 @@ const DecoratedUnionProps = decorate( // <-- without the type argument, w Injecting multiple classes into a component is as straightforward as possible. Take the following code for example. The classes `one` and `two` are both available with type information on the `classes`-prop passed in by `withStyles`. -```jsx +```tsx import { Theme, withStyles, WithStyles } from "material-ui/styles"; import * as React from "react"; @@ -105,7 +162,7 @@ const styles = (theme: Theme) => ({ }, }); -type Props = { +interface Props { someProp: string; }; @@ -131,7 +188,7 @@ When adding custom properties to the `Theme`, you may continue to use it in a st The following example adds an `appDrawer` property that is merged into the one exported by `material-ui`: -```js +```ts import { Theme } from '@material-ui/core/styles/createMuiTheme'; import { Breakpoint } from '@material-ui/core/styles/createBreakpoints'; @@ -154,7 +211,7 @@ declare module '@material-ui/core/styles/createMuiTheme' { And a custom theme factory with additional defaulted options: -```js +```ts import createMuiTheme, { ThemeOptions } from '@material-ui/core/styles/createMuiTheme'; export default function createMyTheme(options: ThemeOptions) { @@ -170,7 +227,7 @@ export default function createMyTheme(options: ThemeOptions) { This could be used like: -```js +```ts import createMyTheme from './styles/createMyTheme'; const theme = createMyTheme({ appDrawer: { breakpoint: 'md' }}); From cb6adb702de9860aa1f9a888682f5f534ea57891 Mon Sep 17 00:00:00 2001 From: Tom Crockett Date: Sun, 27 May 2018 16:55:03 -0700 Subject: [PATCH 03/11] Add InjectedStyles helper type --- .../src/pages/guides/typescript/typescript.md | 118 +++++++++--------- packages/material-ui/src/index.d.ts | 2 +- packages/material-ui/src/styles/index.d.ts | 1 + .../material-ui/src/styles/withStyles.d.ts | 5 + .../test/typescript/styles.spec.tsx | 47 +++---- .../typescript/styling-comparison.spec.tsx | 22 ++-- 6 files changed, 94 insertions(+), 101 deletions(-) diff --git a/docs/src/pages/guides/typescript/typescript.md b/docs/src/pages/guides/typescript/typescript.md index ebfa5cc56741c3..f8aeb763e7ae72 100644 --- a/docs/src/pages/guides/typescript/typescript.md +++ b/docs/src/pages/guides/typescript/typescript.md @@ -5,11 +5,11 @@ Have a look at the [Create React App with TypeScript](https://github.com/mui-org ## Usage of `withStyles` -The usage of `withStyles` in TypeScript can be a little tricky, so it's worth showing some examples. +The usage of `withStyles` in TypeScript can be a little tricky for a number of reasons, but -### Style rules and type widening +### Using `createStyles` to defeat type widening -First off, a frequent source of confusion is TypeScript's [type widening](https://blog.mariusschulz.com/2017/02/04/typescript-2-1-literal-type-widening), which causes this example not to work: +A frequent source of confusion is TypeScript's [type widening](https://blog.mariusschulz.com/2017/02/04/typescript-2-1-literal-type-widening), which causes this example not to work as expected: ```ts const styles = { @@ -22,7 +22,7 @@ const styles = { withStyles(styles); // Error! ``` -The problem is that the type of the `flexDirection` property here is inferred as `string`, but arbitrary strings are not valid. To fix this, you can pass the styles object directly to `withStyles`: +The problem is that the type of the `flexDirection` property is inferred as `string`, which is too arbitrary. To fix this, you can pass the styles object directly to `withStyles`: ```ts withStyles({ @@ -47,7 +47,9 @@ withStyles(({ palette, spacing }) => ({ })); ``` -A more general solution is to use the `createStyles` helper function to construct your style rules object: +This is because TypeScript [widens the return types of function expressions](https://github.com/Microsoft/TypeScript/issues/241). + +Because of this, we recommend using our `createStyles` helper function to construct your style rules object: ```ts // Non-dependent styles @@ -72,35 +74,60 @@ const styles = ({ palette, spacing }: Theme) => createStyles({ `createStyles` is just the identity function; it doesn't "do anything" at runtime, just helps guide type inference at compile time. -### Styled props +### Augmenting your props with `InjectedStyles` -Once you've obtained a decorator from `withStyles(styles)`, it can be used to decorate either a stateless functional component or a class component. Suppose we have in either case the following props: +Since a component decorated with `withStyles(styles)` gets a special `classes` prop injected, you will want to declare it as such: ```ts interface Props { - text: string; - type: TypographyProps['type']; - color: TypographyProps['color']; -}; + // non-style props + foo: number; + bar: boolean; + // injected style props + classes: { + root: string; + paper: string; + button: string; + }; +} + +const styles = (theme: Theme) => createStyles({ + root: { /* ... */ }, + paper: { /* ... */ }, + button: { /* ... */ }, +}); +``` + +However this isn't very [DRY](https://en.wikipedia.org/wiki/Don%27t_repeat_yourself) because it requires you to maintain the class names (`'root'`, `'paper'`, `'button'`, ...) in two different places. We provide a type operator `InjectedStyles` to help with this, so that you can just write + +```ts +import { InjectedStyles } from '@material-ui/core'; + +interface Props extends InjectedStyles { + foo: number; + bar: boolean; +} + +const styles = (theme: Theme) => createStyles({ + root: { /* ... */ }, + paper: { /* ... */ }, + button: { /* ... */ }, +}); ``` -Functional components are straightforward: +### Decorating components + +Applying the `withStyles(styles)` decorator as a function works as expected: ```tsx -const DecoratedSFC = withStyles(styles)(({ text, type, color, classes }) => ( +const DecoratedSFC = withStyles(styles)(({ text, type, color, classes }: Props) => ( {text} )); -``` - -Class components are a little more cumbersome. Due to a [current limitation in TypeScript's decorator support](https://github.com/Microsoft/TypeScript/issues/4881), `withStyles` can't be used as a class decorator. Instead, we decorate a class component like so: - -```tsx -import { WithStyles } from '@material-ui/core/styles'; const DecoratedClass = withStyles(styles)( - class extends React.Component> { + class extends React.Component { render() { const { text, type, color, classes } = this.props return ( @@ -113,11 +140,13 @@ const DecoratedClass = withStyles(styles)( ); ``` +Unfortunately due to a [current limitation of TypeScript decorators](https://github.com/Microsoft/TypeScript/issues/4881), `withStyles` can't be used as a decorator in TypeScript. + +### Union props + When your `props` are a union, Typescript needs you to explicitly tell it the type, by providing a generic `` parameter to `decorate`: ```tsx -import { WithStyles } from '@material-ui/core/styles'; - interface Book { category: "book"; author: string; @@ -128,10 +157,12 @@ interface Painting { artist: string; } -type Props = Book | Painting; +type BookOrPainting = Book | Painting; + +type Props = BookOrPainting & InjectedStyles; -const DecoratedUnionProps = decorate( // <-- without the type argument, we'd get a compiler error! - class extends React.Component> { +const DecoratedUnionProps = withStyles(styles)( // <-- without the type argument, we'd get a compiler error! + class extends React.Component { render() { const props = this.props; return ( @@ -144,43 +175,6 @@ const DecoratedUnionProps = decorate( // <-- without the type argument, w ); ``` - -### Injecting Multiple Classes - -Injecting multiple classes into a component is as straightforward as possible. Take the following code for example. The classes `one` and `two` are both available with type information on the `classes`-prop passed in by `withStyles`. - -```tsx -import { Theme, withStyles, WithStyles } from "material-ui/styles"; -import * as React from "react"; - -const styles = (theme: Theme) => ({ - one: { - backgroundColor: "red", - }, - two: { - backgroundColor: "pink", - }, -}); - -interface Props { - someProp: string; -}; - -type PropsWithStyles = Props & WithStyles>; - -const Component: React.SFC = ({ - classes, - ...props -}: PropsWithStyles) => ( -
-
One
-
Two
-
-); - -export default withStyles(styles)(Component); -``` - ## Customization of `Theme` When adding custom properties to the `Theme`, you may continue to use it in a strongly typed way by exploiting diff --git a/packages/material-ui/src/index.d.ts b/packages/material-ui/src/index.d.ts index f0070f7e9ce39b..3e64488a5ba395 100644 --- a/packages/material-ui/src/index.d.ts +++ b/packages/material-ui/src/index.d.ts @@ -1,7 +1,7 @@ import * as React from 'react'; import { StyledComponentProps } from './styles'; export { StyledComponentProps }; -export { createStyles } from './styles'; +export { InjectedStyles, createStyles } from './styles'; /** * All standard components exposed by `material-ui` are `StyledComponents` with diff --git a/packages/material-ui/src/styles/index.d.ts b/packages/material-ui/src/styles/index.d.ts index 9e9de75d02f0cd..c3413670895218 100644 --- a/packages/material-ui/src/styles/index.d.ts +++ b/packages/material-ui/src/styles/index.d.ts @@ -5,6 +5,7 @@ export { default as MuiThemeProvider } from './MuiThemeProvider'; export { default as createStyles } from './createStyles'; export { default as withStyles, + InjectedStyles, WithStyles, StyleRules, StyleRulesCallback, diff --git a/packages/material-ui/src/styles/withStyles.d.ts b/packages/material-ui/src/styles/withStyles.d.ts index d29708b5355f9c..2bb7a9db114938 100644 --- a/packages/material-ui/src/styles/withStyles.d.ts +++ b/packages/material-ui/src/styles/withStyles.d.ts @@ -41,6 +41,11 @@ export interface WithStyles extends Partial; } +export type InjectedStyles = WithStyles< + R extends StyleRulesCallback ? K : + R extends StyleRules ? K : never +>; + export interface StyledComponentProps { classes?: Partial>; innerRef?: React.Ref | React.RefObject; diff --git a/packages/material-ui/test/typescript/styles.spec.tsx b/packages/material-ui/test/typescript/styles.spec.tsx index 3c1565c240afa7..91d8087c406fef 100644 --- a/packages/material-ui/test/typescript/styles.spec.tsx +++ b/packages/material-ui/test/typescript/styles.spec.tsx @@ -2,22 +2,22 @@ import * as React from 'react'; import { createStyles, withStyles, - WithStyles, createMuiTheme, MuiThemeProvider, Theme, withTheme, StyleRules, + StyleRulesCallback, + StyledComponentProps, + InjectedStyles, } from '../../src/styles'; import Button from '../../src/Button/Button'; -import { StyleRulesCallback, StyledComponentProps } from '../../src/styles/withStyles'; import blue from '../../src/colors/blue'; import { WithTheme } from '../../src/styles/withTheme'; import { StandardProps } from '../../src'; import { TypographyStyle } from '../../src/styles/createTypography'; // Shared types for examples -type ComponentClassNames = 'root'; interface ComponentProps { text: string; } @@ -37,7 +37,7 @@ const StyledExampleOne = withStyles(styles)(({ classes, text }) ; // Example 2 -const Component: React.SFC> = ({ +const Component: React.SFC> = ({ classes, text, }) =>
{text}
; @@ -46,16 +46,16 @@ const StyledExampleTwo = withStyles(styles)(Component); ; // Example 3 -const styleRule: StyleRules = { +const styleRule = createStyles({ root: { display: 'flex', alignItems: 'stretch', height: '100vh', width: '100%', }, -}; +}); -const ComponentWithChildren: React.SFC> = ({ +const ComponentWithChildren: React.SFC> = ({ classes, children, }) =>
{children}
; @@ -167,7 +167,7 @@ const ComponentWithTheme = withTheme()(({ theme }) =>
{theme.spacing.unit}< ; // withStyles + withTheme -type AllTheProps = WithTheme & WithStyles<'root'>; +type AllTheProps = WithTheme & InjectedStyles; const AllTheComposition = withTheme()( withStyles(styles)(({ theme, classes }: AllTheProps) => ( @@ -181,7 +181,7 @@ const AllTheComposition = withTheme()( // due to https://github.com/Microsoft/TypeScript/issues/4881 //@withStyles(styles) const DecoratedComponent = withStyles(styles)( - class extends React.Component> { + class extends React.Component> { render() { const { classes, text } = this.props; return
{text}
; @@ -193,7 +193,7 @@ const DecoratedComponent = withStyles(styles)( ; // Allow nested pseudo selectors -withStyles(theme => createStyles({ +withStyles(theme => ({ guttered: theme.mixins.gutters({ '&:hover': { textDecoration: 'none', @@ -201,7 +201,7 @@ withStyles(theme => createStyles({ }), listItem: { '&:hover $listItemIcon': { - visibility: 'inherit', + // visibility: 'inherit', }, }, })); @@ -262,27 +262,22 @@ withStyles(theme => createStyles({ // The real test here is with "strictFunctionTypes": false, // but we don't have a way currently to test under varying // TypeScript configurations. - interface IStyle { - content: any; - } - interface IComponentProps { + interface ComponentProps extends InjectedStyles { caption: string; } - type ComponentProps = IComponentProps & WithStyles<'content'>; - - const decorate = withStyles((theme): IStyle => ({ + const styles = (theme: Theme) => createStyles({ content: { margin: 4, }, - })); + }); const Component = (props: ComponentProps) => { return
Hello {props.caption}
; }; - const StyledComponent = decorate(Component); + const StyledComponent = withStyles(styles)(Component); class App extends React.Component { public render() { @@ -299,23 +294,21 @@ withStyles(theme => createStyles({ { // https://github.com/mui-org/material-ui/issues/11191 - const decorate = withStyles(theme => ({ + const styles = (theme: Theme) => createStyles({ main: {}, - })); - - type classList = 'main'; + }); - interface IProps { + interface Props extends InjectedStyles { someProp?: string; } - class SomeComponent extends React.PureComponent> { + class SomeComponent extends React.PureComponent { render() { return
; } } - const DecoratedSomeComponent = decorate(SomeComponent); // note that I don't specify a generic type here + const DecoratedSomeComponent = withStyles(styles)(SomeComponent); ; } diff --git a/packages/material-ui/test/typescript/styling-comparison.spec.tsx b/packages/material-ui/test/typescript/styling-comparison.spec.tsx index a437516e1096b6..ec0b046684e091 100644 --- a/packages/material-ui/test/typescript/styling-comparison.spec.tsx +++ b/packages/material-ui/test/typescript/styling-comparison.spec.tsx @@ -1,29 +1,29 @@ import * as React from 'react'; import Typography, { TypographyProps } from '../../src/Typography/Typography'; -import { withStyles, WithStyles } from '../../src/styles'; +import { withStyles, InjectedStyles, createStyles, Theme } from '../../src/styles'; -const decorate = withStyles(({ palette, spacing }) => ({ +const styles = ({ palette, spacing }: Theme) => createStyles({ root: { padding: spacing.unit, backgroundColor: palette.background.default, color: palette.primary.dark, }, -})); +}) -interface Props { +interface Props extends InjectedStyles { color: TypographyProps['color']; text: string; variant: TypographyProps['variant']; } -const DecoratedSFC = decorate(({ text, variant, color, classes }) => ( +const DecoratedSFC = withStyles(styles)(({ text, variant, color, classes }: Props) => ( {text} )); -const DecoratedClass = decorate( - class extends React.Component> { +const DecoratedClass = withStyles(styles)( + class extends React.Component { render() { const { text, variant, color, classes } = this.props; return ( @@ -35,8 +35,8 @@ const DecoratedClass = decorate( }, ); -const DecoratedNoProps = decorate( - class extends React.Component> { +const DecoratedNoProps = withStyles(styles)( + class extends React.Component> { render() { return Hello, World!; } @@ -59,8 +59,8 @@ interface Painting { type ArtProps = Book | Painting; -const DecoratedUnionProps = decorate( // <-- without the type argument, we'd get a compiler error! - class extends React.Component> { +const DecoratedUnionProps = withStyles(styles)( // <-- without the type argument, we'd get a compiler error! + class extends React.Component> { render() { const props = this.props; return ( From d033ba312c39b2115a0c74119f35fef4dced59d0 Mon Sep 17 00:00:00 2001 From: Tom Crockett Date: Sun, 27 May 2018 17:06:06 -0700 Subject: [PATCH 04/11] Wording tweak --- docs/src/pages/guides/typescript/typescript.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/src/pages/guides/typescript/typescript.md b/docs/src/pages/guides/typescript/typescript.md index f8aeb763e7ae72..c8c443e7e30485 100644 --- a/docs/src/pages/guides/typescript/typescript.md +++ b/docs/src/pages/guides/typescript/typescript.md @@ -76,7 +76,7 @@ const styles = ({ palette, spacing }: Theme) => createStyles({ ### Augmenting your props with `InjectedStyles` -Since a component decorated with `withStyles(styles)` gets a special `classes` prop injected, you will want to declare it as such: +Since a component decorated with `withStyles(styles)` gets a special `classes` prop injected, you will want to define its props accordingly: ```ts interface Props { From 8094a7d2c9e018c43ec534d9974109e14131ce40 Mon Sep 17 00:00:00 2001 From: Tom Crockett Date: Sun, 27 May 2018 17:19:02 -0700 Subject: [PATCH 05/11] Use createStyles and InjectedStyles in more tests --- .../material-ui/src/withWidth/withWidth.spec.tsx | 12 +++++++----- .../material-ui/test/typescript/components.spec.tsx | 13 ++++++------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/packages/material-ui/src/withWidth/withWidth.spec.tsx b/packages/material-ui/src/withWidth/withWidth.spec.tsx index 38c76ec693c6ed..d033183bec09b1 100644 --- a/packages/material-ui/src/withWidth/withWidth.spec.tsx +++ b/packages/material-ui/src/withWidth/withWidth.spec.tsx @@ -1,20 +1,22 @@ import * as React from 'react'; import { Grid } from '..'; -import { Theme } from '../styles'; -import withStyles, { WithStyles } from '../styles/withStyles'; +import { Theme, createStyles } from '../styles'; +import withStyles, { InjectedStyles } from '../styles/withStyles'; import withWidth, { WithWidthProps } from '../withWidth'; -const styles = (theme: Theme) => ({ +const styles = (theme: Theme) => createStyles({ root: { + display: 'flex', + flexDirection: 'column', backgroundColor: theme.palette.common.black, }, }); -interface IHelloProps { +interface IHelloProps extends WithWidthProps, InjectedStyles { name?: string; } -export class Hello extends React.Component> { +export class Hello extends React.Component { public static defaultProps = { name: 'Alex', }; diff --git a/packages/material-ui/test/typescript/components.spec.tsx b/packages/material-ui/test/typescript/components.spec.tsx index 64f501e009a336..362d1af5c4b98e 100644 --- a/packages/material-ui/test/typescript/components.spec.tsx +++ b/packages/material-ui/test/typescript/components.spec.tsx @@ -65,9 +65,8 @@ import { Tooltip, Typography, withMobileDialog, - WithStyles, } from '../../src'; -import { withStyles, StyleRulesCallback } from '../../src/styles'; +import { withStyles, StyleRulesCallback, InjectedStyles, Theme, createStyles } from '../../src/styles'; import { DialogProps } from '../../src/Dialog'; const log = console.log; @@ -663,16 +662,16 @@ const StepperTest = () => }; const TableTest = () => { - const styles: StyleRulesCallback<'paper'> = theme => { + const styles = (theme: Theme) => { const backgroundColor: string = theme.palette.secondary.light; - return { + return createStyles({ paper: { width: '100%', marginTop: theme.spacing.unit * 3, backgroundColor, overflowX: 'auto', }, - }; + }); }; let id = 0; @@ -689,7 +688,7 @@ const TableTest = () => { createData('Gingerbread', 356, 16.0, 49, 3.9), ]; - function BasicTable(props: WithStyles<'paper'>) { + function BasicTable(props: InjectedStyles) { const classes = props.classes; return ( @@ -751,7 +750,7 @@ const TabsTest = () => { }, }); - class BasicTabs extends React.Component> { + class BasicTabs extends React.Component> { state = { value: 0, }; From e51ee2713b5dcae54b7b76052c1773305c84eef4 Mon Sep 17 00:00:00 2001 From: Tom Crockett Date: Sun, 27 May 2018 17:30:01 -0700 Subject: [PATCH 06/11] Get rid of InjectedStyles and overload WithStyles to handle style object types --- docs/src/pages/guides/typescript/typescript.md | 10 +++++----- packages/material-ui/src/index.d.ts | 2 +- packages/material-ui/src/styles/index.d.ts | 1 - packages/material-ui/src/styles/withStyles.d.ts | 16 ++++++++-------- .../material-ui/src/withWidth/withWidth.spec.tsx | 4 ++-- .../test/typescript/components.spec.tsx | 6 +++--- .../material-ui/test/typescript/styles.spec.tsx | 14 +++++++------- .../test/typescript/styling-comparison.spec.tsx | 8 ++++---- 8 files changed, 30 insertions(+), 31 deletions(-) diff --git a/docs/src/pages/guides/typescript/typescript.md b/docs/src/pages/guides/typescript/typescript.md index c8c443e7e30485..4d9dac73df47b8 100644 --- a/docs/src/pages/guides/typescript/typescript.md +++ b/docs/src/pages/guides/typescript/typescript.md @@ -74,7 +74,7 @@ const styles = ({ palette, spacing }: Theme) => createStyles({ `createStyles` is just the identity function; it doesn't "do anything" at runtime, just helps guide type inference at compile time. -### Augmenting your props with `InjectedStyles` +### Augmenting your props using `WithStyles` Since a component decorated with `withStyles(styles)` gets a special `classes` prop injected, you will want to define its props accordingly: @@ -98,12 +98,12 @@ const styles = (theme: Theme) => createStyles({ }); ``` -However this isn't very [DRY](https://en.wikipedia.org/wiki/Don%27t_repeat_yourself) because it requires you to maintain the class names (`'root'`, `'paper'`, `'button'`, ...) in two different places. We provide a type operator `InjectedStyles` to help with this, so that you can just write +However this isn't very [DRY](https://en.wikipedia.org/wiki/Don%27t_repeat_yourself) because it requires you to maintain the class names (`'root'`, `'paper'`, `'button'`, ...) in two different places. We provide a type operator `WithStyles` to help with this, so that you can just write ```ts -import { InjectedStyles } from '@material-ui/core'; +import { WithStyles } from '@material-ui/core'; -interface Props extends InjectedStyles { +interface Props extends WithStyles { foo: number; bar: boolean; } @@ -159,7 +159,7 @@ interface Painting { type BookOrPainting = Book | Painting; -type Props = BookOrPainting & InjectedStyles; +type Props = BookOrPainting & WithStyles; const DecoratedUnionProps = withStyles(styles)( // <-- without the type argument, we'd get a compiler error! class extends React.Component { diff --git a/packages/material-ui/src/index.d.ts b/packages/material-ui/src/index.d.ts index 3e64488a5ba395..34e8abd6fc864e 100644 --- a/packages/material-ui/src/index.d.ts +++ b/packages/material-ui/src/index.d.ts @@ -1,7 +1,6 @@ import * as React from 'react'; import { StyledComponentProps } from './styles'; export { StyledComponentProps }; -export { InjectedStyles, createStyles } from './styles'; /** * All standard components exposed by `material-ui` are `StyledComponents` with @@ -76,6 +75,7 @@ export { Theme, withStyles, WithStyles, + createStyles, withTheme, WithTheme, } from './styles'; diff --git a/packages/material-ui/src/styles/index.d.ts b/packages/material-ui/src/styles/index.d.ts index c3413670895218..9e9de75d02f0cd 100644 --- a/packages/material-ui/src/styles/index.d.ts +++ b/packages/material-ui/src/styles/index.d.ts @@ -5,7 +5,6 @@ export { default as MuiThemeProvider } from './MuiThemeProvider'; export { default as createStyles } from './createStyles'; export { default as withStyles, - InjectedStyles, WithStyles, StyleRules, StyleRulesCallback, diff --git a/packages/material-ui/src/styles/withStyles.d.ts b/packages/material-ui/src/styles/withStyles.d.ts index 2bb7a9db114938..c47bf728958072 100644 --- a/packages/material-ui/src/styles/withStyles.d.ts +++ b/packages/material-ui/src/styles/withStyles.d.ts @@ -37,14 +37,14 @@ export interface WithStylesOptions extends JSS export type ClassNameMap = Record; -export interface WithStyles extends Partial { - classes: ClassNameMap; -} - -export type InjectedStyles = WithStyles< - R extends StyleRulesCallback ? K : - R extends StyleRules ? K : never ->; +export type WithStyles = Partial & { + classes: ClassNameMap< + T extends string ? T : + T extends StyleRulesCallback ? K : + T extends StyleRules ? K : + never + >; +}; export interface StyledComponentProps { classes?: Partial>; diff --git a/packages/material-ui/src/withWidth/withWidth.spec.tsx b/packages/material-ui/src/withWidth/withWidth.spec.tsx index d033183bec09b1..3ed75491a498cc 100644 --- a/packages/material-ui/src/withWidth/withWidth.spec.tsx +++ b/packages/material-ui/src/withWidth/withWidth.spec.tsx @@ -1,7 +1,7 @@ import * as React from 'react'; import { Grid } from '..'; import { Theme, createStyles } from '../styles'; -import withStyles, { InjectedStyles } from '../styles/withStyles'; +import withStyles, { WithStyles } from '../styles/withStyles'; import withWidth, { WithWidthProps } from '../withWidth'; const styles = (theme: Theme) => createStyles({ @@ -12,7 +12,7 @@ const styles = (theme: Theme) => createStyles({ }, }); -interface IHelloProps extends WithWidthProps, InjectedStyles { +interface IHelloProps extends WithWidthProps, WithStyles { name?: string; } diff --git a/packages/material-ui/test/typescript/components.spec.tsx b/packages/material-ui/test/typescript/components.spec.tsx index 362d1af5c4b98e..988b62a170432f 100644 --- a/packages/material-ui/test/typescript/components.spec.tsx +++ b/packages/material-ui/test/typescript/components.spec.tsx @@ -66,7 +66,7 @@ import { Typography, withMobileDialog, } from '../../src'; -import { withStyles, StyleRulesCallback, InjectedStyles, Theme, createStyles } from '../../src/styles'; +import { withStyles, StyleRulesCallback, WithStyles, Theme, createStyles } from '../../src/styles'; import { DialogProps } from '../../src/Dialog'; const log = console.log; @@ -688,7 +688,7 @@ const TableTest = () => { createData('Gingerbread', 356, 16.0, 49, 3.9), ]; - function BasicTable(props: InjectedStyles) { + function BasicTable(props: WithStyles) { const classes = props.classes; return ( @@ -750,7 +750,7 @@ const TabsTest = () => { }, }); - class BasicTabs extends React.Component> { + class BasicTabs extends React.Component> { state = { value: 0, }; diff --git a/packages/material-ui/test/typescript/styles.spec.tsx b/packages/material-ui/test/typescript/styles.spec.tsx index 91d8087c406fef..8f7685ce05493f 100644 --- a/packages/material-ui/test/typescript/styles.spec.tsx +++ b/packages/material-ui/test/typescript/styles.spec.tsx @@ -9,7 +9,7 @@ import { StyleRules, StyleRulesCallback, StyledComponentProps, - InjectedStyles, + WithStyles, } from '../../src/styles'; import Button from '../../src/Button/Button'; import blue from '../../src/colors/blue'; @@ -37,7 +37,7 @@ const StyledExampleOne = withStyles(styles)(({ classes, text }) ; // Example 2 -const Component: React.SFC> = ({ +const Component: React.SFC> = ({ classes, text, }) =>
{text}
; @@ -55,7 +55,7 @@ const styleRule = createStyles({ }, }); -const ComponentWithChildren: React.SFC> = ({ +const ComponentWithChildren: React.SFC> = ({ classes, children, }) =>
{children}
; @@ -167,7 +167,7 @@ const ComponentWithTheme = withTheme()(({ theme }) =>
{theme.spacing.unit}< ; // withStyles + withTheme -type AllTheProps = WithTheme & InjectedStyles; +type AllTheProps = WithTheme & WithStyles; const AllTheComposition = withTheme()( withStyles(styles)(({ theme, classes }: AllTheProps) => ( @@ -181,7 +181,7 @@ const AllTheComposition = withTheme()( // due to https://github.com/Microsoft/TypeScript/issues/4881 //@withStyles(styles) const DecoratedComponent = withStyles(styles)( - class extends React.Component> { + class extends React.Component> { render() { const { classes, text } = this.props; return
{text}
; @@ -263,7 +263,7 @@ withStyles(theme => ({ // but we don't have a way currently to test under varying // TypeScript configurations. - interface ComponentProps extends InjectedStyles { + interface ComponentProps extends WithStyles { caption: string; } @@ -298,7 +298,7 @@ withStyles(theme => ({ main: {}, }); - interface Props extends InjectedStyles { + interface Props extends WithStyles { someProp?: string; } diff --git a/packages/material-ui/test/typescript/styling-comparison.spec.tsx b/packages/material-ui/test/typescript/styling-comparison.spec.tsx index ec0b046684e091..0e6b7f998d3f30 100644 --- a/packages/material-ui/test/typescript/styling-comparison.spec.tsx +++ b/packages/material-ui/test/typescript/styling-comparison.spec.tsx @@ -1,6 +1,6 @@ import * as React from 'react'; import Typography, { TypographyProps } from '../../src/Typography/Typography'; -import { withStyles, InjectedStyles, createStyles, Theme } from '../../src/styles'; +import { withStyles, WithStyles, createStyles, Theme } from '../../src/styles'; const styles = ({ palette, spacing }: Theme) => createStyles({ root: { @@ -10,7 +10,7 @@ const styles = ({ palette, spacing }: Theme) => createStyles({ }, }) -interface Props extends InjectedStyles { +interface Props extends WithStyles { color: TypographyProps['color']; text: string; variant: TypographyProps['variant']; @@ -36,7 +36,7 @@ const DecoratedClass = withStyles(styles)( ); const DecoratedNoProps = withStyles(styles)( - class extends React.Component> { + class extends React.Component> { render() { return Hello, World!; } @@ -60,7 +60,7 @@ interface Painting { type ArtProps = Book | Painting; const DecoratedUnionProps = withStyles(styles)( // <-- without the type argument, we'd get a compiler error! - class extends React.Component> { + class extends React.Component> { render() { const props = this.props; return ( From 828e66430e95174387f6a41a11499007793b19c1 Mon Sep 17 00:00:00 2001 From: Tom Crockett Date: Mon, 28 May 2018 07:17:34 -0700 Subject: [PATCH 07/11] Fill out missing text in typescript guide --- docs/src/pages/guides/typescript/typescript.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/src/pages/guides/typescript/typescript.md b/docs/src/pages/guides/typescript/typescript.md index 4d9dac73df47b8..fe9f648a00451e 100644 --- a/docs/src/pages/guides/typescript/typescript.md +++ b/docs/src/pages/guides/typescript/typescript.md @@ -5,7 +5,7 @@ Have a look at the [Create React App with TypeScript](https://github.com/mui-org ## Usage of `withStyles` -The usage of `withStyles` in TypeScript can be a little tricky for a number of reasons, but +Using `withStyles` in TypeScript can be a little tricky, but there are some utilities to make the experience as painless as possible. ### Using `createStyles` to defeat type widening From 15490cd679b3385614679838554f2d748311fc62 Mon Sep 17 00:00:00 2001 From: Tom Crockett Date: Mon, 28 May 2018 07:26:46 -0700 Subject: [PATCH 08/11] Put styles before props --- .../src/pages/guides/typescript/typescript.md | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/docs/src/pages/guides/typescript/typescript.md b/docs/src/pages/guides/typescript/typescript.md index fe9f648a00451e..0114fd3aa78bfa 100644 --- a/docs/src/pages/guides/typescript/typescript.md +++ b/docs/src/pages/guides/typescript/typescript.md @@ -79,6 +79,12 @@ const styles = ({ palette, spacing }: Theme) => createStyles({ Since a component decorated with `withStyles(styles)` gets a special `classes` prop injected, you will want to define its props accordingly: ```ts +const styles = (theme: Theme) => createStyles({ + root: { /* ... */ }, + paper: { /* ... */ }, + button: { /* ... */ }, +}); + interface Props { // non-style props foo: number; @@ -90,12 +96,6 @@ interface Props { button: string; }; } - -const styles = (theme: Theme) => createStyles({ - root: { /* ... */ }, - paper: { /* ... */ }, - button: { /* ... */ }, -}); ``` However this isn't very [DRY](https://en.wikipedia.org/wiki/Don%27t_repeat_yourself) because it requires you to maintain the class names (`'root'`, `'paper'`, `'button'`, ...) in two different places. We provide a type operator `WithStyles` to help with this, so that you can just write @@ -103,16 +103,16 @@ However this isn't very [DRY](https://en.wikipedia.org/wiki/Don%27t_repeat_yours ```ts import { WithStyles } from '@material-ui/core'; -interface Props extends WithStyles { - foo: number; - bar: boolean; -} - const styles = (theme: Theme) => createStyles({ root: { /* ... */ }, paper: { /* ... */ }, button: { /* ... */ }, }); + +interface Props extends WithStyles { + foo: number; + bar: boolean; +} ``` ### Decorating components From e3d28e3548ef720ec6011788bc5a02d8d99e22b0 Mon Sep 17 00:00:00 2001 From: Tom Crockett Date: Mon, 28 May 2018 07:27:33 -0700 Subject: [PATCH 09/11] Restore visibility setting and use createStyles to fix type error --- packages/material-ui/test/typescript/styles.spec.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/material-ui/test/typescript/styles.spec.tsx b/packages/material-ui/test/typescript/styles.spec.tsx index 8f7685ce05493f..b4076ca3e8e2cb 100644 --- a/packages/material-ui/test/typescript/styles.spec.tsx +++ b/packages/material-ui/test/typescript/styles.spec.tsx @@ -193,7 +193,7 @@ const DecoratedComponent = withStyles(styles)( ; // Allow nested pseudo selectors -withStyles(theme => ({ +withStyles(theme => createStyles({ guttered: theme.mixins.gutters({ '&:hover': { textDecoration: 'none', @@ -201,7 +201,7 @@ withStyles(theme => ({ }), listItem: { '&:hover $listItemIcon': { - // visibility: 'inherit', + visibility: 'inherit', }, }, })); From 4c89da6a91efb21dced1e3b06798036de2c9e0a1 Mon Sep 17 00:00:00 2001 From: Tom Crockett Date: Mon, 28 May 2018 07:33:30 -0700 Subject: [PATCH 10/11] Make test code more idiomatic --- .../test/typescript/styles.spec.tsx | 55 +++++++++---------- 1 file changed, 25 insertions(+), 30 deletions(-) diff --git a/packages/material-ui/test/typescript/styles.spec.tsx b/packages/material-ui/test/typescript/styles.spec.tsx index b4076ca3e8e2cb..b088a3118f1aaf 100644 --- a/packages/material-ui/test/typescript/styles.spec.tsx +++ b/packages/material-ui/test/typescript/styles.spec.tsx @@ -207,49 +207,44 @@ withStyles(theme => createStyles({ })); { - type ListItemContentClassKey = 'root' | 'iiiinset' | 'row'; - const decorate = withStyles( - theme => createStyles({ - // Styled similar to ListItemText - root: { - '&:first-child': { - paddingLeft: 0, - }, - flex: '1 1 auto', - padding: '0 16px', + const styles = (theme: Theme) => createStyles({ + // Styled similar to ListItemText + root: { + '&:first-child': { + paddingLeft: 0, }, + flex: '1 1 auto', + padding: '0 16px', + }, - iiiinset: { - '&:first-child': { - paddingLeft: theme.spacing.unit * 7, - }, + iiiinset: { + '&:first-child': { + paddingLeft: theme.spacing.unit * 7, }, - row: { - alignItems: 'center', - display: 'flex', - flexDirection: 'row', - }, - }), - { name: 'ui-ListItemContent' }, - ); + }, + row: { + alignItems: 'center', + display: 'flex', + flexDirection: 'row', + }, + }); - interface ListItemContentProps extends StyledComponentProps { + interface ListItemContentProps extends WithStyles { inset?: boolean; row?: boolean; } - const ListItemContent = decorate(props => { - const { children, classes, inset, row } = props; - return ( -
+ const ListItemContent = withStyles(styles, { name: 'ui-ListItemContent' })( + ({ children, classes, inset, row }) => ( +
{children}
- ); - }); + ) + ); } { - interface FooProps extends StyledComponentProps<'x' | 'y'> { + interface FooProps extends WithStyles<'x' | 'y'> { a: number; b: boolean; } From 70c9c164c236bdd2f671585a542be7f32c9214a1 Mon Sep 17 00:00:00 2001 From: Tom Crockett Date: Mon, 28 May 2018 07:48:09 -0700 Subject: [PATCH 11/11] Reverse order of type union so that error messages are clearer --- docs/src/pages/guides/typescript/typescript.md | 5 ++++- packages/material-ui/src/styles/withStyles.d.ts | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/docs/src/pages/guides/typescript/typescript.md b/docs/src/pages/guides/typescript/typescript.md index 0114fd3aa78bfa..94d83924700f3d 100644 --- a/docs/src/pages/guides/typescript/typescript.md +++ b/docs/src/pages/guides/typescript/typescript.md @@ -19,7 +19,10 @@ const styles = { } }; -withStyles(styles); // Error! +withStyles(styles); +// ^^^^^^ +// Types of property 'flexDirection' are incompatible. +// Type 'string' is not assignable to type '"-moz-initial" | "inherit" | "initial" | "revert" | "unset" | "column" | "column-reverse" | "row"...'. ``` The problem is that the type of the `flexDirection` property is inferred as `string`, which is too arbitrary. To fix this, you can pass the styles object directly to `withStyles`: diff --git a/packages/material-ui/src/styles/withStyles.d.ts b/packages/material-ui/src/styles/withStyles.d.ts index c47bf728958072..995ed8f92a5c3c 100644 --- a/packages/material-ui/src/styles/withStyles.d.ts +++ b/packages/material-ui/src/styles/withStyles.d.ts @@ -52,7 +52,7 @@ export interface StyledComponentProps { } export default function withStyles( - style: StyleRules | StyleRulesCallback, + style: StyleRulesCallback | StyleRules, options?: WithStylesOptions, ): {

>>(