Decorator shields public methods of a class

[droganov]

After decorating a dropdown:

@withStyles(style)
export default class UiDropdown

I'm no longer able to reference it:

this.dropdownRef.show() results in TypeError: this.dropdownRef.show is not a function

[ljharb]

withStyles is not a decorator - class decorators take (target, name, description) arguments.

withStyles takes (target, options) arguments.

You need to export default withStyles(UiDropdown, options) instead.

[droganov]

@ljharb withStyles takes thunk, options, not target (component)? and returns mustbe a decorator

If I do

@withStyles(styleThunkFunc)
export default class UiDropdown

The component gets decorated, but decorator does not let the component instance to be referenced.

[ljharb]

You're correct about the arguments :-) but you can't use a non-decorator as a decorator, and withStyles both is not, and does not return, a decorator.

[ljharb]

If you can provide a non-decorator repro case I'd be happy to reopen this.

[droganov]

@ljharb the case is that there can be a need to access methods of decorated component from parent component. Here is the example, where button needs to open a dropdown, and if we decorate dropdown this stops working:

import React, { PureComponent, PropTypes } from 'react';
import { Link } from 'react-router';

import { Dropdown, Flex, Tooltip, css, withStyles } from '../index.es6';
import { propTypes as dropdownPropTypes } from '../ui-dropdown/ui-dropdown';

import * as style from './ui-button.style.es6';

@withStyles(
  style.buttonThunk,
  { pureComponent: true },
)
export default class UiButton extends PureComponent {
  static propTypes = {
    children: PropTypes.node,
    dropdown: PropTypes.node,
    dropdownProps: PropTypes.shape(dropdownPropTypes),
    tooltip: PropTypes.node,

    blockLevel: PropTypes.bool,
    round: PropTypes.bool,
    selected: PropTypes.bool,
    uppercase: PropTypes.bool,

    special: PropTypes.bool,
    paper: PropTypes.bool,
    primary: PropTypes.bool,
    warning: PropTypes.bool,

    small: PropTypes.bool,
    large: PropTypes.bool,
    huge: PropTypes.bool,

    first: PropTypes.bool,
    middle: PropTypes.bool,
    last: PropTypes.bool,

    // className: PropTypes.string,
    flexProps: PropTypes.shape({}),
    rize: PropTypes.oneOf([1, 2, 3, 4, 5]),
    href: PropTypes.string,
    onClick: PropTypes.func,

    styles: PropTypes.shape({}),
  };

  setDropdownRef = (ref) => {
    this.dropdown = ref;
  };
  setTooltipRef = (ref) => {
    this.tooltip = ref;
  };

  get buttonStyle() {
    const {
      small,
      large,
      huge,

      special,
      paper,
      primary,
      warning,
      selected,

      round,
      rize,

      first,
      middle,
      last,

      uppercase,

      styles,
    } = this.props;

    return css(
      styles.button,

      small && styles.small,
      large && styles.large,
      huge && styles.huge,

      special && styles.special,
      paper && styles.paper,
      primary && styles.primary,
      warning && styles.warning,
      selected && styles.selected,

      round && styles.round,
      round && small && styles.roundSmall,
      round && large && styles.roundLarge,
      round && huge && styles.roundHuge,

      !round && first && styles.first,
      !round && middle && styles.middle,
      !round && last && styles.last,

      rize && styles[rize],

      uppercase && styles.uppercase,
    );
  }
  get children() {
    const { children } = this.props;
    return Array.isArray(children) ? children : [children];
  }
  get childStyle() {
    const { round, styles } = this.props;
    return css(
      styles.child,
      round && styles.roundChild,
    );
  }
  get element() {
    const { href } = this.props;
    if (!href) return 'button';
    return Link;
  }
  get flexStyle() {
    const { flexProps, round } = this.props;
    return style.getFlexProps(flexProps, round);
  }
  get wraperStyle() {
    const { blockLevel, styles } = this.props;
    return css(
      styles.wrapper,
      blockLevel && styles.blockLevelWrapper,
    );
  }

  handleClick = (event) => {
    const { onClick } = this.props;
    if (this.dropdown) {
      event.preventDefault();
      event.stopPropagation();
      this.dropdown.show();
    } else if (typeof onClick === 'function') {
      onClick(event);
    }
  }

  showTooltip = () => (this.tooltip && this.tooltip.show());
  hideTooltip = () => (this.tooltip && this.tooltip.hide());

  render() {
    const { dropdown, href, tooltip } = this.props;
    return (
      <div
        {...this.wraperStyle}
      >
        <Flex
          {...this.flexStyle}
          className={style.buttonClassName}
          element={this.element}
          onClick={this.handleClick}
          onMouseEnter={this.showTooltip}
          onMouseLeave={this.hideTooltip}
          to={href}
          {...this.buttonStyle}
        >
          {this.children.map(
            (child, key) =>
              <span
                key={key}
                {...this.childStyle}
              >
                {child}
              </span>,
          )}
        </Flex>
        {tooltip &&
          <Tooltip ref={this.setTooltipRef}>
            {tooltip}
          </Tooltip>
        }
        {dropdown &&
          <Dropdown
            {...this.props.dropdownProps}
            ref={this.setDropdownRef}
          >
            {dropdown}
          </Dropdown>
        }
      </div>
    );
  }
}

[droganov]

@ljharb I was able to solve this with custom ref prop:

<Dropdown
   {...this.props.dropdownProps}
   getDropdownRef={this.setDropdownRef}
>
   {dropdown}
</Dropdown>

[ljharb]

Glad you were able to solve it - but again, anywhere you've done @withStyles you've written invalid code, because withStyles is not a decorator.

[droganov]

@ljharb the code I wrote works as a decorator, more than that you have a readme case for decoration: https://github.com/airbnb/react-with-styles#example-usage

But anyway, my current solution is a hack.

I've found an article with explanation on how to setup the HOC ref:
https://medium.com/@franleplant/react-higher-order-components-in-depth-cf9032ee6c3e#.wpd0g697h

Please see under "Accessing the instance via Refs"

[lencioni]

It's true, we have that in the documentation. Perhaps we should remove that for now, or modify this to be a proper decorator. @droganov would you be interested in opening a PR for this?

[droganov]

@lencioni this is not about decoration actually, I'm interested in having access to wrapped component and I even provided a reference :-)

[ljharb]

I guess what I'm saying is one of two things:

1. stop using it as a decorator, the "bug" will disappear, because you're using it incorrectly as a decorator.
2. we should fix withStyles so that it is a proper decorator (which will probably mean a breaking change) and include tests to ensure that this continues to work correctly (which I can't conceive going wrong)

@droganov if you use withStyles as a function, but not as a decorator, without your workaround, what happens?

[droganov]

@ljharb I can't access wrapped component regardless of wrapping method:

...
export default withStyles(style, { pureComponent: true })(UiDropdown);
...
// Button
...
<Dropdown ref={this.setDropdownRef}>
   {dropdown}
</Dropdown>
...
handleClick = (event) => {
   this.dropdown.show();
}

Works the same way:

[ljharb]

Thanks - can you provide the entire source of UiDropdown?

[droganov]

@ljharb sure!

import React, { PureComponent, PropTypes } from 'react';

import { Overlay, Paper, css, withStyles } from '../index.es6';
import { propTypes as overlayPropTypes } from '../ui-overlay/ui-overlay';

import style from './ui-dropdown.style.es6';

export const propTypes = {
  ref: PropTypes.func,
  overlayProps: PropTypes.shape(overlayPropTypes),
  visible: PropTypes.bool,

  center: PropTypes.bool,
  right: PropTypes.bool,
  // bottomLeft: PropTypes.bool,
};

class UiDropdown extends PureComponent {
  static propTypes = {
    ...propTypes,
    children: PropTypes.node.isRequired,
  };
  state = { visible: this.props.visible };

  componentWillReceiveProps({ visible }) {
    if (visible !== this.props.visible) this.visible = visible;
  }

  get containerStyle() {
    const { center, right, styles } = this.props;
    return css(
      styles.contaner,
      center && styles.center,
      right && styles.right,
    );
  }

  get visible() {
    return this.state.visible;
  }
  set visible(visible) {
    this.setState({ visible });
  }

  hide = () => { this.visible = false; }
  show() { this.visible = true; }
  toggle = () => { this.visible = !this.visible; }

  render() {
    if (!this.visible) return null;
    const { children, overlayProps } = this.props;
    return (
      <div
        {...this.containerStyle}
      >
        <Overlay
          transparent
          {...overlayProps}
          onClose={this.hide}
        />
        <Paper onClick={this.hide}>
          {children}
        </Paper>
      </div>
    );
  }
}


export default withStyles(style, { pureComponent: true })(UiDropdown);

[ljharb]

Where's the line that errors? (this.dropdownRef.show())

[droganov]

@ljharb it's in UiButton I posted it earlier

handleClick = (event) => {
    const { onClick } = this.props;
    if (this.dropdown) {
      event.preventDefault();
      event.stopPropagation();
      this.dropdown.show();
    } else if (typeof onClick === 'function') {
      onClick(event);
    }
  }

[ljharb]

Sorry to repeat over and over again, but this happens even when UiButton does not use withStyles as a decorator? (in other words, @withStyles appears nowhere in your codebase)

[droganov]

@ljharb let me repeat it once again :-) it fails in both cases

<UiDropdown ref={...} /> — ref points to withStyles HOC when I expect it to point to the wrapped component i.e. UiDropdown.

And if I look here, I'm not able how find any ref delegation
https://github.com/airbnb/react-with-styles/blob/master/src/withStyles.jsx#L45

I suggest it can be solved this way:

Example: In the following example we explore how to access instance methods and the instance itself of the WrappedComponent via refs

function refsHOC(WrappedComponent) {
  return class RefsHOC extends React.Component {
    proc(wrappedComponentInstance) {
      wrappedComponentInstance.method()
    }
    
    render() {
      const props = Object.assign({}, this.props, {ref: this.proc.bind(this)})
      return <WrappedComponent {...props}/>
    }
  }
}

[ljharb]

ahhhh i see what you mean. the ref prop is special in React, and string refs are deprecated.

What you want to do instead is make a special prop, say, getTheRef, that takes a callback, and have the inner component explicitly pass its ref through the prop, rather than relying on the ref prop.

In fact, I believe your proposed solution won't work because React itself filter out the ref prop before it gets to actual component code.

[droganov]

@ljharb yes, it's about ref, but I used function ref, not string.

I've forked the package and will try to setup proc on weekend. I will suggest a PR if succeed

[ljharb]

That's better :-) but an explicit prop to provide the ref is better than using the ref prop, which is very brittle.

[droganov]

I think it should proxy method calls, not ref itself

[droganov]

so ref will still point to the HOC, but HOC.method() will go down to wrappedComponent.method()

[ljharb]

Instance methods of react components should only be called from within that component. Calling it in a ref callback isn't a great idea.

[droganov]

Why? React allows public methods. It's up to developer which religion to follow :-)

[droganov]

In my case it is parent-to-child communication, it can be done directly I think

[ljharb]

That violates React's one-way data model. It's certainly up to the developer to follow or ignore best practices and idioms, but that doesn't exempt it from being a foolish decision.

[droganov]

One-way is from parent to child in this case, it's all about the transport. As a parent you can say something directly to your child, or can write it down on the fridge (props). I prefer to say

[ljharb]

The only mechanism for communication is props. ref is a special prop that violates encapsulation. If you use literally any other prop name besides "key" or "ref", I would expect what you want to work just fine.

[droganov]

Actually yes, if we want to control the state of UI we need to pass data with props. I agree here. But there are a bunch if situations like prototyping, when you don't need to go the full cycle and need to craft something fast. I try to support both approaches.

[daisy1754]

Resurrecting old thread... I feel like there are legit use cases for ref. Indeed, official react doc says:

There are a few good use cases for refs:

• Managing focus, text selection, or media playback.
• Triggering imperative animations.
• Integrating with third-party DOM libraries.

Moreover, I feel like it is confusing as a user of library.
Assume we have code like below. myref2 and myref3 are called, while myref1 is not called.

<MyReactWithStyleCustomComponent
  ref={myref1} />
<MyCustomComponent
  ref={myref2} />
<input
  ref={myref3} />

Can ref support be implemented?

[ljharb]

Use cases notwithstanding, it's a far better design to have explicit ref props if you need them.

[richburdon]

Use case: the wrapped component class encapsulates a canvas DOM element and exposes a public method getSize. How would you access that?

As above, ref has legit puposes aside from breaking data binding encapsulation.

[ljharb]

@richburdon the canvas element would of course use ref, but your wrapper would have an explicit prop like “canvasRef”