cssinjs / theming

Unified CSSinJS theming solution for React
300 stars 39 forks source link

Opt out of error if there is no theme context in the tree #13

Closed kentcdodds closed 7 years ago

kentcdodds commented 7 years ago

This is what we do in glamorous

function generateWarningMessage(Comp) {
  const componentName = Comp.displayName || Comp.name || 'FunctionComponent'
  // eslint-disable-next-line max-len
  return `glamorous warning: Expected component called "${componentName}" which uses withTheme to be within a ThemeProvider but none was found.`
}
// ...

// ...
    componentWillMount() {
      if (!this.context[CHANNEL]) {
        if (process.env.NODE_ENV !== 'production' && !this.warned) {
          this.warned = true
          // eslint-disable-next-line no-console
          console.warn(generateWarningMessage(ComponentToTheme))
        }
      }
// ...

I'd also love it if this could be disabled. For GlamorousComponents, they all need to be themeable, but not always will they be inside a ThemeProvider (you can pass the theme/override via props). So in glamorous we also accept options: withTheme(Comp, {noWarn: true}).

iamstarkov commented 7 years ago

right now withTheme will throw error if its used without ThemeProvider https://github.com/iamstarkov/theming/blob/master/src/create-with-theme.js#L22

kentcdodds commented 7 years ago

Ah, in that case, I'd like a way to opt out of that :)

iamstarkov commented 7 years ago

you might want to take a look at themeListener in #3, because it's exactly about that level of deep integration.

iamstarkov commented 7 years ago

pls take a look into that, there is also published theming package in my npm scope with that pr published

kentcdodds commented 7 years ago

That looks like it'll work :+1: Thanks!