Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[typescript] Fix withStyles typing for class components; remove usage as TS decorator #8561

Merged
merged 2 commits into from
Oct 6, 2017
Merged

[typescript] Fix withStyles typing for class components; remove usage as TS decorator #8561

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
45041c6
Fix withStyles typing for class components; remove usage as TS decorator
pelotom Oct 5, 2017
b777094
Remove unnecessary non-null assertion
pelotom Oct 5, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 3 additions & 19 deletions src/styles/withStyles.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,22 +26,6 @@ export type WithStyles<ClassKey extends string = string> = {
export default function withStyles<ClassKey extends string>(
style: StyleRules<ClassKey> | StyleRulesCallback<ClassKey>,
options?: WithStylesOptions
): {
/**
* Decorating a stateless functional component.
*/
<P>(
component: React.StatelessComponent<P & WithStyles<ClassKey>>
): StyledComponent<P, ClassKey>;

/**
* Decorating a class component. This is slightly less type safe than the
* function decoration case, due to current restrictions on TypeScript
* decorators (https://github.com/Microsoft/TypeScript/issues/4881). The
* upshot is that one has to use the non-null assertion operator (`!`) when
* accessing `props.classes`.
*/
<P, C extends React.ComponentClass<P & StyledComponentProps<ClassKey>>>(
component: C
): C;
};
): <P>(
component: React.ComponentType<P & WithStyles<ClassKey>>
) => StyledComponent<P, ClassKey>;
3 changes: 2 additions & 1 deletion test/typescript/components.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ import Table, {
} from '../../src/Table';
import { withStyles, StyleRulesCallback } from '../../src/styles';
import { withResponsiveFullScreen, DialogProps } from '../../src/Dialog';
import { WithStyles } from '../../src/styles/withStyles';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we add this to the exports of material-ui/styles, so people don't have multiple imports? :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WithStyles is already exported in styles no? 🤔

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, sir ... I am going to blame this on Firday morning 😴

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think withStyles is currently exported in material-ui/styles since it keeps coming back as undefined for me.

Copy link
Member Author

@pelotom pelotom Oct 12, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amyasapp these comments were about WithStyles, which is a type... but it certainly seems like withStyles is exported from material-ui/styles.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I'm using v0.19.4 and not v1 beta. Would that change anything? My coworkers and I grep'd for withStyles and couldn't find it anywhere in our node_modules.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amyasapp yeah, I don't think there's a withStyles in v0.19.4.


const log = console.log;
const FakeIcon = () => <div>ICON</div>;
Expand Down Expand Up @@ -717,7 +718,7 @@ const TabsTest = () => {
},
});

class BasicTabs extends React.Component<{ classes: { root: string } }> {
class BasicTabs extends React.Component<WithStyles<'root'|'button'>> {
state = {
value: 0,
};
Expand Down
18 changes: 10 additions & 8 deletions test/typescript/styles.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,15 +120,17 @@ const AllTheStyles: React.SFC<AllTheProps> = ({ theme, classes }) => (

const AllTheComposition = withTheme(withStyles(styles)(AllTheStyles));

@withStyles(styles)
class DecoratedComponent extends React.Component<
ComponentProps & StyledComponentProps<'root'>
> {
render() {
const { classes, text } = this.props;
return <div className={classes!.root}>{text}</div>;
// Can't use withStyles effectively as a decorator in TypeScript
// due to https://github.com/Microsoft/TypeScript/issues/4881
//@withStyles(styles)
const DecoratedComponent = withStyles(styles)(
class extends React.Component<ComponentProps & WithStyles<'root'>> {
render() {
const { classes, text } = this.props;
return <div className={classes.root}>{text}</div>;
}
}
}
);

// no 'classes' property required at element creation time (#8267)
<DecoratedComponent text="foo" />;