@samitouri / QOS-React-2 / commits / de1dacdb28

Refactor composite component update flow

Fixes #1392. Previously, ReactComponent.updateComponent set the new props and owner and updated the component's refs. Because it did the actual props assignment, it had to be called by ReactCompositeComponent between componentWillMount and componentDidMount, meaning that it was skipped if shouldComponentUpdate returned false. Now, updateComponent takes the old and new descriptors and only updates refs, allowing a subclass to call updateComponent at a convenient time (specifically, it can be before `this._descriptor` is updated if the subclass also overrides performUpdateIfNecessary). The new update cycle for composites is: - receiveComponent is called which stores `this._pendingDescriptor` and calls performUpdateIfNecessary directly or a state update is enqueued, in which case ReactUpdates will call performUpdateIfNecessary - performUpdateIfNecessary ensures that an update is still pending (which might no longer be the case if an update is queued for a subcomponent that was updated through a parent's reconciliation) and calls updateComponent with the old and new descriptors - updateComponent calls super to update refs, then calls componentWillReceiveProps (if applicable) and shouldComponentUpdate -- if shouldComponentUpdate returns false, `this._descriptor`, `this.props`, and `this.state` are updated and the update is skipped; else, _performComponentUpdate is called - _performComponentUpdate calls componentWillUpdate and _updateRenderedComponent, and enqueues the componentDidUpdate call (in that order) - _updateRenderedComponent calls render and updates the rendered component appropriately For DOM components (essentially unchanged): - receiveComponent is called which does a short-circuit check for descriptor equality and delegates to super (which stores `this._pendingDescriptor` and calls performUpdateIfNecessary) - performUpdateIfNecessary (not overridden) sets new values for `this.props`, etc. and calls updateComponent - updateComponent does the DOM property and children updates (some synchronously, some queued for the transaction close) For text and ART components (unchanged): - receiveComponent updates the DOM or ART directly based on the new props - updateComponent is never called Notable changes: - When shouldComponentUpdate returns false, refs are still updated - ReactDefaultPerf now includes lifecycle methods in component timings - updateComponent's signature changed (technically this is part of the public API, though we've never documented it) Test Plan: jest

Ben Alpert committed Jul 30, 2014 at 14:02 UTC de1dacdb2887349d09b839bb79d008ae0bf057b0
5 files changed +184 -109
src/browser/ReactTextComponent.js
+11
@@ -18,6 +18,7 @@ var ReactElement = require('ReactElement');
18
19 var assign = require('Object.assign');
20 var escapeTextForBrowser = require('escapeTextForBrowser');
21 +var invariant = require('invariant');
22
23 /**
24 * Text nodes violate a couple assumptions that React makes about components:
@@ -84,12 +85,22 @@ assign(ReactTextComponent.prototype, ReactComponent.Mixin, {
85 receiveComponent: function(nextComponent, transaction) {
86 var nextProps = nextComponent.props;
87 if (nextProps !== this.props) {
88 + // TODO: Save this as pending props and use performUpdateIfNecessary
89 + // and/or updateComponent to do the actual update for consistency with
90 + // other component types?
91 this.props = nextProps;
92 ReactComponent.BackendIDOperations.updateTextContentByID(
93 this._rootNodeID,
94 nextProps
95 );
96 }
97 + },
98 +
99 + updateComponent: function() {
100 + invariant(
101 + false,
102 + 'ReactTextComponent: updateComponent() should never be called'
103 + );
104 }
105
106 });
src/browser/ui/ReactDOMComponent.js
+4 -2
@@ -303,18 +303,20 @@ ReactDOMComponent.Mixin = {
303 *
304 * @param {ReactReconcileTransaction} transaction
305 * @param {ReactElement} prevElement
306 + * @param {ReactElement} nextElement
307 * @internal
308 * @overridable
309 */
310 updateComponent: ReactPerf.measure(
311 'ReactDOMComponent',
312 'updateComponent',
312 - function(transaction, prevElement) {
313 + function(transaction, prevElement, nextElement) {
314 assertValidProps(this._currentElement.props);
315 ReactComponent.Mixin.updateComponent.call(
316 this,
317 transaction,
317 - prevElement
318 + prevElement,
319 + nextElement
320 );
321 this._updateDOMProperties(prevElement.props, transaction);
322 this._updateDOMChildren(prevElement.props, transaction);
src/core/ReactComponent.js
+3 -4
@@ -323,7 +323,7 @@ var ReactComponent = {
323 this.props = nextElement.props;
324 this._owner = nextElement._owner;
325 this._pendingElement = null;
326 - this.updateComponent(transaction, prevElement);
326 + this.updateComponent(transaction, prevElement, nextElement);
327 },
328
329 /**
@@ -331,11 +331,10 @@ var ReactComponent = {
331 *
332 * @param {ReactReconcileTransaction} transaction
333 * @param {object} prevElement
334 + * @param {object} nextElement
335 * @internal
336 */
336 - updateComponent: function(transaction, prevElement) {
337 - var nextElement = this._currentElement;
338 -
337 + updateComponent: function(transaction, prevElement, nextElement) {
338 // If either the owner or a `ref` has changed, make sure the newest owner
339 // has stored a reference to `this`, and the previous owner (if different)
340 // has forgotten the reference to `this`. We use the element instead
src/core/ReactCompositeComponent.js
+130 -103
@@ -1006,6 +1006,26 @@ var ReactCompositeComponentMixin = {
1006 }
1007 },
1008
1009 + receiveComponent: function(nextElement, transaction) {
1010 + if (nextElement === this._currentElement &&
1011 + nextElement._owner != null) {
1012 + // Since elements are immutable after the owner is rendered,
1013 + // we can do a cheap identity compare here to determine if this is a
1014 + // superfluous reconcile. It's possible for state to be mutable but such
1015 + // change should trigger an update of the owner which would recreate
1016 + // the element. We explicitly check for the existence of an owner since
1017 + // it's possible for an element created outside a composite to be
1018 + // deeply mutated and reused.
1019 + return;
1020 + }
1021 +
1022 + ReactComponent.Mixin.receiveComponent.call(
1023 + this,
1024 + nextElement,
1025 + transaction
1026 + );
1027 + },
1028 +
1029 /**
1030 * If any of `_pendingElement`, `_pendingState`, or `_pendingForceUpdate`
1031 * is set, update the component.
@@ -1028,64 +1048,107 @@ var ReactCompositeComponentMixin = {
1048 return;
1049 }
1050
1031 - var nextContext = this.context;
1032 - var nextProps = this.props;
1033 - var nextElement = this._currentElement;
1051 + var prevElement = this._currentElement;
1052 + var nextElement = prevElement;
1053 if (this._pendingElement != null) {
1054 nextElement = this._pendingElement;
1036 - nextContext = this._processContext(nextElement._context);
1037 - nextProps = this._processProps(nextElement.props);
1055 this._pendingElement = null;
1056 + }
1057 +
1058 + this.updateComponent(
1059 + transaction,
1060 + prevElement,
1061 + nextElement
1062 + );
1063 + },
1064 +
1065 + /**
1066 + * Perform an update to a mounted component. The componentWillReceiveProps and
1067 + * shouldComponentUpdate methods are called, then (assuming the update isn't
1068 + * skipped) the remaining update lifecycle methods are called and the DOM
1069 + * representation is updated.
1070 + *
1071 + * By default, this implements React's rendering and reconciliation algorithm.
1072 + * Sophisticated clients may wish to override this.
1073 + *
1074 + * @param {ReactReconcileTransaction} transaction
1075 + * @param {ReactElement} prevParentElement
1076 + * @param {ReactElement} nextParentElement
1077 + * @internal
1078 + * @overridable
1079 + */
1080 + updateComponent: ReactPerf.measure(
1081 + 'ReactCompositeComponent',
1082 + 'updateComponent',
1083 + function(transaction, prevParentElement, nextParentElement) {
1084 + // Update refs regardless of what shouldComponentUpdate returns
1085 + ReactComponent.Mixin.updateComponent.call(
1086 + this,
1087 + transaction,
1088 + prevParentElement,
1089 + nextParentElement
1090 + );
1091
1040 - this._compositeLifeCycleState = CompositeLifeCycle.RECEIVING_PROPS;
1041 - if (this.componentWillReceiveProps) {
1042 - this.componentWillReceiveProps(nextProps, nextContext);
1092 + var prevContext = this.context;
1093 + var prevProps = this.props;
1094 + var nextContext = prevContext;
1095 + var nextProps = prevProps;
1096 + // Distinguish between a props update versus a simple state update
1097 + if (prevParentElement !== nextParentElement) {
1098 + nextContext = this._processContext(nextParentElement._context);
1099 + nextProps = this._processProps(nextParentElement.props);
1100 +
1101 + this._compositeLifeCycleState = CompositeLifeCycle.RECEIVING_PROPS;
1102 + if (this.componentWillReceiveProps) {
1103 + this.componentWillReceiveProps(nextProps, nextContext);
1104 + }
1105 }
1044 - }
1106
1046 - this._compositeLifeCycleState = null;
1107 + this._compositeLifeCycleState = null;
1108
1048 - var nextState = this._pendingState || this.state;
1049 - this._pendingState = null;
1109 + var nextState = this._pendingState || this.state;
1110 + this._pendingState = null;
1111
1051 - var shouldUpdate =
1052 - this._pendingForceUpdate ||
1053 - !this.shouldComponentUpdate ||
1054 - this.shouldComponentUpdate(nextProps, nextState, nextContext);
1112 + var shouldUpdate =
1113 + this._pendingForceUpdate ||
1114 + !this.shouldComponentUpdate ||
1115 + this.shouldComponentUpdate(nextProps, nextState, nextContext);
1116
1056 - if (__DEV__) {
1057 - if (typeof shouldUpdate === "undefined") {
1058 - console.warn(
1059 - (this.constructor.displayName || 'ReactCompositeComponent') +
1060 - '.shouldComponentUpdate(): Returned undefined instead of a ' +
1061 - 'boolean value. Make sure to return true or false.'
1062 - );
1117 + if (__DEV__) {
1118 + if (typeof shouldUpdate === "undefined") {
1119 + console.warn(
1120 + (this.constructor.displayName || 'ReactCompositeComponent') +
1121 + '.shouldComponentUpdate(): Returned undefined instead of a ' +
1122 + 'boolean value. Make sure to return true or false.'
1123 + );
1124 + }
1125 + }
1126 +
1127 + if (!shouldUpdate) {
1128 + // If it's determined that a component should not update, we still want
1129 + // to set props and state but we shortcut the rest of the update.
1130 + this._currentElement = nextParentElement;
1131 + this.props = nextProps;
1132 + this.state = nextState;
1133 + this.context = nextContext;
1134 +
1135 + // Owner cannot change because shouldUpdateReactComponent doesn't allow
1136 + // it. TODO: Remove this._owner completely.
1137 + this._owner = nextParentElement._owner;
1138 +
1139 + return;
1140 }
1064 - }
1141
1066 - if (shouldUpdate) {
1142 this._pendingForceUpdate = false;
1143 // Will set `this.props`, `this.state` and `this.context`.
1144 this._performComponentUpdate(
1070 - nextElement,
1145 + nextParentElement,
1146 nextProps,
1147 nextState,
1148 nextContext,
1149 transaction
1150 );
1076 - } else {
1077 - // If it's determined that a component should not update, we still want
1078 - // to set props and state.
1079 - this._currentElement = nextElement;
1080 - this.props = nextProps;
1081 - this.state = nextState;
1082 - this.context = nextContext;
1083 -
1084 - // Owner cannot change because shouldUpdateReactComponent doesn't allow
1085 - // it. TODO: Remove this._owner completely.
1086 - this._owner = nextElement._owner;
1087 - }
1088 - },
1151 + }),
1152
1153 /**
1154 * Merges new props and state, notifies delegate methods of update and
@@ -1105,7 +1168,6 @@ var ReactCompositeComponentMixin = {
1168 nextContext,
1169 transaction
1170 ) {
1108 - var prevElement = this._currentElement;
1171 var prevProps = this.props;
1172 var prevState = this.state;
1173 var prevContext = this.context;
@@ -1123,10 +1185,7 @@ var ReactCompositeComponentMixin = {
1185 // it. TODO: Remove this._owner completely.
1186 this._owner = nextElement._owner;
1187
1126 - this.updateComponent(
1127 - transaction,
1128 - prevElement
1129 - );
1188 + this._updateRenderedComponent(transaction);
1189
1190 if (this.componentDidUpdate) {
1191 transaction.getReactMountReady().enqueue(
@@ -1136,73 +1195,41 @@ var ReactCompositeComponentMixin = {
1195 }
1196 },
1197
1139 - receiveComponent: function(nextElement, transaction) {
1140 - if (nextElement === this._currentElement &&
1141 - nextElement._owner != null) {
1142 - // Since elements are immutable after the owner is rendered,
1143 - // we can do a cheap identity compare here to determine if this is a
1144 - // superfluous reconcile. It's possible for state to be mutable but such
1145 - // change should trigger an update of the owner which would recreate
1146 - // the element. We explicitly check for the existence of an owner since
1147 - // it's possible for a element created outside a composite to be
1148 - // deeply mutated and reused.
1149 - return;
1150 - }
1151 -
1152 - ReactComponent.Mixin.receiveComponent.call(
1153 - this,
1154 - nextElement,
1155 - transaction
1156 - );
1157 - },
1158 -
1198 /**
1160 - * Updates the component's currently mounted DOM representation.
1161 - *
1162 - * By default, this implements React's rendering and reconciliation algorithm.
1163 - * Sophisticated clients may wish to override this.
1199 + * Call the component's `render` method and update the DOM accordingly.
1200 *
1201 * @param {ReactReconcileTransaction} transaction
1166 - * @param {ReactElement} prevElement
1202 * @internal
1168 - * @overridable
1203 */
1170 - updateComponent: ReactPerf.measure(
1171 - 'ReactCompositeComponent',
1172 - 'updateComponent',
1173 - function(transaction, prevParentElement) {
1174 - ReactComponent.Mixin.updateComponent.call(
1175 - this,
1204 + _updateRenderedComponent: function(transaction) {
1205 + var prevComponentInstance = this._renderedComponent;
1206 + var prevRenderedElement = prevComponentInstance._currentElement;
1207 + var nextRenderedElement = this._renderValidatedComponent();
1208 + if (shouldUpdateReactComponent(prevRenderedElement, nextRenderedElement)) {
1209 + prevComponentInstance.receiveComponent(
1210 + nextRenderedElement,
1211 + transaction
1212 + );
1213 + } else {
1214 + // These two IDs are actually the same! But nothing should rely on that.
1215 + var thisID = this._rootNodeID;
1216 + var prevComponentID = prevComponentInstance._rootNodeID;
1217 + prevComponentInstance.unmountComponent();
1218 + this._renderedComponent = instantiateReactComponent(
1219 + nextRenderedElement,
1220 + this._currentElement.type
1221 + );
1222 + var nextMarkup = this._renderedComponent.mountComponent(
1223 + thisID,
1224 transaction,
1177 - prevParentElement
1225 + this._mountDepth + 1
1226 + );
1227 + ReactComponent.BackendIDOperations.dangerouslyReplaceNodeWithMarkupByID(
1228 + prevComponentID,
1229 + nextMarkup
1230 );
1179 -
1180 - var prevComponentInstance = this._renderedComponent;
1181 - var prevElement = prevComponentInstance._currentElement;
1182 - var nextElement = this._renderValidatedComponent();
1183 - if (shouldUpdateReactComponent(prevElement, nextElement)) {
1184 - prevComponentInstance.receiveComponent(nextElement, transaction);
1185 - } else {
1186 - // These two IDs are actually the same! But nothing should rely on that.
1187 - var thisID = this._rootNodeID;
1188 - var prevComponentID = prevComponentInstance._rootNodeID;
1189 - prevComponentInstance.unmountComponent();
1190 - this._renderedComponent = instantiateReactComponent(
1191 - nextElement,
1192 - this._currentElement.type
1193 - );
1194 - var nextMarkup = this._renderedComponent.mountComponent(
1195 - thisID,
1196 - transaction,
1197 - this._mountDepth + 1
1198 - );
1199 - ReactComponent.BackendIDOperations.dangerouslyReplaceNodeWithMarkupByID(
1200 - prevComponentID,
1201 - nextMarkup
1202 - );
1203 - }
1231 }
1205 - ),
1232 + },
1233
1234 /**
1235 * Forces an update. This should only be invoked when it is known with
src/core/__tests__/ReactCompositeComponent-test.js
+36
@@ -1483,4 +1483,40 @@ describe('ReactCompositeComponent', function() {
1483 );
1484 });
1485
1486 + it('should update refs if shouldComponentUpdate gives false', function() {
1487 + var Static = React.createClass({
1488 + shouldComponentUpdate: function() {
1489 + return false;
1490 + },
1491 + render: function() {
1492 + return <div>{this.props.children}</div>;
1493 + }
1494 + });
1495 + var Component = React.createClass({
1496 + render: function() {
1497 + if (this.props.flipped) {
1498 + return <div>
1499 + <Static ref="static0" key="B">B (ignored)</Static>
1500 + <Static ref="static1" key="A">A (ignored)</Static>
1501 + </div>;
1502 + } else {
1503 + return <div>
1504 + <Static ref="static0" key="A">A</Static>
1505 + <Static ref="static1" key="B">B</Static>
1506 + </div>;
1507 + }
1508 + }
1509 + });
1510 +
1511 + var comp = ReactTestUtils.renderIntoDocument(<Component flipped={false} />);
1512 + expect(comp.refs.static0.getDOMNode().textContent).toBe('A');
1513 + expect(comp.refs.static1.getDOMNode().textContent).toBe('B');
1514 +
1515 + // When flipping the order, the refs should update even though the actual
1516 + // contents do not
1517 + comp.setProps({flipped: true});
1518 + expect(comp.refs.static0.getDOMNode().textContent).toBe('B');
1519 + expect(comp.refs.static1.getDOMNode().textContent).toBe('A');
1520 + });
1521 +
1522 });