Typing mismatch for fontWeight property (maybe others?) about stylex HOT 11 CLOSED

@timwehrle The nested object case is handled elsewhere. The fix would be as simple as:

type fontWeight =
  | 'inherit'
  | 'normal'
  | 'bold'
  | 'bolder'
  | 'lighter'
  | 100
  | 200
  | 300
  | 400
  | 500
  | 600
  | 700
  | 800
  | 900
  | string
  | number;

@purohitdheeraj The race is on!

arbitrary number would not satisfy type def of TTokens as it only accepts string

Yeah, as explained we cannot accept raw numbers as variable values. It is error prone because you may expect some number to be auto-converted to px values which won't happen.

The types function will let you do something like this, which should solve it:

const vars = stylex.defineVars({
  heavy: stylex.types.number(700),
  red500: stylex.types.color('tomato'),
  size_lg: stylex.types.length('64px'),


  // And I'm thinking about the following patterns:
  size_sm: stylex.types.rem(2),
  size_md: stylex.types.px(36),

  // Further, I want something like this, but it needs to be immutable/unthemeable:
  media_mobile: stylex.types.media('(max-width: 480px)'),
});

I'll write a full proposal with details about the types function, but know that it will end up generating @property --varname declarations which will give your variables types even in CSS. This can be useful for a bunch of things including being able to animate CSS variable values.

I'm also working on a full CSS parser which will be used to validate the values being passed to the type function.

from stylex.

@kevintyj There are some types functions that I'm working on that will make some of this better. We don't accept number values for tokens yet because while StyleX will usually convert numbers to px values based on the property name, variables don't have a type.

In the meantime we have two options:

1. Add string equivalents for all the number values: '100' | '200' | .... This is safe and would compile to exact same thing.
2. Simply add string | number at the end of the list of types. This would allow arbitrary number values that are not multiples of 100.

Considering the increasing use of variable fonts, I think option 2 is good for this.

@purohitdheeraj You are welcome to put up this PR, but be prepared for some discussion about the better option mentioned above.

from stylex.

StyleXCSSTypes.d.ts

It should work? I just tested it out and was able to make a commit to that file. Perhaps check your local gitignore config?

git config --get core.excludesfile

EDIT I also dont think thats the file you should edit, it should be the StyleXCSSTypes.js file

from stylex.

@nmn

Considering the increasing use of variable fonts, I think option 2 is good for this.

I agree that option 2 would be a better temporary fix until the new type function is adopted! The only downside being that arbitrary number would not satisfy type def of TTokens as it only accepts string

from stylex.

@purohitdheeraj Please edit the .js file in src/. StyleX is authored in Flow and the TS types are generated from that. You're looking for StyleXCSSTypes.js.

After that, running npm run build will update the auto-generated StyleXCSSTypes.d.ts with the new type.

NOTE: StyleXTypes.d.ts (not StyleXCSSTypes.d.ts) is the only typescript file in the Git repo. It is needed for workarounds for types that can't be converted from Flow to Typescript. Everything else is generated.

from stylex.

This is a bug in our type definition. Your usage is correct. Will fix.

from stylex.

@nmn can i take this please,
we only need replacement of that right ? from integer to string
or can add additional string type

from stylex.

type fontWeight should also support of type number (variable fonts) but I feel that for tokens, it should accept number values (other than just strings) and then from the create function reject using StyleXCSSTypes
ie: making a token for padding and spacing values as numbers

from stylex.

@kevintyj You‘re right but I guess this approach in the following code could work. And if I understand this right, this allows strings “400”, “bolder” and numeric values like 400? But I guess it’s a bit more complicated than that. 😄

// /packages/stylex/src/StyleXTypes.js

type TTokens = $ReadOnly<{
  [string]:
    | number
    | string
    | $ReadOnly<{ default: number | string, [string]: number | string }>,
}>;

from stylex.

hello everyone, this question might be silly

i made the changes in StyleXCSSTypes.d.ts fontWeight type
but its not getting saved and not identified by git
its ignored by gitlens

what should be the next step

• do i need to run test or build , its package file

even if someone has already solved it
please guide me how make changes in such package files

from stylex.

Thank you so much @nmn and @kevintyj
just raised a PR for it

from stylex.