fix utility props value type about override the Theme interface
#345
Conversation
strozw
force-pushed
the
fix_style_props_value_type_in_system_packages
branch
from
e5a42c0
to
78859fa
Compare
strozw
force-pushed
the
fix_style_props_value_type_in_system_packages
branch
from
78859fa
to
e0942c5
Compare
strozw
changed the title
strozw
changed the title
strozw
changed the title
Theme interface in typescript
strozw
changed the title
Theme interface in typescriptTheme interface
packages/system/src/types.ts
Outdated
| @@ -126,7 +126,7 @@ declare type SynthesizedPath<T extends {}> = { | |||
| export type ThemeNamespaceValue< | |||
| K extends string, | |||
| T extends ITheme, | |||
| > = SynthesizedPath<T[K]> | |||
| > = SynthesizedPath<T[K]> | number | string | false | |||
There was a problem hiding this comment.
I don't think the fix is correct. ThemeNamespaceValue means a value in the theme namespace, nothing else. However, I see what you want to fix, but I think it is not the right place to do it.
There was a problem hiding this comment.
Thank you for your review.
Sure, my fix wasn't fit for name.
Read the code again and investigate the appropriate corrections.
There was a problem hiding this comment.
1st, I reverted prev changes, and changed SystemProp insted of ThemeNamespaeceValue for fix utility props. I think this type name means utility props, but what do you think?
2nd, above changes is not cover same problem about each themeGetter. so I changed ThemeGetter for it.
３rd, the first change on SystemProps found an issue where col and row props didn't support false, so we fixed each one.
This reverts commit 7a430a9.
|
thanks for your review.
Indeed, I think it should be safe for Typing to be more accurate and limited. This PR will be closed and I'll reconsider if we can contribute in another way. |
Summary
this PR is fix below issue by add
string | number | falseintoThemeNamespaceValue(7a430a9).Fixes #344
and fix type-errors about theme value types in tests files.
Test plan
npm installin project rootnpx tsc --noEmit -p ./packages/systemnpx tsc --noEmit -p ./packages/styled-componentsnpx tsc --noEmit -p ./packages/emotionnpm testin project root)