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

Types don't work on on createUseStyles #1439

Closed
goleary opened this issue Jan 7, 2021 · 8 comments · Fixed by #1454
Closed

Types don't work on on createUseStyles #1439

goleary opened this issue Jan 7, 2021 · 8 comments · Fixed by #1454

Comments

@goleary
Copy link

goleary commented Jan 7, 2021

Expected behavior:
I expect the same type inference/autocompletion when using createUseStyles that I get when I use the style attribute in jsx. Here you can see the autocompletion offered on <div style...
image

Describe the bug:
There is no autocompletion when using createUseStyles
image

I'm a little bit confused because it appears as though a PR was merged to fix this behaviour but it doesn't seem to be working: #1352

@moshest is the behavior you were aiming to fix with that PR? or am I misunderstanding something?

Codesandbox link:
https://codesandbox.io/s/react-jss-ts-13cqf

Versions (please complete the following information):

  • jss: 10.5.0
  • react-jss: 10.5.0
  • react: 17.0.0
    Feel free to add any additional versions which you may think are relevant to the bug.
@moshest
Copy link
Member

moshest commented Jan 9, 2021

Honestly it's hard to follow. A few changes made since this PR and maybe it broken again.

This is the current status:

export type JssStyle =
  | {
      [K in keyof NormalCssProperties]:
        | NormalCssValues<K>
        | JssStyle
        | Func<NormalCssValues<K> | JssStyle | undefined>
        | Observable<NormalCssValues<K> | JssStyle | undefined>
    }
  | {
      [K: string]:
        | JssValue
        | JssStyle
        | Func<JssValue | JssStyle | undefined>
        | Observable<JssValue | JssStyle | undefined>
    }

This is the working PR version:

export type JssStyle = {
  [K in keyof NormalCssProperties | string]:
    | NormalCssValues<K>
    | JssStyle
    | Func<NormalCssValues<K> | JssStyle | undefined>
 }

Can you change the declaration locally and check if that fixes it for you?

@goleary
Copy link
Author

goleary commented Jan 12, 2021

@moshest when I make the change you suggested I see no change.

I also changed Styles to match what it was in your PR:

export type Styles<Name extends string | number | symbol = string> = Record<
  Name,
  JssStyle | string | Func<JssStyle | string | null | undefined>
>

But it's still not working.

I even updated my codesandbox to use the version where this was supposed to have been fixed (10.3.0), but I still get no autocomplete.

@moshest
Copy link
Member

moshest commented Jan 12, 2021

Well it needs further investigating.

I can share that I did this fix on TypeScript 3.x and the attached test file on the PR worked with autocomplete in WebStorm.
I will update if I will find some time to look into that and apply a new fix. Feel free to dive in in the meantime.

@kof kof added the typescript label Jan 18, 2021
@kof
Copy link
Member

kof commented Jan 18, 2021

@moshest I have added you to the core team since you have contributed to many typescript changes already, so you can review and merge them as you see fit, feel free to ping me if anything in twitter dm https://twitter.com/kof

@ymor
Copy link

ymor commented Feb 5, 2021

I believe #1423 has fixed this issue and now works correctly in 10.5.1

@moshest
Copy link
Member

moshest commented Feb 7, 2021

@goleary can you confirm as well?

@goleary
Copy link
Author

goleary commented Feb 8, 2021

Thanks for the tip @ymor & the ping @moshest

It is certainly better in the latest release. I am now getting autocomplete on property names.

That said, the traditional react style prop provides autocomplete for not only the property names, but also the values:

image

That is not working with createUseStyles:

image

Was this ever working in react-jss?

@k-yle
Copy link
Member

k-yle commented Feb 17, 2021

Was this ever working in react-jss?

we've being using JSS for 2 years and it's never worked. but I've made a PR that should make this work for TypeScript and JavaScript users: #1454

@kof kof closed this as completed in #1454 Feb 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants