allow any kind of theme for themeProvider and helper methods #544
Conversation
| @@ -17,7 +17,7 @@ const { | |||
| // see: https://github.com/palantir/tslint/issues/3505 | |||
| // It's possibly due to the rule upstream in tslint-config-standard | |||
| // tslint:disable-next-line | |||
| } = styledComponents as styledComponents.ThemedStyledComponentsModule<IThemeProps> | |||
| } = styledComponents as styledComponents.ThemedStyledComponentsModule<any> | |||
There was a problem hiding this comment.
There's got to be a better way than just using any here, right? Wouldn't this lose all intellisense for theme props in all brave components (${p => p.theme.xyz})? Perhaps Partial<IThemeProps> would work?
There was a problem hiding this comment.
Partial won't work. <IThemeProps & any> might, but not sure what that will get us. Maybe it's good. Either way seem comment on brave/brave-core#3300 as to whether this is needed yet.
There was a problem hiding this comment.
Approving since brave/brave-core#3300 was merged, but with a caveat: This change will prevent any type checking for any theme properties in any component in any repo. That seems bad when theme properties will change often, and we'd want to catch any issues before release. @cezaraugusto to create the follow-up issue.
|
follow-up issue #545 |
This allows
ThemedStyledComponentsModuleto receive any theme as a prop. Needed for brave/brave-core#3300.The reasoning I had is that we can type feature-specific themes in there instead of forcing a theme in root level. I'm open for discussion regarding better options.