babel-plugin-stateful-functional-react-components icon indicating copy to clipboard operation
babel-plugin-stateful-functional-react-components copied to clipboard

Proposal: Avoid reconciliation by rewriting unbounded functions (Vote)

Open roman01la opened this issue 8 years ago • 8 comments

Issue

In the following example both onClick handlers are created as different functions on every update, which causes shouldComponentUpdate optimization to fail on child components:

Input

const Counter = ({ text }, { val } = { val: 0 }, setState) => (
  <div>
    <h1>{text}</h1>
    <div>
      <button onClick={() => setState({ val: val - 1 })}>-</button>
      <span>{val}</span>
      <button onClick={() => setState({ val: val + 1 })}>+</button>
    </div>
  </div>
);

Output

class Counter extends React.Component {
  constructor() {
    super();
    this.state = { val: 0 };
  }
  render() {

    const { text } = this.props;
    const { val } = this.state;

    return (
      <div>
        <h1>{text}</h1>
        <div>
          <button onClick={() => this.setState({ val: val - 1 })}>-</button>
          <span>{val}</span>
          <button onClick={() => this.setState({ val: val + 1 })}>+</button>
        </div>
      </div>
    );
  }
}

Solution

Introduce a helper function bind which will act like a marker to rewrite passed in functions in the following way:

  1. Create a unique ID uuid
  2. In component's constructor assign a function passed into bind to an instance of the component under uuid property
  3. Replace bind with this[uuid] in render method

Advantages

  • Once a function is defined in constructor it will stay the same thus not triggering child components update

Disadvantages

  • This adds complexity into plugin transform implementation
  • It is possible to use only instance properties, props and state in those functions because they will be moved into constructor

Thoughts

Does it makes sense to introduce this optimization since functional components are mostly used as a lightweight, presentational components without containing any CPU bound code?

Input

const Counter = ({ text }, { val } = { val: 0 }, { setState, bind }) => (
  <div>
    <h1>{text}</h1>
    <div>
      <button onClick={bind(() => setState({ val: val - 1 }))}>-</button>
      <span>{val}</span>
      <button onClick={bind(() => setState({ val: val + 1 }))}>+</button>
    </div>
  </div>
);

Output

class Counter extends React.Component {
  constructor() {
    super();
    this.state = { val: 0 };
    this[uuid] = () => this.setState({ val: this.state.val - 1 });
    this[uuid] = () => this.setState({ val: this.state.val + 1 });
  }
  render() {

    const { text } = this.props;
    const { val } = this.state;

    return (
      <div>
        <h1>{text}</h1>
        <div>
          <button onClick={this[uuid]}>-</button>
          <span>{val}</span>
          <button onClick={this[uuid]}>+</button>
        </div>
      </div>
    );
  }
}

roman01la avatar Apr 10 '17 12:04 roman01la

For debugging purpose bind helper could also accept a method name (bindTo name makes more sense in this case):

<button onClick={bindTo('decrement', () => setState({ val: val - 1 }))}>-</button>

Now I'm even more skeptical with this approach. This optimizations adds boilerplate code which is what the syntax is trying to avoid 🤔

roman01la avatar Jun 02 '17 11:06 roman01la

I believe this transformation is simpler:

Input

const Counter = ({ text }, { val } = { val: 0 }, setState) => (
  <div>
    <h1>{text}</h1>
    <div>
      <button onClick={() => setState({ val: val - 1 })}>-</button>
      <span>{val}</span>
      <button onClick={() => setState({ val: val + 1 })}>+</button>
    </div>
  </div>
);

Output

class Counter extends React.Component {
  constructor() {
    super();
    this.state = { val: 0 };
    this._temp = this.setState.bind(this);
    this._temp2 = this.setState.bind(this);
  }

  render() {

    const { text } = this.props;
    const { val } = this.state;

    return (
      <div>
        <h1>{text}</h1>
        <div>
          <button onClick={() => this._temp({val: this.state.val - 1})}>-</button>
          <span>{val}</span>
          <button onClick={() => this._temp({val: this.state.val + 1})}>-</button>
        </div>
      </div>
    );
  }
}

You need to keep track of the use of this in the setState function and bind a temporary method in the constructor. I used random and unique in scope identifier.

xtuc avatar Jun 02 '17 11:06 xtuc

@xtuc Not sure how this helps. This still creates a new function () => this._temp({val: this.state.val - 1}) on every render call. The whole function declaration should be binded in constructor, not just setState inside of it.

roman01la avatar Jun 02 '17 11:06 roman01la

Yes, that's correct. Your first solution is good.

You could also pass this as argument (and rename it) instead of binding it to the instance. It will maybe help for hot reload capabilities.

xtuc avatar Jun 02 '17 11:06 xtuc

@xtuc Good catch. Could you provide an example code? I'm not quite sure how exactly this should work.

roman01la avatar Jun 02 '17 11:06 roman01la

function _temp(self) {
  self.setState({ val: self.state.val - 1 });
}

function _temp2(self) {
  self.setState({ val: self.state.val + 1 });
}

class Counter extends React.Component {
  constructor() {
    super();
    this.state = { val: 0 };
  }

  render() {

    const { text } = this.props;
    const { val } = this.state;

    return (
      <div>
        <h1>{text}</h1>
        <div>
          <button onClick={() => _temp(this)}>-</button>
          <span>{val}</span>
          <button onClick={() => _temp2(this)}>-</button>
        </div>
      </div>
    );
  }
}

This is just an idea. I know I already had issues with bound function and hot reloading.

xtuc avatar Jun 02 '17 12:06 xtuc

I agree that binding in constructor will introduce a problem when hot-reloading. But the reality is that hot-reloading will break any way, because these are stateful components. I probably need to add a note on this in the readme. This is not something that can be fixed :(

roman01la avatar Jun 02 '17 12:06 roman01la

I've spent some time playing around with possible implementation. There are too many edge cases that need to be addressed, not sure if it worth it.

For example this

const Form = (props, { val } = { val: 0 }, { setState, bindTo }) => {
  <Input value={val} onChange={bindTo('_handleChange', () => setState({ val }))} />
}

Can be transformed into this, where reference to val is lost. Which means that it should track usage of value in scope and handle it properly in different cases depending on what's the source of that value

class Form extends React.Component {
  constructor() {
    super();
    this.state = { val: 0 };

    this._handleChange = () => this.setState({ val }); // reference to `val` is lost here
  }

  render() {
    const props = this.props;
    const { val } = this.state;

    React.createElement(Input, { value: val, onChange: this._handleChange });
  }

}

roman01la avatar Jun 02 '17 16:06 roman01la