Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add defineClass to addons #443

Closed
wants to merge 5 commits into from
Closed

Add defineClass to addons #443

wants to merge 5 commits into from

Conversation

brainkim
Copy link
Contributor

Warning: lot of code!
This pull request adds a defineClass method to addons similar to Twitter Flight's defineComponent. See the tests for usage examples. I've tried to keep changes to core files to a minimum, and have only exposed/extracted some parts of ReactCompositeComponent to prevent duplicated code.

@brainkim
Copy link
Contributor Author

@zpao asked me to motivate this pull request so here we go:

defineClass vs createClass

Fig. 1: Timer example from website with createClass

var Timer = React.createClass({
  getInitialState: function() {
    return {secondsElapsed: 0};
  },
  tick: function() {
    this.setState({secondsElapsed: this.state.secondsElapsed + 1});
  },
  componentDidMount: function() {
    this.interval = setInterval(this.tick, 1000);
  },
  componentWillUnmount: function() {
    clearInterval(this.interval);
  },
  render: function() {
    return React.DOM.div({},
      'Seconds Elapsed: ' + this.state.secondsElapsed
    );
  }
});

React.renderComponent(Timer({}), mountNode);

Fig 2: Timer example with defineClass

var Timer = React.defineClass(function() {
  this.initialState({secondsElapsed: 0});

  this.tick = function() {
    this.setState({secondsElapsed: this.state.secondsElapsed + 1});
  };

  this.after('mount', function() {
    this.interval = setInterval(this.tick, 1000);
  });

  this.before('unmount', function() {
    clearInterval(this.interval);
  });

  this.render = function() {
    return React.DOM.div({},
      'Seconds Elapsed: ' + this.state.secondsElapsed
    );
  };
});

React.renderComponent(Timer({}), mountNode);

Arguments in favor of defineClass

1. No more giant object literals (GOLs)

createClass uses the same _.extends inheritance pattern that has engendered countless debates about whether commas should go first or last. Subjectively, I think GOLs are ugly, especially when using them to define modules, partly because the : operator is much harder to notice in code than the = operator, and partly because although we indent object literals as though they are blocks, they don't represent a scope, but a confining subset of javascript syntax. Using defineClass puts us in familiar territory in the sense that all component properties are assigned with =, and the indentation represents an actual block within which you can hide variables and execute arbitrary code. Using GOLs feels claustrophobic, whereas using function definitions gives us room to breathe.

Alternatively, there are also objective disadvantages to using GOLs. For instance, you cannot reference the GOL itself within a GOL unless you're inside a method definition. This problem is made clear by the fact that React pre-emptively auto-binds methods, mainly because there is no trivial way to bind the method otherwise. With defineClass, you could bind methods to the component using Function.prototype.bind (I say 'could' because the current implementation, which relies on createClass, does not allow for this).

Additionally, you can call methods which have already been defined on the component. In fig. 2, we use the initialState and the before/after methods, rather than defining verbose getInitialState, componentDidMount, and componentWillUnmount properties. In the pull request, I have limited the pre-defined methods to those which correspond to the createClass specification, but it is easy and fun to think of other methods you might want available at definition time. You can also do more sophisticated things, like defining several properties at once (fig. 3).

Fig. 3: Metaprogramming in javascript? (taken straight from pull request)

function AdviceDefinition() {
  'before after around filter'.split(' ').forEach(function(joinPoint) {
    this[joinPoint] = function(methodName, callback) {
      wrapMethod.call(this, joinPoint, methodName, callback);
    };
  }.bind(this));
}

2. Abstracting the React component lifecycle with aspect-oriented programming

My favorite part about the defineClass approach is that it allows us to use aspect-oriented programming. I posit that underlying React's sophisticated "mount-receiveProps-update-unmount" lifecycle are latent methods which could be defined and then hooked into with advice methods. This sort of refactoring would make the React source code clearer, less verbose, and more resilient to future changes to the lifecycle.

One real use-case for the advice api would be something like counting the number of renders a component makes. With createClass, you would do this by putting code inside the render function which manually counts the number of times it is called. With defineClass, you can use a mixin which counts the render before or after it has completed, separating your component code from measurement code. In short, defineClass allows for drop-in logging, debugging, and performance-testing.

3. Better mixins.

Fig. 4: Mixin example from website

/** @jsx React.DOM */

var SetIntervalMixin = {
  componentWillMount: function() {
    this.intervals = [];
  },
  setInterval: function() {
    this.intervals.push(setInterval.apply(null, arguments));
  },
  componentWillUnmount: function() {
    this.intervals.map(clearInterval);
  }
};

var TickTock = React.createClass({
  mixins: [SetIntervalMixin], // Use the mixin
  getInitialState: function() {
    return {seconds: 0};
  },
  componentDidMount: function() {
    this.setInterval(this.tick, 1000); // Call a method on the mixin
  },
  tick: function() {
    this.setState({seconds: this.state.seconds + 1});
  },
  render: function() {
    return (
      <p>
        React has been running for {this.state.seconds} seconds.
      </p>
    );
  }
});

React.renderComponent(
  <TickTock />,
  document.getElementById('example')
);

Fig. 5: Mixin example with defineClass

/** @jsx React.DOM */

function SetIntervalDefinition() {
  this.before('mount', function() {
    this.intervals = [];
  });
  this.setInterval = function() {
    this.intervals.push(setInterval.apply(null, arguments));
  };
  this.before('unmount', function() {
    this.intervals.map(clearInterval);
  });
}

var TickTock = React.defineClass(SetIntervalDefinition, function() {
  this.initialState({seconds: 0});

  this.after('mount', function() {
    this.setInterval(this.tick, 1000);
  });

  this.tick = function() {
    this.setState({seconds: this.state.seconds + 1});
  };

  this.render = function() {
    return (
      <p>
        React has been running for {this.state.seconds} seconds.
      </p>
    );
  };
});

React.renderComponent(
  <TickTock />,
  document.getElementById('example')
);

With createClass, you can add mixins to your spec by adding objects to the mixins array. This is ugly. With defineClass, definitions are chained, similar to how middleware works in Node. Every definition is a mixin, and none are privileged. This is beautiful. Additionally, mixins are more powerful because they expose methods which their successors can call at definition time. I firmly believe that you can use defineClass to create more modular components.

Arguments against defineClass

1. Compatibility between createClass and defineClass

One could argue that React shouldn't have two ways to create components. This is a fair point, but I believe it's easy to convert definitions to specs and specs to definitions, for instance, by calling definitions with new. This is evident in the way defineClass is currently implemented, which is to use definitions to create a spec and then call createClass with that spec. This limits some of the abilities of defineClass but was done to get the code working quickly without touching core files.

2. Google Closure compatibility

Yeah, this is a problem. But the people who want closure compatibility the most are those Clojurescript freaks, who already have problems with using GOLs and inheritance. We shouldn't pre-optimize for Google Closure advanced compilations, and I have a couple ideas that might make defineClass Closure-compatible.

3. What is this?

One difficult thing to reason about is the this keyword within defineClass. Outside of methods, it represents the component in-transit, or the component as it is being built, so doing something like calling this.setState would result in an error. Inside methods however, this represents the completed and instantiated component. The mismatch between this outside and inside methods is troubling and mind-bending.

4. Safety

React has a complex system to make sure some properties aren't defined more than once, and that properties which are defined more than once using mixins do not overwrite each other. I'm firmly in the camp of 'giving developers enough rope to hang themselves with' but I have also shown in the pull request how we might approach this level of safety. Specifically, I prevent certain advice methods from being called on defineOnce methods (which seem special). There are other ways to prevent methods from being overwritten, for instance, by adding methods at definition time like 'overwrite' or 'safeMethod'.

TL;DR: defineClass makes some of the problems that React faces trivial, and could result in cleaner, more modular code. Thanks for your time. 💪

@brainkim
Copy link
Contributor Author

Gonna close this. This is more of a proof-of-concept than an actual pull request, and if I end up wanting this API I'll just implement in downstream. 💪 But, looking through the source, you guys could do with a solution to all the method wrapping that you're doing for testing/performance :P

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant