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

Support typings for jss-plugin-nested #1036

Closed
worudso opened this issue Feb 17, 2019 · 7 comments
Closed

Support typings for jss-plugin-nested #1036

worudso opened this issue Feb 17, 2019 · 7 comments
Labels
bug It went crazy and killed everyone. complexity:low You can fix it, c'mon!

Comments

@worudso
Copy link

worudso commented Feb 17, 2019

Is your feature request related to a problem? Please describe.

jss.createStyleSheet({
 foo: {
  "&:hover": {
    background: "blue"
  },
 }
});

this code can't pass type check.

Describe the solution you'd like
#795
I'd thought the first item in this issue(Add typings to the extend plugin and preset default)
is about the typings for plugin-nested, but I checked the v10.0.0-alpha.10 and it seems not.
Is there any reason that the typings for nested rule is not included in the next release?

Are you willing to implement it?
Yes

@HenriBeck
Copy link
Member

I just fixed the issue in #973

Should be released with the next alpha, though this will only allow you to write this but not correctly type-check this.

@HenriBeck HenriBeck added bug It went crazy and killed everyone. complexity:low You can fix it, c'mon! labels Feb 17, 2019
@worudso
Copy link
Author

worudso commented Feb 25, 2019

@HenriBeck Can I test new type now?
https://www.npmjs.com/package/@types/jss
I can't find any type for 10.x.

@HenriBeck
Copy link
Member

Because the types are now inside jss, you should be able to remove @types/jss

@worudso
Copy link
Author

worudso commented Mar 5, 2019

export type Style = {[key: string]: Style | any}
export type Styles<Name extends string = string> = Record<Name, Style>

@HenriBeck it seems that now everything goes any.

@worudso
Copy link
Author

worudso commented Mar 5, 2019

// index.d.ts
import { CSSProperties } from "jss/css";

export type Style = { [key: string]: Style } | CSSProperties
export type Styles<Name extends string = string> = Record<Name, Style>

Is there any problem in this code?
At least it can properly check types of the code below

jss.createStyleSheet({
  a: {
    fontWeight: "bold",
    "&:hover": {
      "hi": {
        backgroundColor: "red"
      }
    }
  },
});

@HenriBeck
Copy link
Member

The first problem is that this type-definitions doesn't support function values, observables (though they are not that often used), expandable values, array values.

And &:hover rules are only supported when you have the nested plugin installed. So ideally the type would only allow nested rules when you have the nested plugin installed, though this isn't possible because the plugins are applied at runtime.

@worudso
Copy link
Author

worudso commented Mar 5, 2019

@HenriBeck I think the current type definition is little helpful. It passes type checkings for any style with any possible plugin, but at the same time, it can't autocomplete any property at all.

The first problem is that this type-definitions doesn't support function values, observables (though they are not that often used), expandable values, array values.

I don't know about function values, observables well. But array value, maybe it's something like

const styles = {
  container: {
    transition: [['opacity', '200ms'], ['width', '300ms']]
  }
}

Is this what you mean? Then I think it can be solved by just introducing JssValue.

export type JssValue =
  | string
  | number
  | Array<string | number | Array<string | number> | '!important'>
  | null
  | false;
export type Style = { [key: string]: Style | JssValue } | CSSProperties

It's just a draft. I didn't check it worked, But I'm willing to make it better.
I think that JssValue should contain every type vanilla jss allows e.g, function, array.

So ideally the type would only allow nested rules when you have the nested plugin installed, though this isn't possible because the plugins are applied at runtime.

I totally agree with you. But, how about making Style generic variable? Honestly, I think it's the only option we have.

jss.createStyleSheet<NestedStyle>({
  // ...
})

Of course, we can offer default Style for vanilla js.

And as far as I know, Observable is plugin-supported just like nested style which I'm asking.
But I found they are defined in vanilla jss.

import { ObservableProperties } from "jss/css";

export type CSSProperties =
	& ObservableProperties<csstype.Properties<Length>>
	& ObservableProperties<csstype.PropertiesHyphen<Length>>;

Why isn't there typing for plugin-nested? It seems just a simple inconsistency.
If putting every type into vanilla js is hard(I think you considered it not only hard but also wrong, and I agree with you), then seperating into each plugin would be better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug It went crazy and killed everyone. complexity:low You can fix it, c'mon!
Projects
None yet
Development

No branches or pull requests

2 participants