[core] Distinguish JSSProperties and CSSProperties

mui · Jan 19, 2020 · d345232 · d345232
1 parent f1080b6
commit d345232
Show file tree

Hide file tree

Showing 5 changed files with 32 additions and 20 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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?