From 505110414c727d21627d80f2b2c346efe9fb02a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Greg=20Berg=C3=A9?= Date: Wed, 27 Sep 2017 10:50:24 +0200 Subject: [PATCH 1/4] Add test to reproduce props bug in React 16 --- .../enzyme-test-suite/test/Adapter-spec.jsx | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/packages/enzyme-test-suite/test/Adapter-spec.jsx b/packages/enzyme-test-suite/test/Adapter-spec.jsx index 77453299b..f9f980269 100644 --- a/packages/enzyme-test-suite/test/Adapter-spec.jsx +++ b/packages/enzyme-test-suite/test/Adapter-spec.jsx @@ -492,6 +492,39 @@ describe('Adapter', () => { }); }); + it('render node with updated props', () => { + const Dummy = () => null; + + class Counter extends React.Component { + constructor(props) { + super(props); + this.state = { count: 0 }; + } + + increment() { + this.setState({ count: this.state.count + 1 }); + } + + render() { + return ; + } + } + + const options = { mode: 'mount' }; + const renderer = adapter.createRenderer(options); + + renderer.render(); + + let tree = renderer.getNode(); + expect(tree.rendered.props.count).to.equal(0); + tree.instance.increment(); + tree = renderer.getNode(); + expect(tree.rendered.props.count).to.equal(1); + tree.instance.increment(); + tree = renderer.getNode(); + expect(tree.rendered.props.count).to.equal(2); + }); + it('renders basic shallow as well', () => { // eslint-disable-next-line react/require-render-return class Bar extends React.Component { From 4a3762a01000d789255cdfa37c42cc120e75e49f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Greg=20Berg=C3=A9?= Date: Fri, 29 Sep 2017 23:41:50 +0200 Subject: [PATCH 2/4] Fix node determinism in React v16 adapter Fix #1163 --- .../src/ReactSixteenAdapter.js | 7 +- .../src/findCurrentFiberUsingSlowPath.js | 103 ++++++++++++++++++ 2 files changed, 107 insertions(+), 3 deletions(-) create mode 100644 packages/enzyme-adapter-react-16/src/findCurrentFiberUsingSlowPath.js diff --git a/packages/enzyme-adapter-react-16/src/ReactSixteenAdapter.js b/packages/enzyme-adapter-react-16/src/ReactSixteenAdapter.js index c401f312d..476be87be 100644 --- a/packages/enzyme-adapter-react-16/src/ReactSixteenAdapter.js +++ b/packages/enzyme-adapter-react-16/src/ReactSixteenAdapter.js @@ -19,6 +19,7 @@ import { createMountWrapper, propsWithKeysAndRef, } from 'enzyme-adapter-utils'; +import findCurrentFiberUsingSlowPath from './findCurrentFiberUsingSlowPath'; const HostRoot = 3; const ClassComponent = 2; @@ -63,7 +64,7 @@ function toTree(vnode) { // TODO(lmr): I'm not really sure I understand whether or not this is what // i should be doing, or if this is a hack for something i'm doing wrong // somewhere else. Should talk to sebastian about this perhaps - const node = vnode.alternate !== null ? vnode.alternate : vnode; + const node = findCurrentFiberUsingSlowPath(vnode); switch (node.tag) { case HostRoot: // 3 return toTree(node.child); @@ -71,7 +72,7 @@ function toTree(vnode) { return { nodeType: 'class', type: node.type, - props: { ...vnode.memoizedProps }, + props: { ...node.memoizedProps }, key: node.key, ref: node.ref, instance: node.stateNode, @@ -83,7 +84,7 @@ function toTree(vnode) { return { nodeType: 'function', type: node.type, - props: { ...vnode.memoizedProps }, + props: { ...node.memoizedProps }, key: node.key, ref: node.ref, instance: null, diff --git a/packages/enzyme-adapter-react-16/src/findCurrentFiberUsingSlowPath.js b/packages/enzyme-adapter-react-16/src/findCurrentFiberUsingSlowPath.js new file mode 100644 index 000000000..13922da51 --- /dev/null +++ b/packages/enzyme-adapter-react-16/src/findCurrentFiberUsingSlowPath.js @@ -0,0 +1,103 @@ +function findCurrentFiberUsingSlowPath(fiber) { + const alternate = fiber.alternate; + if (!alternate) { + return fiber; + } + // If we have two possible branches, we'll walk backwards up to the root + // to see what path the root points to. On the way we may hit one of the + // special cases and we'll deal with them. + let a = fiber; + let b = alternate; + while (true) { // eslint-disable-line + const parentA = a.return; + const parentB = parentA ? parentA.alternate : null; + if (!parentA || !parentB) { + // We're at the root. + break; + } + + // If both copies of the parent fiber point to the same child, we can + // assume that the child is current. This happens when we bailout on low + // priority: the bailed out fiber's child reuses the current child. + if (parentA.child === parentB.child) { + let child = parentA.child; + while (child) { + if (child === a) { + // We've determined that A is the current branch. + return fiber; + } + if (child === b) { + // We've determined that B is the current branch. + return alternate; + } + child = child.sibling; + } + // We should never have an alternate for any mounting node. So the only + // way this could possibly happen is if this was unmounted, if at all. + throw new Error('Unable to find node on an unmounted component.'); + } + + if (a.return !== b.return) { + // The return pointer of A and the return pointer of B point to different + // fibers. We assume that return pointers never criss-cross, so A must + // belong to the child set of A.return, and B must belong to the child + // set of B.return. + a = parentA; + b = parentB; + } else { + // The return pointers point to the same fiber. We'll have to use the + // default, slow path: scan the child sets of each parent alternate to see + // which child belongs to which set. + // + // Search parent A's child set + let didFindChild = false; + let child = parentA.child; + while (child) { + if (child === a) { + didFindChild = true; + a = parentA; + b = parentB; + break; + } + if (child === b) { + didFindChild = true; + b = parentA; + a = parentB; + break; + } + child = child.sibling; + } + if (!didFindChild) { + // Search parent B's child set + child = parentB.child; + while (child) { + if (child === a) { + didFindChild = true; + a = parentB; + b = parentA; + break; + } + if (child === b) { + didFindChild = true; + b = parentB; + a = parentA; + break; + } + child = child.sibling; + } + if (!didFindChild) { + throw new Error('Child was not found in either parent set. This indicates a bug ' + + 'in React related to the return pointer. Please file an issue.'); + } + } + } + } + if (a.stateNode.current === a) { + // We've determined that A is the current branch. + return fiber; + } + // Otherwise B has to be current branch. + return alternate; +} + +module.exports = findCurrentFiberUsingSlowPath; From a302bfba1f64de4e1eb9a8d1dfb6b72546a298e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Greg=20Berg=C3=A9?= Date: Fri, 29 Sep 2017 23:43:43 +0200 Subject: [PATCH 3/4] Comment findCurrentFiberUsingSlowPath method --- .../enzyme-adapter-react-16/src/findCurrentFiberUsingSlowPath.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/enzyme-adapter-react-16/src/findCurrentFiberUsingSlowPath.js b/packages/enzyme-adapter-react-16/src/findCurrentFiberUsingSlowPath.js index 13922da51..53d2267e1 100644 --- a/packages/enzyme-adapter-react-16/src/findCurrentFiberUsingSlowPath.js +++ b/packages/enzyme-adapter-react-16/src/findCurrentFiberUsingSlowPath.js @@ -1,3 +1,4 @@ +// Extracted from https://github.com/facebook/react/blob/7bdf93b17a35a5d8fcf0ceae0bf48ed5e6b16688/src/renderers/shared/fiber/ReactFiberTreeReflection.js#L104-L228 function findCurrentFiberUsingSlowPath(fiber) { const alternate = fiber.alternate; if (!alternate) { From 8490f357281bfd3d90d017c06aa6977c669f6cb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Greg=20Berg=C3=A9?= Date: Sat, 30 Sep 2017 00:07:48 +0200 Subject: [PATCH 4/4] Do not use arrow function in tests (React 13, 14) --- packages/enzyme-test-suite/test/Adapter-spec.jsx | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/enzyme-test-suite/test/Adapter-spec.jsx b/packages/enzyme-test-suite/test/Adapter-spec.jsx index f9f980269..63ce98976 100644 --- a/packages/enzyme-test-suite/test/Adapter-spec.jsx +++ b/packages/enzyme-test-suite/test/Adapter-spec.jsx @@ -91,7 +91,7 @@ describe('Adapter', () => { hydratedTreeMatchesUnhydrated(); }); - it('treats mixed children correctlyf', () => { + it('treats mixed children correctly', () => { class Foo extends React.Component { render() { return ( @@ -493,7 +493,11 @@ describe('Adapter', () => { }); it('render node with updated props', () => { - const Dummy = () => null; + class Dummy extends React.Component { + render() { + return null; + } + } class Counter extends React.Component { constructor(props) {