javivelasco / react-css-themr

Easy theming and composition for CSS Modules.
MIT License
591 stars 68 forks source link

Add theme to childContext #22

Closed wtgtybhertgeghgtwtg closed 7 years ago

wtgtybhertgeghgtwtg commented 8 years ago

Let's say I have some two components, ChildComponent

@themr('ChildComponent')
class ChildComponent extends Component {
  render() {
    const { theme } = this.props;
    return (
      <div className={theme.child}></div>
    )
  }
}

and ParentComponent

@themr('ParentComponent')
class ParentComponent extends Component {
  render() {
    const { theme } = this.props;
    return (
      <div className={theme.parent}>
        <ChildComponent theme={theme} />
      </div>
    )
  }
}

For the theme prop of ParentComponent to be applied to ChildComponent, it has to be explicitly passed down. It would be a breaking change, but could Themed define getChildContext so that the theme of ParentComponent is automatically part of the theme of ChildComponent?

javivelasco commented 8 years ago

I guess there is something we could do to handle these cases explicitly. If a child component can be considered part of the parent component, they both can define (and must define) the same key. For example themr('Radio', style).

If you provide a theme via context it's going to work nicely because they use the same key but I guess that if you pass down a style to a parent of an specific type, it makes sense if this component writes in the context the updated theme. Therefore, the child would read from context the theme you passed to the parent via props.

This update is not breaking and has no downsides in my opinion apart from the cost of writing in the context when a theme is received

raveclassic commented 8 years ago

@wtgtybhertgeghgtwtg You can easily set the same theme for different keys when constructing context:

import css from './shared.css';
const context = {
 ChildComponent: css,
 ParentComponent: css
};

On the other hand, if you accept exactly the same theme in different components why not to use the same keys (names) for them?

UPDATE: If your case is about including a theme passed to props, then I believe the best way here is to explicitly extract child's theme from props and to pass it down. I've opened #24 to help with such cases.

UPDATE: The reason of avoiding context is that it can't hold dynamic values when using pure-rendering: https://github.com/facebook/react/issues/2517

javivelasco commented 8 years ago

Yeah, you're right, that's what we are doing with react-toolbox. Just using the same key for both child and parent if the belong to the same complex component

raveclassic commented 7 years ago

@wtgtybhertgeghgtwtg What you are suggesting is a good old cascade when Parent component affects all possible Children on all levels deeper. Such behavior is unsafe and was meant to be eliminated by introduction of css-modules. However, ThemeProviders could still be nested to make such behavior explicit and #19 tracks it. @javivelasco I believe this issue can be closed.

javivelasco commented 7 years ago

Cool, thanks @raveclassic !