facebook / stylex

StyleX is the styling system for ambitious user interfaces.
https://stylexjs.com
MIT License
8.41k stars 310 forks source link

0.4.1: The inferred type of this node exceeds the maximum length the compiler will serialize. #336

Closed olivierpascal closed 10 months ago

olivierpascal commented 10 months ago

The problem

The inferred type of this node exceeds the maximum length the compiler will serialize.
An explicit type annotation is needed. ts(7056)

How to reproduce

// bug.ts

import type { UserAuthoredStyles } from '@stylexjs/stylex/lib/StyleXTypes';
import * as stylex from '@stylexjs/stylex';

type StyleKey = 'a' | 'b';

//           vvvvvv
export const styles = stylex.create<{
  [key in StyleKey]: UserAuthoredStyles;
}>({
  a: { margin: '0' },
  b: { padding: '0' },
});
nmn commented 10 months ago

Why are you adding the explicit type annotation? It's also the wrong type annotation.

You should never import UserAuthoredStyles. It's not a public type.

And let stylex.create infer the types. StyleX requires styles to be defined statically in the same file, this makes them inferable as well. For extra safety, you can use as const for any local constant you use for your styles.

This will generate the correct and most accurate type:

import * as stylex from '@stylexjs/stylex';

export const styles = stylex.create({
  a: { margin: 0 },
  b: { padding: 0 },
});
olivierpascal commented 10 months ago

Thanks for this answer.

Why are you adding the explicit type annotation and it's the wrong type annotation.

Tell me if I'm wrong, the original type annotation is:

export type Stylex$Create = <
  const S extends {
    [key: string]: UserAuthoredStyles | ((...args: any) => UserAuthoredStyles);
  },
>(
  styles: S,
) => MapNamespaces<S>;

About the 'why', this theme can be provided externally by a lib user. So I want to add type safety so that the user cannot define non-allowed class keys, also to add autocompletion on allowed class keys.

I don't want this to be allowed:

export const styles = stylex.create({
  c: { margin: 0 },
});
olivierpascal commented 10 months ago

You should never import UserAuthoredStyles. It's not a public type.

@nmn Mm I need this. My workaround is so dirty and I'm so ashamed of it now, that you would definitely kick ban me eternally from this repo if I share it here.

nmn commented 10 months ago

@olivierpascal You should be doing this then:

type StyleObj = {[key: 'a' | 'b']: unknown};
export const styles = stylex.create({
  c: { margin: 0 },
} satisfies StyleObj);

This will ensure that the keys are one of the keys you wanted and nothing else. You can make the type more specific as needed as well.

I'm not sure this will solve the auto-complete case for now, but that won't be possible with another Babel plugin that I'm working on.

Your type above effectively erases the const S extends part of the generic part of StyleX$Create. This makes typescript treat the generated style as a union of all possible styles and not what it actually is.

olivierpascal commented 10 months ago

@nmn I see two major issues:

I could do:

type StyleObj = {[key: 'a' | 'b']: unknown};
export const styles = stylex.create({
  a: { anything: 'does not matter' },
} satisfies StyleObj);
nmn commented 10 months ago

eslint will complain that 'Styles must be represented as JavaScript objects' (bug?)

Yeah, this is a bug.

no auto-complete for sure, and no type safe for sure for css properties

It can be more type safe, if you define a more complete type for StyleObj.

But what you should be doing is configuring the ESLint plugin to do this for you. You can constrain what values are allowed for various CSS properties with the ESLint rule.

olivierpascal commented 10 months ago

All I would like to do is:

type StyleObj = {[key: 'a' | 'b']: AnyValidCssProperty};
nmn commented 10 months ago

If you use that for a satisfies clause, that should be fine.

olivierpascal commented 10 months ago

Yes, but:

You should never import UserAuthoredStyles. It's not a public type.

Is it any alternative way to do so?

nmn commented 10 months ago

You could try import type { CSSProperties } from '.../StyleXCSSTypes'; instead. You'd need to extend this type to add all the pseudo elements you want to support (::after etc.) See CSSPropertiesWithExtras as an example.

type UserAuthoredStyles = CSSPropertiesWithExtras | { [key: string]: unknown };

UserAuthoredStyles doesn't offer any type safety as it allows everything. And stylex.create should be giving you the auto-complete anyway.


Mm I need this. My workaround is so dirty and I'm so ashamed of it now

What are you trying to achieve? This whole pattern feels unnecessary. You should never need to constrain what keys are being used with stylex.create. After all, that's like disallowing what your variable names can be.

I feel like you're trying to do something that can achieved with a much simpler pattern.