garronej / tss-react

✨ Dynamic CSS-in-TS solution, based on Emotion
https://tss-react.dev
MIT License
659 stars 37 forks source link

useStyles classes should be able to extends new class names #128

Open Jack-Works opened 1 year ago

Jack-Works commented 1 year ago

We're upgrading from v3 to v4 and are happy to find the new feature, but we found some problem with TypeScript.

In our old implementation, we call it useStyleExtends and it is implemented in the follow way:

import { useRef } from 'react'
import type { Cx } from 'tss-react'
export type ClassNameMap<ClassKey extends string = string> = { [P in ClassKey]: string }
export function useStylesExtends<Input extends { classes: ClassNameMap; cx: Cx }, PropsOverwriteKeys extends string>(
    defaultStyles: Input,
    props: { classes?: Partial<ClassNameMap<PropsOverwriteKeys>> },
    useConfigHooks?: () => { classes: Partial<ClassNameMap<PropsOverwriteKeys>> },
): Omit<Input, 'classes'> & { classes: Input['classes'] & Partial<ClassNameMap<PropsOverwriteKeys>> } {
    const { current: useConfigOverwriteHook } = useRef(useConfigHooks || (() => ({ classes: {} })))
    const configOverwrite = useConfigOverwriteHook()
    const { classes, cx } = defaultStyles

    const allKeys = new Set([
        ...Object.keys(defaultStyles.classes),
        ...(props.classes ? Object.keys(props.classes) : []),
        ...Object.keys(configOverwrite.classes),
    ])
    const result: Record<string, string> = {}
    for (const key of allKeys) {
        result[key] = cx((classes as any)[key], (props.classes as any)?.[key], (configOverwrite.classes as any)?.[key])
    }
    return { ...defaultStyles, classes: result } as any
}

This implementation allows us to extend the type of classes in the props. This is our pattern of customizable components.

interface withClasses<ClassKeys extends string> {
    classes?: [ClassKeys] extends [never] ? never : Partial<Record<ClassKeys, string>>
}
interface XProps extends withClasses<'className' | 'className2'> {}
const useStyle = makeStyles()({
    selfStyle: { ... }
})
function X(props: XProps) {
    const { classes } = useStylesExtends(useStyle(), props)
    classes.selfStyle // OK
    classes.className // OK
    classes.className2 // OK
}

After migrating to tss-react v4 new API, we found the new API is not sufficient to replace our old one:

function X(props: XProps) {
    const { classes } = useStyle(undefined, { props })
    //                                        ~~~~~
    // Type 'XProps' is not assignable to type '{ classes?: Record<string, string> | undefined; } & Record<string, unknown>'.
    //     Type 'XProps' is not assignable to type 'Record<string, unknown>'.
    //         Index signature for type 'string' is missing in type 'XProps'.ts(2322)
    classes.selfStyle // OK
    classes.className // Type err!
    classes.className2 // Type err!
}
Jack-Works commented 1 year ago

We temporally cast makeStyles as the following type, it works well.

{
    makeStyles: <Params = void, RuleNameSubsetReferencableInNestedSelectors extends string = never>(params?: {
        name?: string | Record<string, unknown>
        uniqId?: string
    }) => <RuleName extends string>(
        cssObjectByRuleNameOrGetCssObjectByRuleName:
            | Record<RuleName, CSSObject>
            | ((
                  theme: Theme,
                  params: Params,
                  classes: Record<RuleNameSubsetReferencableInNestedSelectors, string>,
              ) => Record<RuleNameSubsetReferencableInNestedSelectors | RuleName, CSSObject>),
    ) => <StyleOverrides extends { classes?: { [key in string]?: string | undefined } }>(
        params: Params,
        styleOverrides?: {
            props: StyleOverrides
            ownerState?: Record<string, unknown>
        },
    ) => {
        classes: StyleOverrides extends { classes?: { [key in infer ExtraKeys]?: string | undefined } }
            ? Record<ExtraKeys | RuleName, string>
            : Record<RuleName, string>
        theme: Theme
        css: Css
        cx: Cx
    }
}
garronej commented 1 year ago

Hey @Jack-Works,

Thank you for the feedback! I understand your usecase, it makes sences.
What I do currently if I something like this but I get this is not optimal.

import { useMergedClasses } from "tss-react";

type Props = {
    //classes?: { foo?: string; bar?: string; };
    classes?: Omit<Partial<ReturnType<typeof useStyles>["classes"]>, "onlyInternal">;
};

function MyTestComponentForMergedClassesInternal(props: Props) {
    const { classes } = useStyles({ "color": "pink" }, { props });
    //NOTE: Only the classes will be read from props, 
    //you could write { props: { classes: props.classes } } instead of { props }
    //and it would work the same. 

    return (
        <div className={classes.foo}>
            <span className={classes.somethingForOverwriting}>
            </span>
            <span className={classes.onlyIntenal}>
            </span>   
        </div>
    );
}

const useStyles = makeStyles<{ color: string; }>()({
    "foo": {
        "border": "3px dotted black",
        "backgroundColor": "red"
    }
    "somethingForOverwriting": {},
    "onlyInternal": {
         "color": "red"
    }
});

I'll think about it. Glad you found a workaround for now.

Best regards