Skip to content

Commit

Permalink
[typescript] Constrain props type param appropriately in withStyles, …
Browse files Browse the repository at this point in the history
…withTheme, withWidth HOCs (#11003)

* Rewrite withStyles definition, using Omit

My experience is that the current `withStyles` declaration works poorly with Typescript 2.8.  
I'm not sure that it's the compiler version that's the issue, though.
At least in my usage, it fails to remove `WithStyles<ClassKey>` from the return type.

This definition works better in my experience, doesn't rely on subtle inferencer behavior in the same way, and seems to be in the mainstream of current usage.  

There's a built-in `Exclude` function in Typescript 2.8 that does what `Omit` does.
I used `Omit` for compatibility with older compilers.

* Fix test issues; redesign withTheme, withWidth using Omit also.

* Use preferred style in tests

* Remove another inferrable generic parameter from test.

* Remove another inferrable generic parameter from test.

* Make inclusion of WithStyles in the innerComponent's type def optional.

* Commit for review and suggestions; DOES NOT COMPILE.

Remove commented-on code from withStyles;
use ConsistentWith typewrapper;
add SFC test for withWidth
add FAILING test for doc scenario 2 (withStyles of union)

* Redefine with* to remove Omit.
Trim obsolete doc.
Rename "decorate" function to "withMyStyles" in doc.
Remove redundant failing tests added to clarify Omit problem.

* Bring back explicit generic parameter on union case in test, doc.
In package.json, up-rev Typescript to >2.6.2, and print Typescript
version in "typescript" run command.

* Rename withMyStyles back to decorate.
Use single quotes in test.
In package.json, revert typescript to 2.6.1, remove version-printing run command
In package.lock, revert typescript to 2.6.1.

* Add trailing comma to function params in styling-comparison test.

* Fix package.json, yarn.lock.

* Remove newline from typescript doc.

* Remove needless Omit imports.

* Remove extra explicit from doc, add trailing semicolon to function

* Remove comment from styling-comparison.spec.tsx
  • Loading branch information
estaub authored and pelotom committed Apr 17, 2018
1 parent 9887613 commit 0e86314
Show file tree
Hide file tree
Showing 9 changed files with 57 additions and 50 deletions.
41 changes: 1 addition & 40 deletions docs/src/pages/guides/typescript/typescript.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,45 +56,7 @@ const DecoratedClass = decorate(
);
```

Note that in the class example you didn't need to annotate `<Props>` in the call to `decorate`; type inference took care of everything. However, there are 2 scenarios where you _do_ need to provide an explicit type argument to `decorate`.

Scenario 1: your styled component takes _no_ additional props in addition to `classes`. The natural thing would be to write:

```jsx
import { WithStyles } from 'material-ui/styles';

const DecoratedNoProps = decorate(
class extends React.Component<WithStyles<'root'>> {
render() {
return (
<Typography classes={this.props.classes}>
Hello, World!
</Typography>
);
}
}
);
```

Unfortunately, TypeScript infers the wrong type in this case and you'll have trouble when you go to make an element of this component. In this case, you'll need to provide an explicit `{}` type argument, like so:

```jsx
import { WithStyles } from 'material-ui/styles';

const DecoratedNoProps = decorate<{}>( // <-- note the type argument!
class extends React.Component<WithStyles<'root'>> {
render() {
return (
<Typography classes={this.props.classes}>
Hello, World!
</Typography>
);
}
}
);
```

Scenario 2: `Props` is a union type. Again, to avoid getting a compiler error, you'll need to provide an explict type argument:
When your `props` are a union, Typescript needs you to explicitly tell it the type, by providing a generic `<Props>` parameter to `decorate`:

```jsx
import { WithStyles } from 'material-ui/styles';
Expand Down Expand Up @@ -125,7 +87,6 @@ const DecoratedUnionProps = decorate<Props>( // <-- without the type argument, w
);
```

To avoid worrying about these 2 edge cases, it may be a good habit to always provide an explicit type argument to `decorate`.

### Injecting Multiple Classes

Expand Down
3 changes: 3 additions & 0 deletions packages/material-ui/src/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ type Diff<T extends string, U extends string> = ({ [P in T]: P } &
/** @internal */
export type Omit<T, K extends keyof T> = Pick<T, Diff<keyof T, K>>;

/** @internal */
export type ConsistentWith<O> = Partial<O> & Record<string, any>;

export namespace PropTypes {
type Alignment = 'inherit' | 'left' | 'center' | 'right' | 'justify';
type Color = 'inherit' | 'primary' | 'secondary' | 'default';
Expand Down
16 changes: 11 additions & 5 deletions packages/material-ui/src/styles/withStyles.d.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import * as React from 'react';
import { WithTheme } from '../styles/withTheme';
import { ConsistentWith } from '..';
import { Theme } from './createMuiTheme';
import * as CSS from 'csstype';

Expand Down Expand Up @@ -34,9 +36,8 @@ export interface WithStylesOptions {

export type ClassNameMap<ClassKey extends string = string> = Record<ClassKey, string>;

export interface WithStyles<ClassKey extends string = string> {
export interface WithStyles<ClassKey extends string = string> extends Partial<WithTheme> {
classes: ClassNameMap<ClassKey>;
theme?: Theme;
}

export interface StyledComponentProps<ClassKey extends string = string> {
Expand All @@ -47,6 +48,11 @@ export interface StyledComponentProps<ClassKey extends string = string> {
export default function withStyles<ClassKey extends string>(
style: StyleRules<ClassKey> | StyleRulesCallback<ClassKey>,
options?: WithStylesOptions,
): <P>(
component: React.ComponentType<P & WithStyles<ClassKey>>,
) => React.ComponentType<P & StyledComponentProps<ClassKey>>;
): {
(
component: React.ComponentType<WithStyles<ClassKey>>,
): React.ComponentType<StyledComponentProps<ClassKey>>;
<P extends ConsistentWith<WithStyles<ClassKey>>>(
component: React.ComponentType<P & WithStyles<ClassKey>>,
): React.ComponentType<P & StyledComponentProps<ClassKey>>;
};
3 changes: 2 additions & 1 deletion packages/material-ui/src/styles/withTheme.d.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { Theme } from './createMuiTheme';
import { ConsistentWith } from '..';

export interface WithTheme {
theme: Theme;
}

declare const withTheme: () => <P>(
declare const withTheme: () => <P extends ConsistentWith<WithTheme>>(
component: React.ComponentType<P & WithTheme>,
) => React.ComponentClass<P>;

Expand Down
3 changes: 2 additions & 1 deletion packages/material-ui/src/utils/withWidth.d.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Breakpoint } from '../styles/createBreakpoints';
import { ConsistentWith } from '..';

export interface WithWidthOptions {
resizeInterval: number;
Expand All @@ -22,6 +23,6 @@ export function isWidthUp(

export default function withWidth(
options?: WithWidthOptions,
): <P>(
): <P extends ConsistentWith<WithWidthProps>>(
component: React.ComponentType<P & WithWidthProps>,
) => React.ComponentClass<P & Partial<WithWidthProps>>;
7 changes: 7 additions & 0 deletions packages/material-ui/src/utils/withWidth.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,10 @@ export class Hello extends React.Component<IHelloProps & WithWidthProps & WithSt
const Decorated = withWidth()(withStyles(styles)(Hello));

<Decorated name="Bob" />;

const WidthSFC = withWidth()<{
// shouldn't need to specify width here; it's a given
name: string;
}>(({ width, name }) => <div style={{ width }}>hello, {name}</div>);

<WidthSFC name="Hortense" />;
2 changes: 1 addition & 1 deletion packages/material-ui/test/typescript/components.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -718,7 +718,7 @@ const TableTest = () => {
);
}

return withStyles(styles)<{}>(BasicTable);
return withStyles(styles)(BasicTable);
};

const TabsTest = () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/material-ui/test/typescript/styles.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ const ComponentWithChildren: React.SFC<WithStyles<ComponentClassNames>> = ({
children,
}) => <div className={classes.root}>{children}</div>;

const StyledExampleThree = withStyles(styleRule)<{}>(ComponentWithChildren);
const StyledExampleThree = withStyles(styleRule)(ComponentWithChildren);
<StyledExampleThree />;

// Also works with a plain object
Expand Down
30 changes: 29 additions & 1 deletion packages/material-ui/test/typescript/styling-comparison.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ const DecoratedClass = decorate(
},
);

const DecoratedNoProps = decorate<{}>(
const DecoratedNoProps = decorate(
class extends React.Component<WithStyles<'root'>> {
render() {
return <Typography classes={this.props.classes}>Hello, World!</Typography>;
Expand All @@ -46,3 +46,31 @@ const DecoratedNoProps = decorate<{}>(
const sfcElem = <DecoratedSFC text="Hello, World!" variant="title" color="secondary" />;
const classElem = <DecoratedClass text="Hello, World!" variant="title" color="secondary" />;
const noPropsElem = <DecoratedNoProps />;

interface Book {
category: 'book';
author: string;
}

interface Painting {
category: 'painting';
artist: string;
}

type ArtProps = Book | Painting;

const DecoratedUnionProps = decorate<ArtProps>( // <-- without the type argument, we'd get a compiler error!
class extends React.Component<ArtProps & WithStyles<'root'>> {
render() {
const props = this.props;
return (
<Typography classes={props.classes}>
{props.category === 'book' ? props.author : props.artist}
</Typography>
);
}
},
);

const unionPropElem = <DecoratedUnionProps category="book" author="Twain, Mark" />;

0 comments on commit 0e86314

Please sign in to comment.