Skip to content

Commit

Permalink
Improve Types for Hooks (#1460)
Browse files Browse the repository at this point in the history
This commit applies changes to ensure the `createUseStyles` hook has
accurate TS intellisense for props and themes. The details of the work
may be found below.

Note: Changes are for TypeScript files **only**. Minimal changes were
applied in order to minimize regression testing and the potential for
cascading negative effects.

General:
* Updated .eslintignore to ignore TSX type tests in addition to TS type
  tests.

JSS:
* `Style` now expects type parameters for `Props` and `Theme`.
* `JssStyle` now expects type parameters for `Props` and `Theme`.
* `Func` now expects type parameters for `Props` and `Theme`.
* Types are arranged to prevent unnecessary/confusing parameter
  shadowing. Once a function in a style object is introduced, if the
  function returns an object, none of the returned object's properties
  (or nested properties) may define a function that has access to
  `Props` or `Theme`.
* Updated tests for `Styles` type.
* Where necessary for the compiler, updated other types (mainly for
  `createStyleSheet` and `StyleSheet`.
* Includes minor automated changes.

React-JSS:
* `createUseStyles` now expects type parameters for `Props` and `Theme`.
* `data` in `useStyles` now expects the proper typed argument (`Props`
  with an optional `Theme`.)
* Types are arranged to prevent unnecessary/confusing parameter
  shadowing. If the argument to `createUseStyles` is a function of
  `Theme`, then no properties (or nested properties) in the object that
  the function returns are permitted to define functions with access to
  `Theme`.
* Updated types for `withStyles` (and related types) to be compatible
  with new changes.
* Added several typed tests for `createUseStyles` and `withStyles` to
  ensure that everything behaves as expected.
  • Loading branch information
ITenthusiasm authored Mar 11, 2021
1 parent b0510be commit ea195bf
Show file tree
Hide file tree
Showing 6 changed files with 481 additions and 31 deletions.
1 change: 1 addition & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ flow-typed/
packages/**/dist/
examples/**/static/
*.ts
*.tsx
30 changes: 18 additions & 12 deletions packages/jss/src/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,33 +7,36 @@ import {Properties as CSSProperties} from 'csstype'
// TODO: refactor to only include Observable types if plugin is installed.
import {Observable} from 'indefinite-observable'

// TODO: Type data better, currently typed as any for allowing to override it
type Func<R> = ((data: any) => R)
type Func<P, T, R> = T extends undefined ? ((data: P) => R) : ((data: P & {theme: T}) => R)

type NormalCssProperties = CSSProperties<string | number>
type NormalCssValues<K> = K extends keyof NormalCssProperties ? NormalCssProperties[K] : JssValue

export type JssStyle =
export type JssStyle<Props = any, Theme = undefined> =
| {
[K in keyof NormalCssProperties]:
| NormalCssValues<K>
| JssStyle
| Func<NormalCssValues<K> | JssStyle | undefined>
| JssStyle<Props, Theme>
| Func<Props, Theme, NormalCssValues<K> | JssStyle<undefined, undefined> | undefined>
| Observable<NormalCssValues<K> | JssStyle | undefined>
}
| {
[K: string]:
| JssValue
| JssStyle
| Func<JssValue | JssStyle | undefined>
| JssStyle<Props, Theme>
| Func<Props, Theme, JssValue | JssStyle<undefined, undefined> | undefined>
| Observable<JssValue | JssStyle | undefined>
}

export type Styles<Name extends string | number | symbol = string> = Record<
export type Styles<
Name extends string | number | symbol = string,
Props = unknown,
Theme = undefined
> = Record<
Name,
| JssStyle
| JssStyle<Props, Theme>
| string
| Func<JssStyle | string | null | undefined>
| Func<Props, Theme, JssStyle<undefined, undefined> | string | null | undefined>
| Observable<JssStyle | string | null | undefined>
>
export type Classes<Name extends string | number | symbol = string> = Record<Name, string>
Expand Down Expand Up @@ -213,7 +216,10 @@ export interface StyleSheet<RuleName extends string | number | symbol = string |
* Create and add rules.
* Will render also after Style Sheet was rendered the first time.
*/
addRules(styles: Partial<Styles<RuleName>>, options?: Partial<RuleOptions>): Rule[]
addRules(
styles: Partial<Styles<RuleName, any, undefined>>,
options?: Partial<RuleOptions>
): Rule[]
/**
* Get a rule by name.
*/
Expand Down Expand Up @@ -248,7 +254,7 @@ export interface JssOptions {

export interface Jss {
createStyleSheet<Name extends string | number | symbol>(
styles: Partial<Styles<Name>>,
styles: Partial<Styles<Name, any, undefined>>,
options?: StyleSheetFactoryOptions
): StyleSheet<Name>
removeStyleSheet(sheet: StyleSheet): this
Expand Down
42 changes: 37 additions & 5 deletions packages/jss/tests/types/Styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,18 @@ interface Props {
flag?: boolean
}

interface Theme {
color: string
}

declare const color$: Observable<'cyan'>
declare const style$: Observable<{
backgroundColor: 'fuchsia'
transform: 'translate(0px, 205px)'
}>

const styles: Styles = {
// General Types Check
const styles: Styles<string, Props> = {
basic: {
textAlign: 'center',
display: 'flex',
Expand All @@ -22,7 +27,7 @@ const styles: Styles = {
textAlign: 'center',
display: 'flex',
width: '100%',
justifyContent: (props: Props) => (props.flag ? 'center' : undefined)
justifyContent: props => (props.flag ? 'center' : undefined)
},
inner: {
textAlign: 'center',
Expand All @@ -33,7 +38,7 @@ const styles: Styles = {
fontSize: 12
}
},
func: (props: Props) => ({
func: props => ({
display: 'flex',
flexDirection: 'column',
justifyContent: 'center',
Expand All @@ -43,8 +48,8 @@ const styles: Styles = {
position: 'relative',
pointerEvents: props.flag ? 'none' : null
}),
funcNull: (props: Props) => null,
funcWithTerm: (props: Props) => ({
funcNull: props => null,
funcWithTerm: props => ({
width: props.flag ? 377 : 272,
height: props.flag ? 330 : 400,
boxShadow: '0px 2px 20px rgba(0, 0, 0, 0.08)',
Expand All @@ -67,3 +72,30 @@ const styles: Styles = {
to: {opacity: 1}
}
}

// Test supplied Props and Theme
// Verify that nested parameter declarations are banned
const stylesPropsAndTheme: Styles<string, Props, Theme> = {
rootParamDeclaration: ({flag, theme}) => ({
fontWeight: 'bold',
// @ts-expect-error
nothingAllowed: ({flag, theme}) => ''
}),
anotherClass: {
color: 'red',
innerParamDeclaration1: ({flag, theme}) => '',
innerParamDeclaration2: ({flag, theme}) => ({
backgroundColor: 'blue',
// @ts-expect-error
nothingAllowed: ({flag, theme}) => ''
})
}
}

// Test the className types
const stylesClassNames: Styles<number, unknown, unknown> = {
// @ts-expect-error
stringClassName: '',
[1]: '',
[2]: ''
}
31 changes: 17 additions & 14 deletions packages/react-jss/src/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,17 +37,21 @@ declare const JssContext: Context<{
disableStylesGeneration: boolean
}>

type ClassesForStyles<S extends Styles | ((theme: any) => Styles)> = Classes<
S extends (theme: any) => Styles ? keyof ReturnType<S> : keyof S
>
type ClassesForStyles<
S extends Styles<any, any, any> | ((theme: any) => Styles<any, any, undefined>)
> = Classes<S extends (theme: any) => Styles<any, any, undefined> ? keyof ReturnType<S> : keyof S>

interface WithStylesProps<S extends Styles | ((theme: any) => Styles)> {
interface WithStylesProps<
S extends Styles<any, any, any> | ((theme: any) => Styles<any, any, undefined>)
> {
classes: ClassesForStyles<S>
}
/**
* @deprecated Please use `WithStylesProps` instead
*/
type WithStyles<S extends Styles | ((theme: any) => Styles)> = WithStylesProps<S>
type WithStyles<
S extends Styles<any, any, any> | ((theme: any) => Styles<any, any, undefined>)
> = WithStylesProps<S>

declare global {
namespace Jss {
Expand All @@ -72,26 +76,25 @@ interface CreateUseStylesOptions<Theme = DefaultTheme> extends BaseOptions<Theme
name?: string
}

declare function createUseStyles<Theme = DefaultTheme, C extends string = string>(
styles: Styles<C> | ((theme: Theme) => Styles<C>),
declare function createUseStyles<C extends string = string, Props = unknown, Theme = DefaultTheme>(
styles: Styles<C, Props, Theme> | ((theme: Theme) => Styles<C, Props, undefined>),
options?: CreateUseStylesOptions<Theme>
): (data?: unknown) => Classes<C>
): (data?: Props & {theme?: Theme}) => Classes<C>

type GetProps<C> = C extends ComponentType<infer P> ? P : never

declare function withStyles<
ClassNames extends string | number | symbol,
S extends Styles<ClassNames> | ((theme: any) => Styles<ClassNames>)
>(
styles: S,
declare function withStyles<ClassNames extends string | number | symbol, Props, Theme>(
styles:
| Styles<ClassNames, Props, Theme>
| ((theme: Theme) => Styles<ClassNames, Props, undefined>),
options?: WithStylesOptions
): <C>(
comp: C
) => ComponentType<
JSX.LibraryManagedAttributes<
C,
Omit<GetProps<C>, 'classes'> & {
classes?: Partial<ClassesForStyles<S>>
classes?: Partial<ClassesForStyles<typeof styles>>
innerRef?: RefObject<any> | ((instance: any) => void)
}
>
Expand Down
Loading

0 comments on commit ea195bf

Please sign in to comment.