@samitouri / QOS-React / commits / 3717b71c64

Resolve ref callbacks

During the deletion phase we call detachRefs on any deleted refs. During the insertion/update phase we call attachRef on class and host components. Unfortunately, when a ref switches, we are supposed to call all the unmounts before doing any mounts. This means that we have to expact the deletion phase to also include updates in case they need to detach their ref.

Sebastian Markbage committed Oct 7, 2016 at 15:24 UTC 3717b71c6444f813d44e10f7f42fe4e2c8f31023
4 files changed +115 -12
src/renderers/shared/fiber/ReactChildFiber.js
+5
@@ -221,11 +221,13 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
221 if (current == null || current.type !== element.type) {
222 // Insert
223 const created = createFiberFromElement(element, priority);
224 + created.ref = element.ref;
225 created.return = returnFiber;
226 return created;
227 } else {
228 // Move based on index
229 const existing = useFiber(current, priority);
230 + existing.ref = element.ref;
231 existing.pendingProps = element.props;
232 existing.return = returnFiber;
233 return existing;
@@ -317,6 +319,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
319 switch (newChild.$$typeof) {
320 case REACT_ELEMENT_TYPE: {
321 const created = createFiberFromElement(newChild, priority);
322 + created.ref = newChild.ref;
323 created.return = returnFiber;
324 return created;
325 }
@@ -643,6 +646,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
646 if (child.type === element.type) {
647 deleteRemainingChildren(returnFiber, child.sibling);
648 const existing = useFiber(child, priority);
649 + existing.ref = element.ref;
650 existing.pendingProps = element.props;
651 existing.return = returnFiber;
652 return existing;
@@ -657,6 +661,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
661 }
662
663 const created = createFiberFromElement(element, priority);
664 + created.ref = element.ref;
665 created.return = returnFiber;
666 return created;
667 }
src/renderers/shared/fiber/ReactFiberCommitWork.js
+40 -12
@@ -40,6 +40,29 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
40 const insertBefore = config.insertBefore;
41 const removeChild = config.removeChild;
42
43 + function detachRef(current : Fiber) {
44 + const ref = current.ref;
45 + if (ref) {
46 + ref(null);
47 + }
48 + }
49 +
50 + function attachRef(current : ?Fiber, finishedWork : Fiber, instance : any) {
51 + const ref = finishedWork.ref;
52 + if (current) {
53 + const currentRef = current.ref;
54 + if (currentRef && currentRef !== ref) {
55 + // TODO: This needs to be done in a separate pass before any other refs
56 + // gets resolved. Otherwise we might invoke them in the wrong order
57 + // when the same ref switches between two components.
58 + currentRef(null);
59 + }
60 + }
61 + if (ref) {
62 + ref(instance);
63 + }
64 + }
65 +
66 function getHostParent(fiber : Fiber) : ?I {
67 let parent = fiber.return;
68 while (parent) {
@@ -163,9 +186,6 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
186 // Recursively delete all host nodes from the parent.
187 // TODO: Error handling.
188 const parent = getHostParent(current);
166 - if (!parent) {
167 - return;
168 - }
189 // We only have the top Fiber that was inserted but we need recurse down its
190 // children to find all the terminal nodes.
191 // TODO: Call componentWillUnmount on all classes as needed. Recurse down
@@ -177,7 +197,9 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
197 commitNestedUnmounts(node);
198 // After all the children have unmounted, it is now safe to remove the
199 // node from the tree.
180 - removeChild(parent, node.stateNode);
200 + if (parent) {
201 + removeChild(parent, node.stateNode);
202 + }
203 } else {
204 commitUnmount(node);
205 if (node.child) {
@@ -202,10 +224,16 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
224 function commitUnmount(current : Fiber) : void {
225 switch (current.tag) {
226 case ClassComponent: {
227 + detachRef(current);
228 const instance = current.stateNode;
229 if (typeof instance.componentWillUnmount === 'function') {
230 instance.componentWillUnmount();
231 }
232 + return;
233 + }
234 + case HostComponent: {
235 + detachRef(current);
236 + return;
237 }
238 }
239 }
@@ -239,7 +267,7 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
267 finishedWork.callbackList = null;
268 callCallbacks(callbackList, instance);
269 }
242 - // TODO: Fire update refs
270 + attachRef(current, finishedWork, instance);
271 return;
272 }
273 case HostContainer: {
@@ -251,14 +279,14 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
279 return;
280 }
281 case HostComponent: {
254 - if (finishedWork.stateNode == null || !current) {
255 - throw new Error('This should only be done during updates.');
256 - }
257 - // Commit the work prepared earlier.
258 - const newProps = finishedWork.memoizedProps;
259 - const oldProps = current.memoizedProps;
282 const instance : I = finishedWork.stateNode;
261 - commitUpdate(instance, oldProps, newProps);
283 + if (instance != null && current) {
284 + // Commit the work prepared earlier.
285 + const newProps = finishedWork.memoizedProps;
286 + const oldProps = current.memoizedProps;
287 + commitUpdate(instance, oldProps, newProps);
288 + }
289 + attachRef(current, finishedWork, instance);
290 return;
291 }
292 case HostText: {
src/renderers/shared/fiber/ReactFiberCompleteWork.js
+4
@@ -175,6 +175,10 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
175 // TODO: This seems like unnecessary duplication.
176 workInProgress.stateNode = instance;
177 workInProgress.output = instance;
178 + if (workInProgress.ref) {
179 + // If there is a ref on a host node we need to schedule a callback
180 + markUpdate(workInProgress);
181 + }
182 }
183 workInProgress.memoizedProps = newProps;
184 return null;
src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js
+66
@@ -637,6 +637,72 @@ describe('ReactIncrementalSideEffects', () => {
637
638 });
639
640 + it('invokes ref callbacks after insertion/update/unmount', () => {
641 +
642 + var classInstance = null;
643 +
644 + var ops = [];
645 +
646 + class ClassComponent extends React.Component {
647 + render() {
648 + classInstance = this;
649 + return <span />;
650 + }
651 + }
652 +
653 + function FunctionalComponent(props) {
654 + return <span />;
655 + }
656 +
657 + function Foo(props) {
658 + return (
659 + props.show ?
660 + <div>
661 + <ClassComponent ref={n => ops.push(n)} />
662 + <FunctionalComponent ref={n => ops.push(n)} />
663 + <div ref={n => ops.push(n)} />
664 + </div> :
665 + null
666 + );
667 + }
668 +
669 + ReactNoop.render(<Foo show={true} />);
670 + ReactNoop.flush();
671 + expect(ops).toEqual([
672 + classInstance,
673 + // no call for functional components
674 + div(),
675 + ]);
676 +
677 + ops = [];
678 +
679 + // Refs that switch function instances get reinvoked
680 + ReactNoop.render(<Foo show={true} />);
681 + ReactNoop.flush();
682 + expect(ops).toEqual([
683 + // TODO: All detach should happen first. Currently they're interleaved.
684 + // detach
685 + null,
686 + // reattach
687 + classInstance,
688 + // detach
689 + null,
690 + // reattach
691 + div(),
692 + ]);
693 +
694 + ops = [];
695 +
696 + ReactNoop.render(<Foo show={false} />);
697 + ReactNoop.flush();
698 + expect(ops).toEqual([
699 + // unmount
700 + null,
701 + null,
702 + ]);
703 +
704 + });
705 +
706 // TODO: Test that mounts, updates, refs, unmounts and deletions happen in the
707 // expected way for aborted and resumed render life-cycles.
708