StyleXStyles in typescript library component

GCoboFacePhi commented 3 weeks ago

Describe the issue

I am changing the styles from @stitches/react to StyleX. In a component I am adding a prop which is className to which I am adding the StyleXStyles type, as it comes in the documentation and in the nextjs-example itself. But when I do the build ( I use Tsup ) it is failing me.

Im using v0.8.0

Expected behavior

Correct type

Steps to reproduce

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

type Props = {
  children: React.ReactNode;
  className?: StyleXStyles;
};

export const FlexContainer = ({ children, className }: Props) => {
  return <div {...stylex.props(styles.flex, className)}>
  {children}
</div>;
};

And className prop has this error

Argument of type '(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<...> | undefin...' is not assignable to parameter of type 'StyleXArray<boolean | CompiledStyles | readonly [CompiledStyles, unique symbol] | null | undefined>'.

To avoid this error i have to change the type to this

  className?: StyleXArray<
      | (null | undefined | CompiledStyles)
      | boolean
      | Readonly<[CompiledStyles, InlineStyles]>
    >;

Once the type is changed, I am implementing this component in a nextjs app and I am adding a style to it.

import { FlexContainer } from '@facephi/ui-react';
import stylex from '@stylexjs/stylex';

const style = stylex.create({
  base: {
    color: 'white',
    backgroundColor: {
      default: 'red',
      ':hover': 'blue',
    },
  },
});

export default function FormPage() {
  return (
    <FlexContainer className={style.base}>
      <p>asdasd</p>
    </FlexContainer>
  );
}

but the prop className still fails

Type 'Readonly<{ readonly color: StyleXClassNameFor<"color", "white">; readonly backgroundColor: StyleXClassNameFor<"backgroundColor", "red" | "blue">; }>' is not assignable to type 'StyleXArray<boolean | CompiledStyles | readonly [CompiledStyles, unique symbol]> & string'. Type 'Readonly<{ readonly color: StyleXClassNameFor<"color", "white">; readonly backgroundColor: StyleXClassNameFor<"backgroundColor", "red" | "blue">; }>' is not assignable to type 'Readonly<{ [key: string]: void | StyleXClassName; }> & string'. Type 'Readonly<{ readonly color: StyleXClassNameFor<"color", "white">; readonly backgroundColor: StyleXClassNameFor<"backgroundColor", "red" | "blue">; }>' is not assignable to type 'string'

Test case

No response

Additional comments

No response

nmn commented 3 weeks ago

All your code examples look correct. Could you, by any chance, help me with a small project where you can reproduce this issue?

I tried to verify these types and they all work correctly on my end. There is a high likelihood that the problem is specific to creating a library and using tsup somehow.

Outside of this, I have a PR up with some type improvements which should be merged and released this week. Try version v0.9.0-beta.1 to see if that helps fix your issue at all.

Also, we recommend using @stylexjs/cli or the community SWC plugin instead for NextJS for now. Look at #618 for an example of how to set up StyleX with the CLI for NextJS. But again, since you're trying to build an external package, things may need to be customised.

GCoboFacePhi commented 3 weeks ago

All your code examples look correct. Could you, by any chance, help me with a small project where you can reproduce this issue?

I tried to verify these types and they all work correctly on my end. There is a high likelihood that the problem is specific to creating a library and using tsup somehow.

Outside of this, I have a PR up with some type improvements which should be merged and released this week. Try version v0.9.0-beta.1 to see if that helps fix your issue at all.

Also, we recommend using @stylexjs/cli or the community SWC plugin instead for NextJS for now. Look at #618 for an example of how to set up StyleX with the CLI for NextJS. But again, since you're trying to build an external package, things may need to be customised.

https://github.com/GCoboFacePhi/styleX-tsup-design-system

Here is a quick example.

In packages/components/flexContainer/src/flexContainer/model.ts you have the commented type to avoid build errors.

In packages/components/theme/src/variables/colors.stylex.ts you have the variables that will be exported to all the components that are needed.

In apps/next-js/src/app/page.tsx you have an example of the FlexContainer component extending the styles.

The goal of this example repository is to update the official documentation by adding this example, creating a real component library.

Each component must have own package.json to install it independently.
Use the variables in the components that are needed (for example a Button).
Use the styles through CSS and not in the JS bundle as it is now in the different applications (the one in the example is from NextJS).

nmn commented 3 weeks ago

The first thing I noticed is that you're re-exporting a .stylex.ts file. This is explicitly not allowed. .stylex.ts files must always be imported and used directly.

See https://stylexjs.com/docs/learn/theming/defining-variables/#rules-when-defining-variables.

The next problem that I see is that you're trying to precompile / pre-bundle the @ui/theme package. This is currently not supported. Packages that export files that use StyleX should be exported uncompiled. The "app" is responsible for compiling all StyleX across all your packages.

Even if you transform files, bundling is never going to be supported as we want to only bundle the files that you actually use within your application, and so only the styles from those files should be included.

So, tsup is the wrong tool for this job. You should do a minimal transform to strip TS types only.

nmn commented 3 weeks ago

@GCoboFacePhi I fixed most of the issues but theming is still broken because of yarn behaviour AFAIK. Theming APIs in StyleX depend on file location to work. This works when using NPM and mono-repos because NPM symlinks internal packages. Yarn doesn't do this and the file locations don't end up resolving to the same location. And so while I've fixed the compilation and type issues, defineVars from across package boundaries isn't working yet.

I have already fixed this on the main branch of StyleX where all filePaths are resolved relative to the nearest package.json file if possible. See your test repo for a PR from me.

To answer your original question about the TS types. The root cause of the problem was:

Props & React.AllHTMLAttributes<HTMLDivElement>;

React.AllHTMLAttributes<HTMLDivElement> contains className?: string within while you were using the prop className for StyleXStyles. So, after using &, the resolved type was StyleXStyles | string, which is not a valid value that can be passed into stylex.props and hence the type error.

Using Omit<React.AllHTMLAttributes<HTMLDivElement>, "className"> instead fixed the type error.

nmn commented 3 weeks ago

Update: All the issues have been fixed in v0.9.2. I've updated my PR to your repro repo which should show you how to set up dependencies. tsup is going to be incompatible and should be avoided.

facebook / stylex