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.

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

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

Open
Jack-Works opened this issue Nov 22, 2022 · 2 comments
Open

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

Jack-Works opened this issue Nov 22, 2022 · 2 comments

Comments

@Jack-Works
Copy link

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
Copy link
Author

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
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants