From d345232bc226a0bb3a3738005619ae9384ad4c67 Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann <silbermann.sebastian@gmail.com> Date: Fri, 17 Jan 2020 11:01:35 +0100 Subject: [PATCH 1/2] [core] Distinguish JSSProperties and CSSProperties --- .../components/DynamicInlineStyle.tsx | 5 ++--- .../src/withStyles/withStyles.d.ts | 22 ++++++++++++++----- .../material-ui-styles/test/styles.spec.tsx | 6 ++--- .../material-ui/src/styles/createMixins.d.ts | 6 ++--- .../src/styles/createTypography.d.ts | 13 ++++++----- 5 files changed, 32 insertions(+), 20 deletions(-) diff --git a/docs/src/pages/customization/components/DynamicInlineStyle.tsx b/docs/src/pages/customization/components/DynamicInlineStyle.tsx index 06a8a5c8b4ea46..11478d48c4dbad 100644 --- a/docs/src/pages/customization/components/DynamicInlineStyle.tsx +++ b/docs/src/pages/customization/components/DynamicInlineStyle.tsx @@ -2,9 +2,8 @@ import React from 'react'; import Button from '@material-ui/core/Button'; import FormControlLabel from '@material-ui/core/FormControlLabel'; import Switch from '@material-ui/core/Switch'; -import { createStyles } from '@material-ui/core/styles'; -const styles = createStyles({ +const styles: Record<'button' | 'buttonBlue', React.CSSProperties> = { button: { background: 'linear-gradient(45deg, #FE6B8B 30%, #FF8E53 90%)', borderRadius: 3, @@ -18,7 +17,7 @@ const styles = createStyles({ background: 'linear-gradient(45deg, #2196f3 30%, #21cbf3 90%)', boxShadow: '0 3px 5px 2px rgba(33, 203, 243, .30)', }, -}); +}; export default function DynamicInlineStyle() { const [color, setColor] = React.useState('default'); diff --git a/packages/material-ui-styles/src/withStyles/withStyles.d.ts b/packages/material-ui-styles/src/withStyles/withStyles.d.ts index b3610bd3857969..7729d3942952f6 100644 --- a/packages/material-ui-styles/src/withStyles/withStyles.d.ts +++ b/packages/material-ui-styles/src/withStyles/withStyles.d.ts @@ -1,5 +1,5 @@ import * as React from 'react'; -import { PropInjector, IsEmptyInterface } from '@material-ui/types'; +import { PropInjector } from '@material-ui/types'; import * as CSS from 'csstype'; import * as JSS from 'jss'; import { DefaultTheme } from '../defaultTheme'; @@ -18,7 +18,14 @@ export interface BaseCSSProperties extends CSS.Properties<number | string> { export interface CSSProperties extends BaseCSSProperties { // Allow pseudo selectors and media queries - [k: string]: BaseCSSProperties[keyof BaseCSSProperties] | CSSProperties; + // `unknown` is used since TS does not allow assigning an interface without + // an index signature to one with an index signature. This is to allow type safe + // module augmentation. + // Technically we want any key not typed in `BaseCSSProperties` to be of type + // `CSSProperties` but this doesn't work. The index signature needs to cover + // BaseCSSProperties as well. Usually you would use `BaseCSSProperties[keyof BaseCSSProperties]` + // but this would not allow assigning React.CSSProperties to CSSProperties + [k: string]: unknown | CSSProperties; } export type BaseCreateCSSProperties<Props extends object = {}> = { @@ -43,9 +50,14 @@ export interface CreateCSSProperties<Props extends object = {}> */ export type StyleRules<Props extends object = {}, ClassKey extends string = string> = Record< ClassKey, - IsEmptyInterface<Props> extends true - ? CSSProperties | (() => CSSProperties) - : CreateCSSProperties<Props> | ((props: Props) => CreateCSSProperties<Props>) + // JSS property bag + | CSSProperties + // JSS property bag based on a theme + | (() => CSSProperties) + // JSS property bag where values are based on props + | CreateCSSProperties<Props> + // JSS property bag based on props + | ((props: Props) => CreateCSSProperties<Props>) >; /** diff --git a/packages/material-ui-styles/test/styles.spec.tsx b/packages/material-ui-styles/test/styles.spec.tsx index 122883b97a7b6e..3a8794d2ce2eb7 100644 --- a/packages/material-ui-styles/test/styles.spec.tsx +++ b/packages/material-ui-styles/test/styles.spec.tsx @@ -455,14 +455,14 @@ function forwardRefTest() { // If there are no props, use the definition that doesn't accept them // https://github.com/mui-org/material-ui/issues/16198 - // $ExpectType Record<"root", CSSProperties | (() => CSSProperties)> + // $ExpectType Record<"root", CSSProperties | (() => CSSProperties) | CreateCSSProperties<{}> | ((props: {}) => CreateCSSProperties<{}>)> const styles = createStyles({ root: { width: 1, }, }); - // $ExpectType Record<"root", CSSProperties | (() => CSSProperties)> + // $ExpectType Record<"root", CSSProperties | (() => CSSProperties) | CreateCSSProperties<{}> | ((props: {}) => CreateCSSProperties<{}>)> const styles2 = createStyles({ root: () => ({ width: 1, @@ -473,7 +473,7 @@ function forwardRefTest() { foo: boolean; } - // $ExpectType Record<"root", CreateCSSProperties<testProps> | ((props: testProps) => CreateCSSProperties<testProps>)> + // $ExpectType Record<"root", CSSProperties | (() => CSSProperties) | CreateCSSProperties<testProps> | ((props: testProps) => CreateCSSProperties<testProps>)> const styles3 = createStyles({ root: (props: testProps) => ({ width: 1, diff --git a/packages/material-ui/src/styles/createMixins.d.ts b/packages/material-ui/src/styles/createMixins.d.ts index cbf7aced080d69..2b22e567e12577 100644 --- a/packages/material-ui/src/styles/createMixins.d.ts +++ b/packages/material-ui/src/styles/createMixins.d.ts @@ -1,10 +1,10 @@ import { Breakpoints } from './createBreakpoints'; import { Spacing } from './createSpacing'; -import { CSSProperties } from './withStyles'; +import * as React from 'react'; export interface Mixins { - gutters: (styles?: CSSProperties) => CSSProperties; - toolbar: CSSProperties; + gutters: (styles?: React.CSSProperties) => React.CSSProperties; + toolbar: React.CSSProperties; // ... use interface declaration merging to add custom mixins } diff --git a/packages/material-ui/src/styles/createTypography.d.ts b/packages/material-ui/src/styles/createTypography.d.ts index 96191e64d841f4..8bdcd0da96c225 100644 --- a/packages/material-ui/src/styles/createTypography.d.ts +++ b/packages/material-ui/src/styles/createTypography.d.ts @@ -1,4 +1,5 @@ import { Palette } from './createPalette'; +import * as React from 'react'; import { CSSProperties } from './withStyles'; export type Variant = @@ -18,17 +19,17 @@ export type Variant = export interface FontStyle extends Required<{ - fontFamily: CSSProperties['fontFamily']; + fontFamily: React.CSSProperties['fontFamily']; fontSize: number; - fontWeightLight: CSSProperties['fontWeight']; - fontWeightRegular: CSSProperties['fontWeight']; - fontWeightMedium: CSSProperties['fontWeight']; - fontWeightBold: CSSProperties['fontWeight']; + fontWeightLight: React.CSSProperties['fontWeight']; + fontWeightRegular: React.CSSProperties['fontWeight']; + fontWeightMedium: React.CSSProperties['fontWeight']; + fontWeightBold: React.CSSProperties['fontWeight']; }> {} export interface FontStyleOptions extends Partial<FontStyle> { htmlFontSize?: number; - allVariants?: CSSProperties; + allVariants?: React.CSSProperties; } // TODO: which one should actually be allowed to be subject to module augmentation? From 78f5ec8fff38a6a72fb4f56049b891910f808c76 Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann <silbermann.sebastian@gmail.com> Date: Mon, 20 Jan 2020 12:16:54 +0100 Subject: [PATCH 2/2] Simplify CSSProperties --- packages/material-ui-styles/src/withStyles/withStyles.d.ts | 2 -- packages/material-ui-styles/test/styles.spec.tsx | 6 +++--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/material-ui-styles/src/withStyles/withStyles.d.ts b/packages/material-ui-styles/src/withStyles/withStyles.d.ts index 7729d3942952f6..3bd02d7900e150 100644 --- a/packages/material-ui-styles/src/withStyles/withStyles.d.ts +++ b/packages/material-ui-styles/src/withStyles/withStyles.d.ts @@ -52,8 +52,6 @@ export type StyleRules<Props extends object = {}, ClassKey extends string = stri ClassKey, // JSS property bag | CSSProperties - // JSS property bag based on a theme - | (() => CSSProperties) // JSS property bag where values are based on props | CreateCSSProperties<Props> // JSS property bag based on props diff --git a/packages/material-ui-styles/test/styles.spec.tsx b/packages/material-ui-styles/test/styles.spec.tsx index 3a8794d2ce2eb7..9b6f271afdf4e5 100644 --- a/packages/material-ui-styles/test/styles.spec.tsx +++ b/packages/material-ui-styles/test/styles.spec.tsx @@ -455,14 +455,14 @@ function forwardRefTest() { // If there are no props, use the definition that doesn't accept them // https://github.com/mui-org/material-ui/issues/16198 - // $ExpectType Record<"root", CSSProperties | (() => CSSProperties) | CreateCSSProperties<{}> | ((props: {}) => CreateCSSProperties<{}>)> + // $ExpectType Record<"root", CSSProperties | CreateCSSProperties<{}> | ((props: {}) => CreateCSSProperties<{}>)> const styles = createStyles({ root: { width: 1, }, }); - // $ExpectType Record<"root", CSSProperties | (() => CSSProperties) | CreateCSSProperties<{}> | ((props: {}) => CreateCSSProperties<{}>)> + // $ExpectType Record<"root", CSSProperties | CreateCSSProperties<{}> | ((props: {}) => CreateCSSProperties<{}>)> const styles2 = createStyles({ root: () => ({ width: 1, @@ -473,7 +473,7 @@ function forwardRefTest() { foo: boolean; } - // $ExpectType Record<"root", CSSProperties | (() => CSSProperties) | CreateCSSProperties<testProps> | ((props: testProps) => CreateCSSProperties<testProps>)> + // $ExpectType Record<"root", CSSProperties | CreateCSSProperties<testProps> | ((props: testProps) => CreateCSSProperties<testProps>)> const styles3 = createStyles({ root: (props: testProps) => ({ width: 1,