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

[not a bug] What is the correct way to type a CSS property, as React.CSSProperties is not #659

Open
olivierpascal opened this issue Aug 8, 2024 · 6 comments
Labels
bug Something isn't working

Comments

@olivierpascal
Copy link

olivierpascal commented Aug 8, 2024

Describe the issue

Typescript is not happy when using certain properties with dynamic styles.

Expected behavior

No type error.

Steps to reproduce

import stylex, { type StyleXStyles } from '@stylexjs/stylex';

type Props = {
  style: StyleXStyles;
};

export const MyDiv: React.FC<Props> = ({ style }) => (
  <div {...stylex.props(style)} />
);

////////////////////////////////////////////////////////////////////////////////

const myStyles = stylex.create({
  myWrap: (value: React.CSSProperties['flexWrap']) => ({
    flexWrap: value,
  }),
});

export const MyComponent: React.FC = () => (
  <MyDiv style={myStyles.myWrap('wrap')} />
  //     ^^^^^ ts error here
);

Error:

Type 'readonly [Readonly<{ flexWrap: StyleXClassNameFor<"flexWrap", FlexWrap | undefined>; }>, unique symbol]' is not assignable to type 'StyleXStyles'.
  Type 'readonly [Readonly<{ flexWrap: StyleXClassNameFor<"flexWrap", FlexWrap | undefined>; }>, unique symbol]' is not assignable to type 'readonly [Readonly<Readonly<{ readonly theme?: StyleXClassNameFor<"theme", Readonly<string | null | undefined>> | undefined; readonly MozOsxFontSmoothing?: StyleXClassNameFor<"MozOsxFontSmoothing", Readonly<...>> | undefined; ... 527 more ...; readonly '::-webkit-search-results-decoration'?: StyleXClassNameFor<...> ...'.
    Type 'readonly [Readonly<{ flexWrap: StyleXClassNameFor<"flexWrap", FlexWrap | undefined>; }>, unique symbol]' is not assignable to type 'readonly [Readonly<Readonly<{ readonly theme?: StyleXClassNameFor<"theme", Readonly<string | null | undefined>> | undefined; readonly MozOsxFontSmoothing?: StyleXClassNameFor<"MozOsxFontSmoothing", Readonly<...>> | undefined; ... 527 more ...; readonly '::-webkit-search-results-decoration'?: StyleXClassNameFor<...> ...'.
      Type at position 0 in source is not compatible with type at position 0 in target.
        The types of 'flexWrap' are incompatible between these types.
          Type 'StyleXClassNameFor<"flexWrap", FlexWrap | undefined>' is not assignable to type 'StyleXClassNameFor<"flexWrap", Readonly<"initial" | "inherit" | "unset" | "nowrap" | "wrap" | "wrap-reverse" | null | undefined>> | undefined'.
            Type 'StyleXClassNameFor<"flexWrap", FlexWrap | undefined>' is not assignable to type 'StyleXClassNameFor<"flexWrap", Readonly<"initial" | "inherit" | "unset" | "nowrap" | "wrap" | "wrap-reverse" | null | undefined>>'.
              Type 'StyleXClassNameFor<"flexWrap", FlexWrap | undefined>' is not assignable to type '{ _opaque: unique symbol; _key: "flexWrap"; _value: Readonly<"initial" | "inherit" | "unset" | "nowrap" | "wrap" | "wrap-reverse" | null | undefined>; }'.ts(2322)
Test.tsx(4, 3): The expected type comes from property 'style' which is declared here on type 'IntrinsicAttributes & Props'
@olivierpascal olivierpascal added the bug Something isn't working label Aug 8, 2024
@olivierpascal
Copy link
Author

I realize that it's not really a bug and that StyleX use a different type than React.CSSProperties['flexWrap'].

So I am wondering what is the correct way to type CSS properties with StyleX.

@olivierpascal olivierpascal changed the title Type error for certain properties with dynamic styles [not a bug] What is the correct way to type a CSS property, as React.CSSProperties is not Aug 8, 2024
@nmn
Copy link
Contributor

nmn commented Sep 9, 2024

The type are intentionally not public because dynamic styles should only be used for numbers or colors as those are the only values that are truly dynamic.

You should not be using dynamic styles for values that are essentially enums.


Stay tuned for more type improvements soon.

@nmn nmn closed this as completed Sep 9, 2024
@necolas
Copy link
Contributor

necolas commented Sep 9, 2024

dynamic styles should only be used for numbers or colors as those are the only values that are truly dynamic

Unfortunately this assumption is not true. Anything that accepts a length, angle, etc., can also be dynamic

@nmn
Copy link
Contributor

nmn commented Sep 24, 2024

Anything that accepts a length, angle, etc., can also be dynamic

Correct. I should've used the word "numeric". Any value that contains numbers are good candidates to be set with dynamic styles.

@nmn
Copy link
Contributor

nmn commented Sep 24, 2024

@olivierpascal While I look for a better solution that doesn't need importing more types, the solution in the meantime is to use the CSSProperties type from StyleX itself.

This should work. The intention is that you should never have to import this CSSProperties type directly, but there is currently no good solution when defining the argument type of a dynamic style function.

import stylex, { type StyleXStyles } from '@stylexjs/stylex';
import type {CSSProperties} from '@stylexjs/stylex/lib/StyleXCSSTypes.d.ts'

type Props = {
  style: StyleXStyles;
};

export const MyDiv: React.FC<Props> = ({ style }) => (
  <div {...stylex.props(style)} />
);

////////////////////////////////////////////////////////////////////////////////

const myStyles = stylex.create({
  myWrap: (value: CSSProperties['flexWrap']) => ({
    flexWrap: value,
  }),
});

export const MyComponent: React.FC = () => (
  <MyDiv style={myStyles.myWrap('wrap')} />
);

@necolas necolas reopened this Oct 10, 2024
@necolas
Copy link
Contributor

necolas commented Oct 10, 2024

Reopening as we're working on a related solution

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants