jss
jss copied to clipboard
cssinjs
fix: add fallbacks to jssStyle
Corresponding Issue(s):
What Would You Like to Add/Fix?
Todo
- [ ] Add test(s) that verify the modified behavior
- [ ] Add documentation if it changes public API
Expectations on Changes
Changelog
The change seems correct to me, fallbacks are optional, this is waiting for a review from anyone from @cssinjs/typescript team
Hmmm... At a first glance, the change doesn't appear to be harmful. However, I have seen things behave strangely when something like Record<keys, values> or { [key: string]: value } is mixed with an explicit interface as it is here. So...actually it may not work out. I might explore in the TypeScript playground for a bit.
May I ask what the fallback property is intended for?
Hmmm. I'm more confident that this won't work now. In the TypeScript playground I tried:
type Blah = { pizza: string | number } & { [K: string]: number };
const blah: Blah = {
pizza: "1",
}
and got an error because "1" is not a number. Things like { [key: string]: value } kind of absorb everything. So the additional explicit interfaces you try to define end up being negated/nullified, effectively. This is another issue that I've had many headaches with in my side projects.
Were the tests run for this?
@ITenthusiasm fallbacks is basically how we describe css fallbacks in js, because we can't use the same property twice with objects https://cssinjs.org/jss-syntax?v=v10.6.0#fallbacks
This is an example of what I did:
https://www.typescriptlang.org/play?jsx=0&ts=4.2.3#code/C4TwDgpgBAUgznAyqANtAvFA3gWAFBRQBmAhiigEYkDGA1nAPwBcUcwATgJYB2A5vgF8oAMigAKfIUIAfbJKmEiAeyUsA5MqVqostVXZr5hAUZ1yCCqAG0A0izZc+AXRbcArgFsKEdqYEBKfHxqJW42KBIWeCRUDGxiFRYAIn0kgBpiMkoaenVScio6ODUMtSVgAAsfYpYAJgBmASA
Interesting. So if the second type in the OR type chain is removed, then the error I mentioned earlier gets created. Seems inconsistent... hm....
Yeah. Due to the inconsistencies, I don't think I'm willing to approve this unless there are some robust tests to show nothing unexpected would happen.
Interesting. So if the second type in the
ORtype chain is removed, then the error I mentioned earlier gets created. Seems inconsistent... hm....
Please show by example which this change is inconsistent.
I removed the second type in the OR but that working again. So that is consistent.
@hosseinmd I need to delegate the decision to @ITenthusiasm because he is much deeper than I am in TS
@ITenthusiasm The idea of JssStyle is not throw error anyway, that is just for helping to VSCode intellisense. This is the main mission of second OR and JssValue to prevent throwing error.
type Blah = { pizza: string | number } & { [K: string]: number }; const blah: Blah = { pizza: "1", }
This example is not correct. You should use OR, like original code.
VSCode intellisense popup is recognized the display property type correctly
@hosseinmd I just pulled it and it doesn't compile, I get tons of errors, is it because of recent changes in master?
When I run it locally, I see 64 errors
Oleg-9EJHCC:jss isonen$ yarn tsc
yarn run v1.15.2
$ /Users/isonen/work/cssinjs/jss/node_modules/.bin/tsc
packages/jss/src/index.d.ts:19:13 - error TS2456: Type alias 'JssStyle' circularly references itself.
19 export type JssStyle<Props = any, Theme = undefined> = {
~~~~~~~~
packages/jss/src/index.d.ts:20:15 - error TS2315: Type 'JssStyle' is not generic.
20 fallbacks?: JssStyle<Props, Theme> | (JssStyle<Props, Theme>[])
~~~~~~~~~~~~~~~~~~~~~~
packages/jss/src/index.d.ts:20:41 - error TS2315: Type 'JssStyle' is not generic.
20 fallbacks?: JssStyle<Props, Theme> | (JssStyle<Props, Theme>[])
~~~~~~~~~~~~~~~~~~~~~~
packages/jss/src/index.d.ts:25:11 - error TS2315: Type 'JssStyle' is not generic.
25 | JssStyle<Props, Theme>
~~~~~~~~~~~~~~~~~~~~~~
packages/jss/src/index.d.ts:26:51 - error TS2315: Type 'JssStyle' is not generic.
26 | Func<Props, Theme, NormalCssValues<K> | JssStyle<undefined, undefined> | undefined>
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
packages/jss/src/index.d.ts:32:11 - error TS2315: Type 'JssStyle' is not generic.
32 | JssStyle<Props, Theme>
~~~~~~~~~~~~~~~~~~~~~~
packages/jss/src/index.d.ts:33:41 - error TS2315: Type 'JssStyle' is not generic.
33 | Func<Props, Theme, JssValue | JssStyle<undefined, undefined> | undefined>
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
packages/jss/src/index.d.ts:43:5 - error TS2315: Type 'JssStyle' is not generic.
43 | JssStyle<Props, Theme>
~~~~~~~~~~~~~~~~~~~~~~
packages/jss/src/index.d.ts:44:11 - error TS2315: Type 'JssStyle' is not generic.
44 | Array<JssStyle<Props, Theme>>
~~~~~~~~~~~~~~~~~~~~~~
packages/jss/src/index.d.ts:46:24 - error TS2315: Type 'JssStyle' is not generic.
46 | Func<Props, Theme, JssStyle<undefined, undefined> | string | null | undefined>
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
packages/jss/tests/jss-tests.ts:85:3 - error TS2578: Unused '@ts-expect-error' directive.
85 // @ts-expect-error
~~~~~~~~~~~~~~~~~~~
packages/jss/tests/jss-tests.ts:94:3 - error TS2578: Unused '@ts-expect-error' directive.
94 // @ts-expect-error
~~~~~~~~~~~~~~~~~~~
packages/jss/tests/types/Styles.ts:29:21 - error TS7006: Parameter 'props' implicitly has an 'any' type.
29 justifyContent: props => (props.flag ? 'center' : undefined)
~~~~~
packages/jss/tests/types/Styles.ts:40:9 - error TS7006: Parameter 'props' implicitly has an 'any' type.
40 func: props => ({
~~~~~
packages/jss/tests/types/Styles.ts:50:13 - error TS7006: Parameter 'props' implicitly has an 'any' type.
50 funcNull: props => null,
~~~~~
packages/jss/tests/types/Styles.ts:51:17 - error TS7006: Parameter 'props' implicitly has an 'any' type.
51 funcWithTerm: props => ({
~~~~~
packages/jss/tests/types/Styles.ts:53:5 - error TS2783: 'height' is specified more than once, so this usage will be overwritten.
53 height: props.flag ? 330 : 400,
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
packages/jss/tests/types/Styles.ts:60:5
60 ...(props.flag && {
~~~~~~~~~~~~~~~~~~~
61 height: 288
~~~~~~~~~~~~~~~~~
62 })
~~~~~~
This spread always overwrites this property.
packages/jss/tests/types/Styles.ts:78:27 - error TS7031: Binding element 'flag' implicitly has an 'any' type.
78 rootParamDeclaration: ({flag, theme}) => ({
~~~~
packages/jss/tests/types/Styles.ts:78:33 - error TS7031: Binding element 'theme' implicitly has an 'any' type.
78 rootParamDeclaration: ({flag, theme}) => ({
~~~~~
packages/jss/tests/types/Styles.ts:85:31 - error TS7031: Binding element 'flag' implicitly has an 'any' type.
85 innerParamDeclaration1: ({flag, theme}) => '',
~~~~
packages/jss/tests/types/Styles.ts:85:37 - error TS7031: Binding element 'theme' implicitly has an 'any' type.
85 innerParamDeclaration1: ({flag, theme}) => '',
~~~~~
packages/jss/tests/types/Styles.ts:86:31 - error TS7031: Binding element 'flag' implicitly has an 'any' type.
86 innerParamDeclaration2: ({flag, theme}) => ({
~~~~
packages/jss/tests/types/Styles.ts:86:37 - error TS7031: Binding element 'theme' implicitly has an 'any' type.
86 innerParamDeclaration2: ({flag, theme}) => ({
~~~~~
packages/react-jss/tests/types/createUseStyles.ts:29:1 - error TS2578: Unused '@ts-expect-error' directive.
29 // @ts-expect-error
~~~~~~~~~~~~~~~~~~~
packages/react-jss/tests/types/createUseStyles.ts:31:29 - error TS7031: Binding element 'innerTheme' implicitly has an 'any' type.
31 themeNotAllowed: ({theme: innerTheme}) => ({
~~~~~~~~~~
packages/react-jss/tests/types/createUseStyles.ts:58:5 - error TS2578: Unused '@ts-expect-error' directive.
58 // @ts-expect-error
~~~~~~~~~~~~~~~~~~~
packages/react-jss/tests/types/createUseStyles.ts:94:40 - error TS2559: Type '{ property: string; }' has no properties in common with type '{ theme?: Theme | undefined; }'.
94 const themeArg7ClassesPass = themeArg7(expectedCustomProps)
~~~~~~~~~~~~~~~~~~~
packages/react-jss/tests/types/createUseStyles.ts:114:16 - error TS7031: Binding element 'theme' implicitly has an 'any' type.
114 themeOnly: ({theme}) => ({
~~~~~
packages/react-jss/tests/types/createUseStyles.ts:133:20 - error TS7031: Binding element 'property' implicitly has an 'any' type.
133 propsAndTheme: ({property, theme}) => ({
~~~~~~~~
packages/react-jss/tests/types/createUseStyles.ts:133:30 - error TS7031: Binding element 'theme' implicitly has an 'any' type.
133 propsAndTheme: ({property, theme}) => ({
~~~~~
packages/react-jss/tests/types/createUseStyles.ts:153:20 - error TS7031: Binding element 'property' implicitly has an 'any' type.
153 propsAndTheme: ({property, theme}) => ({
~~~~~~~~
packages/react-jss/tests/types/createUseStyles.ts:153:30 - error TS7031: Binding element 'theme' implicitly has an 'any' type.
153 propsAndTheme: ({property, theme}) => ({
~~~~~
packages/react-jss/tests/types/createUseStyles.ts:175:20 - error TS7031: Binding element 'property' implicitly has an 'any' type.
175 singleValue: ({property, theme}) => '',
~~~~~~~~
packages/react-jss/tests/types/createUseStyles.ts:175:30 - error TS7031: Binding element 'theme' implicitly has an 'any' type.
175 singleValue: ({property, theme}) => '',
~~~~~
packages/react-jss/tests/types/createUseStyles.ts:176:16 - error TS7031: Binding element 'property' implicitly has an 'any' type.
176 nestOne: ({property, theme}) => ({
~~~~~~~~
packages/react-jss/tests/types/createUseStyles.ts:176:26 - error TS7031: Binding element 'theme' implicitly has an 'any' type.
176 nestOne: ({property, theme}) => ({
~~~~~
packages/react-jss/tests/types/createUseStyles.ts:188:20 - error TS7031: Binding element 'property' implicitly has an 'any' type.
188 singleValue: ({property, theme}) => '',
~~~~~~~~
packages/react-jss/tests/types/createUseStyles.ts:188:30 - error TS7031: Binding element 'theme' implicitly has an 'any' type.
188 singleValue: ({property, theme}) => '',
~~~~~
packages/react-jss/tests/types/createUseStyles.ts:191:27 - error TS7031: Binding element 'property' implicitly has an 'any' type.
191 innerSingleValue: ({property, theme}) => '',
~~~~~~~~
packages/react-jss/tests/types/createUseStyles.ts:191:37 - error TS7031: Binding element 'theme' implicitly has an 'any' type.
191 innerSingleValue: ({property, theme}) => '',
~~~~~
packages/react-jss/tests/types/createUseStyles.ts:192:21 - error TS7031: Binding element 'property' implicitly has an 'any' type.
192 secondNest: ({property, theme}) => ({
~~~~~~~~
packages/react-jss/tests/types/createUseStyles.ts:192:31 - error TS7031: Binding element 'theme' implicitly has an 'any' type.
192 secondNest: ({property, theme}) => ({
~~~~~
packages/react-jss/tests/types/createUseStyles.ts:205:20 - error TS7031: Binding element 'property' implicitly has an 'any' type.
205 singleValue: ({property, theme}) => '',
~~~~~~~~
packages/react-jss/tests/types/createUseStyles.ts:205:30 - error TS7031: Binding element 'theme' implicitly has an 'any' type.
205 singleValue: ({property, theme}) => '',
~~~~~
packages/react-jss/tests/types/createUseStyles.ts:208:27 - error TS7031: Binding element 'property' implicitly has an 'any' type.
208 innerSingleValue: ({property, theme}) => '',
~~~~~~~~
packages/react-jss/tests/types/createUseStyles.ts:208:37 - error TS7031: Binding element 'theme' implicitly has an 'any' type.
208 innerSingleValue: ({property, theme}) => '',
~~~~~
packages/react-jss/tests/types/createUseStyles.ts:211:33 - error TS7031: Binding element 'property' implicitly has an 'any' type.
211 innerMostSingleValue: ({property, theme}) => '',
~~~~~~~~
packages/react-jss/tests/types/createUseStyles.ts:211:43 - error TS7031: Binding element 'theme' implicitly has an 'any' type.
211 innerMostSingleValue: ({property, theme}) => '',
~~~~~
packages/react-jss/tests/types/createUseStyles.ts:212:22 - error TS7031: Binding element 'property' implicitly has an 'any' type.
212 thirdNest: ({property, theme}) => ({
~~~~~~~~
packages/react-jss/tests/types/createUseStyles.ts:212:32 - error TS7031: Binding element 'theme' implicitly has an 'any' type.
212 thirdNest: ({property, theme}) => ({
~~~~~
packages/react-jss/tests/types/createUseStyles.ts:233:44 - error TS2559: Type '{ property: string; }' has no properties in common with type '{ theme?: Theme | undefined; }'.
233 const noThemeArg8ClassesPass = noThemeArg8(expectedCustomProps)
~~~~~~~~~~~~~~~~~~~
packages/react-jss/tests/types/docs.tsx:21:16 - error TS7031: Binding element 'spacing' implicitly has an 'any' type.
21 padding: ({spacing}) => spacing || 10
~~~~~~~
packages/react-jss/tests/types/docs.tsx:23:14 - error TS7031: Binding element 'theme' implicitly has an 'any' type.
23 myLabel: ({theme, ...props}) => ({
~~~~~
packages/react-jss/tests/types/docs.tsx:60:12 - error TS7006: Parameter 'props' implicitly has an 'any' type.
60 myLabel: props => ({
~~~~~
packages/react-jss/tests/types/docs.tsx:78:14 - error TS7006: Parameter 'props' implicitly has an 'any' type.
78 padding: props => props.spacing || 10
~~~~~
packages/react-jss/tests/types/withStyles.tsx:54:22 - error TS7031: Binding element 'property' implicitly has an 'any' type.
54 someClassName: ({property}) => '',
~~~~~~~~
packages/react-jss/tests/types/withStyles.tsx:66:12 - error TS7031: Binding element 'property' implicitly has an 'any' type.
66 [1]: ({property}) => '',
~~~~~~~~
packages/react-jss/tests/types/withStyles.tsx:99:20 - error TS7031: Binding element 'property' implicitly has an 'any' type.
99 someClassName: ({property}) => '',
~~~~~~~~
packages/react-jss/tests/types/withStyles.tsx:109:20 - error TS7031: Binding element 'theme' implicitly has an 'any' type.
109 someClassName: ({theme}) => '',
~~~~~
packages/react-jss/tests/types/withStyles.tsx:119:20 - error TS7031: Binding element 'property' implicitly has an 'any' type.
119 someClassName: ({property, theme}) => '',
~~~~~~~~
packages/react-jss/tests/types/withStyles.tsx:119:30 - error TS7031: Binding element 'theme' implicitly has an 'any' type.
119 someClassName: ({property, theme}) => '',
~~~~~
packages/react-jss/tests/types/withStyles.tsx:129:10 - error TS7031: Binding element 'property' implicitly has an 'any' type.
129 [1]: ({property, theme}) => '',
~~~~~~~~
packages/react-jss/tests/types/withStyles.tsx:129:20 - error TS7031: Binding element 'theme' implicitly has an 'any' type.
129 [1]: ({property, theme}) => '',
~~~~~
packages/react-jss/tests/types/withStyles.tsx:167:1 - error TS2578: Unused '@ts-expect-error' directive.
167 // @ts-expect-error
~~~~~~~~~~~~~~~~~~~
Found 64 errors.
@hosseinmd honestly, how did you make it pass tsc compiler ... there are dozens of errors
Ok I updated to typescript 4.4.2 and now the only 2 errors I get are these:
167 // @ts-expect-error
~~~~~~~~~~~~~~~~~~~
packages/react-jss/tests/types/withStyles.tsx:170:12 - error TS2345: Argument of type '(theme: MyTheme) => Styles<string, unknown, null>' is not assignable to parameter of type 'Styles<string, unknown, MyTheme> | ((theme: MyTheme) => Styles<string, unknown, undefined>)'.
Type '(theme: MyTheme) => Styles<string, unknown, null>' is not assignable to type '(theme: MyTheme) => Styles<string, unknown, undefined>'.
Type 'Styles<string, unknown, null>' is not assignable to type 'Styles<string, unknown, undefined>'.
'string' index signatures are incompatible.
Type 'string | MinimalObservable<string | JssStyle<any, undefined> | null | undefined> | JssStyle<unknown, null> | JssStyle<unknown, null>[] | ((data: { ...; }) => string | ... 2 more ... | undefined)' is not assignable to type 'string | MinimalObservable<string | JssStyle<any, undefined> | null | undefined> | JssStyle<unknown, undefined> | JssStyle<unknown, undefined>[] | ((data: unknown) => string | ... 2 more ... | undefined)'.
Type '{ fallbacks?: JssStyle<unknown, null> | JssStyle<unknown, null>[] | undefined; } & { alignContent?: AlignContent | MinimalObservable<AlignContent | JssStyle<any, undefined> | undefined> | JssStyle<...> | ((data: { ...; }) => AlignContent | ... 1 more ... | undefined) | undefined; ... 763 more ...; vectorEffect?: Vec...' is not assignable to type 'string | MinimalObservable<string | JssStyle<any, undefined> | null | undefined> | JssStyle<unknown, undefined> | JssStyle<unknown, undefined>[] | ((data: unknown) => string | ... 2 more ... | undefined)'.
Type '{ fallbacks?: JssStyle<unknown, null> | JssStyle<unknown, null>[] | undefined; } & { alignContent?: AlignContent | MinimalObservable<AlignContent | JssStyle<any, undefined> | undefined> | JssStyle<...> | ((data: { ...; }) => AlignContent | ... 1 more ... | undefined) | undefined; ... 763 more ...; vectorEffect?: Vec...' is not assignable to type '{ fallbacks?: JssStyle<unknown, undefined> | JssStyle<unknown, undefined>[] | undefined; } & { [K: string]: JssValue | MinimalObservable<JssValue | JssStyle<any, undefined> | undefined> | JssStyle<...> | ((data: unknown) => JssValue | ... 1 more ... | undefined); }'.
Type '{ fallbacks?: JssStyle<unknown, null> | JssStyle<unknown, null>[] | undefined; } & { alignContent?: AlignContent | MinimalObservable<AlignContent | JssStyle<any, undefined> | undefined> | JssStyle<...> | ((data: { ...; }) => AlignContent | ... 1 more ... | undefined) | undefined; ... 763 more ...; vectorEffect?: Vec...' is not assignable to type '{ fallbacks?: JssStyle<unknown, undefined> | JssStyle<unknown, undefined>[] | undefined; }'.
Types of property 'fallbacks' are incompatible.
Type 'JssStyle<unknown, null> | JssStyle<unknown, null>[] | undefined' is not assignable to type 'JssStyle<unknown, undefined> | JssStyle<unknown, undefined>[] | undefined'.
Type '{ fallbacks?: JssStyle<unknown, null> | JssStyle<unknown, null>[] | undefined; } & { alignContent?: AlignContent | MinimalObservable<AlignContent | JssStyle<any, undefined> | undefined> | JssStyle<...> | ((data: { ...; }) => AlignContent | ... 1 more ... | undefined) | undefined; ... 763 more ...; vectorEffect?: Vec...' is not assignable to type 'JssStyle<unknown, undefined> | JssStyle<unknown, undefined>[] | undefined'.
170 withStyles(passingFunctionNullTheme)(SimpleComponent)
~~~~~~~~~~~~~~~~~~~~~~~~
Found 2 errors.
I have no clue why they happen and its frustrating up to the point to throw ts out altogether and make it someone else's problem.
I pushed my state to this branch: https://github.com/cssinjs/jss/tree/fallbacksType would appreciate help @cssinjs/typescript
Oh those 2 errors happen on master too when upgrading ts to 4.4.2, maybe @ITenthusiasm knows what that is
we should discuss it in this separate PR https://github.com/cssinjs/jss/pull/1550