You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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.
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)
functionAdviceDefinition(){'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 */varSetIntervalMixin={componentWillMount: function(){this.intervals=[];},setInterval: function(){this.intervals.push(setInterval.apply(null,arguments));},componentWillUnmount: function(){this.intervals.map(clearInterval);}};varTickTock=React.createClass({mixins: [SetIntervalMixin],// Use the mixingetInitialState: 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 */functionSetIntervalDefinition(){this.before('mount',function(){this.intervals=[];});this.setInterval=function(){this.intervals.push(setInterval.apply(null,arguments));};this.before('unmount',function(){this.intervals.map(clearInterval);});}varTickTock=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. 💪
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Warning: lot of code!
This pull request adds a
defineClassmethod to addons similar to Twitter Flight'sdefineComponent. 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 ofReactCompositeComponentto prevent duplicated code.