{"parents":["c3be0859100f065c3b94b3cf43d764cc8a79e05a"],"commit":{"subject":"Add some support for reordering","authorEmail":"dan.abramov@gmail.com","revision":"2794b92164058f38683cad49f04ff801e04ff34b","authorName":"Dan Abramov","shortRevision":"2794b92164","authoredAt":"2019-05-31T18:00:56+01:00"},"patch":"diff --git a\/src\/__tests__\/legacy\/__snapshots__\/storeLegacy-v15-test.js.snap b\/src\/__tests__\/legacy\/__snapshots__\/storeLegacy-v15-test.js.snap\nindex 37747be9c6..05e853c2ee 100644\n--- a\/src\/__tests__\/legacy\/__snapshots__\/storeLegacy-v15-test.js.snap\n+++ b\/src\/__tests__\/legacy\/__snapshots__\/storeLegacy-v15-test.js.snap\n@@ -15,6 +15,32 @@ exports[`Store (legacy) collapseNodesByDefault:false should filter DOM nodes fro\n

\n `;\n \n+exports[`Store (legacy) collapseNodesByDefault:false should support adding and removing children: 1: mount 1`] = `\n+[root]\n+ ▾ \n+ ▾

\n+ ▾ \n+

\n+`;\n+\n+exports[`Store (legacy) collapseNodesByDefault:false should support adding and removing children: 2: add child 1`] = `\n+[root]\n+ ▾ \n+ ▾

\n+ ▾ \n+

\n+`;\n+\n+exports[`Store (legacy) collapseNodesByDefault:false should support adding and removing children: 3: remove child 1`] = `\n+[root]\n+ ▾ \n+ ▾

\n+ ▾ \n+

\n+`;\n+\n exports[`Store (legacy) collapseNodesByDefault:false should support collapsing parts of the tree: 1: mount 1`] = `\n [root]\n ▾ \n@@ -185,6 +211,59 @@ exports[`Store (legacy) collapseNodesByDefault:false should support mount and up\n \n exports[`Store (legacy) collapseNodesByDefault:false should support mount and update operations: 3: unmount 1`] = ``;\n \n+exports[`Store (legacy) collapseNodesByDefault:false should support reordering of children: 1: mount 1`] = `\n+[root]\n+ ▾ \n+ ▾

\n+ ▾ \n+ ▾

\n+ ▾ \n+

\n+ ▾ \n+ ▾

\n+ ▾ \n+

\n+`;\n+\n+exports[`Store (legacy) collapseNodesByDefault:false should support reordering of children: 2: reorder children 1`] = `\n+[root]\n+ ▾ \n+ ▾

\n+ ▾ \n+ ▾

\n+ ▾ \n+

\n+ ▾ \n+ ▾

\n+ ▾ \n+

\n+`;\n+\n+exports[`Store (legacy) collapseNodesByDefault:false should support reordering of children: 3: collapse root 1`] = `\n+[root]\n+ ▸ \n+`;\n+\n+exports[`Store (legacy) collapseNodesByDefault:false should support reordering of children: 4: expand root 1`] = `\n+[root]\n+ ▾ \n+ ▾

\n+ ▾ \n+ ▾

\n+ ▾ \n+

\n+ ▾ \n+ ▾

\n+ ▾ \n+

\n+`;\n+\n exports[`Store (legacy) collapseNodesByDefault:true should not filter DOM nodes from the store tree: 1: mount 1`] = `\n [root]\n ▸ \ndiff --git a\/src\/__tests__\/legacy\/storeLegacy-v15-test.js b\/src\/__tests__\/legacy\/storeLegacy-v15-test.js\nindex bc891b3b75..96c8251d77 100644\n--- a\/src\/__tests__\/legacy\/storeLegacy-v15-test.js\n+++ b\/src\/__tests__\/legacy\/storeLegacy-v15-test.js\n@@ -176,10 +176,9 @@ describe('Store (legacy)', () => {\n expect(store).toMatchSnapshot('6: expand Grandparent');\n });\n \n- \/\/ TODO Re-enable this test once the renderer supports it.\n- xit('should support adding and removing children', () => {\n+ it('should support adding and removing children', () => {\n const Root = ({ children }) =>

{children}<\/div>;\n- const Component = () => null;\n+ const Component = () =>

;\n \n const container = document.createElement('div');\n \n@@ -215,10 +214,9 @@ describe('Store (legacy)', () => {\n expect(store).toMatchSnapshot('3: remove child');\n });\n \n- \/\/ TODO Re-enable this test once the renderer supports it.\n- xit('should support reordering of children', () => {\n+ it('should support reordering of children', () => {\n const Root = ({ children }) =>

{children}<\/div>;\n- const Component = () => null;\n+ const Component = () =>

;\n \n const Foo = () =>

{[]}<\/div>;\n const Bar = () => (\n@@ -447,10 +445,9 @@ describe('Store (legacy)', () => {\n expect(store).toMatchSnapshot('6: expand middle node');\n });\n \n- \/\/ TODO Re-enable this test once the renderer supports it.\n xit('should support reordering of children', () => {\n const Root = ({ children }) =>

{children}<\/div>;\n- const Component = () => null;\n+ const Component = () =>

;\n \n const Foo = () =>

{[]}<\/div>;\n const Bar = () => (\ndiff --git a\/src\/backend\/legacy\/renderer.js b\/src\/backend\/legacy\/renderer.js\nindex 0d9491e5da..2e6d65190c 100644\n--- a\/src\/backend\/legacy\/renderer.js\n+++ b\/src\/backend\/legacy\/renderer.js\n@@ -12,6 +12,7 @@ import {\n __DEBUG__,\n TREE_OPERATION_ADD,\n TREE_OPERATION_REMOVE,\n+ TREE_OPERATION_REORDER_CHILDREN,\n } from '..\/..\/constants';\n import getChildren from '.\/getChildren';\n import getData from '.\/getData';\n@@ -49,6 +50,10 @@ export function attach(\n const idToInternalInstanceMap: Map = new Map();\n const idToParentIDMap: Map = new Map();\n const internalInstanceToIDMap: Map = new Map();\n+ const internalInstanceToLastKnownChildrenMap: WeakMap<\n+ InternalInstance,\n+ Array\n+ > = new WeakMap();\n const rootIDs: Set = new Set();\n \n function getID(internalInstance: InternalInstance): number {\n@@ -217,6 +222,7 @@ export function attach(\n const result = fn.apply(this, args);\n currentParentID = prevParentID;\n \n+ recordPendingReorder(internalInstance);\n return result;\n },\n receiveComponent(fn, args) {\n@@ -227,6 +233,7 @@ export function attach(\n const result = fn.apply(this, args);\n currentParentID = prevParentID;\n \n+ recordPendingReorder(internalInstance);\n return result;\n },\n unmountComponent(fn, args) {\n@@ -261,6 +268,7 @@ export function attach(\n \n const pendingMountIDs: Set = new Set();\n const pendingUnmountIDs: Set = new Set();\n+ const pendingReorderIDs: Set = new Set();\n const pendingOperations: Array = [];\n const pendingStringTable: Map = new Map();\n let pendingStringTableLength: number = 0;\n@@ -303,7 +311,11 @@ export function attach(\n \/\/ It should be possible to improve this though, by maintaining a map of id-to-parent,\n \/\/ and crawling upward to the first non-filtered node.\n \/\/ TODO Revisit this and think about it more...\n- if (pendingMountIDs.size > 0 || pendingUnmountIDs.size > 0) {\n+ if (\n+ pendingMountIDs.size > 0 ||\n+ pendingUnmountIDs.size > 0 ||\n+ pendingReorderIDs.size > 0\n+ ) {\n rootIDs.forEach(flushPendingEvents);\n }\n }, 0);\n@@ -396,6 +408,7 @@ export function attach(\n const numUnmountIDs =\n unmountIDs.length + (pendingUnmountedRootID === null ? 0 : 1);\n \n+ const reorderOperations = computePendingReorderOperations();\n const operations = new Uint32Array(\n \/\/ Identify which renderer this update is coming from.\n 2 + \/\/ [rendererID, rootFiberID]\n@@ -406,8 +419,12 @@ export function attach(\n \/\/ All unmounts are batched in a single message.\n \/\/ [TREE_OPERATION_REMOVE, removedIDLength, ...ids]\n (numUnmountIDs > 0 ? 2 + numUnmountIDs : 0) +\n- \/\/ Mount\/update\/reorder operations\n- pendingOperations.length\n+ \/\/ Mount operations\n+ pendingOperations.length +\n+ \/\/ Reorder operation come last because\n+ \/\/ bridge expects them to not change children length.\n+ \/\/ So both mounts and unmounts need to have happened by now.\n+ reorderOperations.length\n );\n \n \/\/ Identify which renderer this update is coming from.\n@@ -444,6 +461,9 @@ export function attach(\n \n \/\/ Fill in the rest of the operations.\n operations.set(pendingOperations, i);\n+ i += pendingOperations.length;\n+\n+ operations.set(reorderOperations, i);\n \n if (__DEBUG__) {\n printOperationsArray(operations);\n@@ -454,12 +474,60 @@ export function attach(\n \n pendingOperations.length = 0;\n pendingMountIDs.clear();\n+ pendingReorderIDs.clear();\n pendingUnmountIDs.clear();\n pendingUnmountedRootID = null;\n pendingStringTable.clear();\n pendingStringTableLength = 0;\n }\n \n+ function computePendingReorderOperations(): Array {\n+ const ops = [];\n+ pendingReorderIDs.forEach(id => {\n+ const internalInstance = idToInternalInstanceMap.get(id);\n+ if (internalInstance === undefined) {\n+ return;\n+ }\n+ const prevChildIDs = internalInstanceToLastKnownChildrenMap.get(\n+ internalInstance\n+ );\n+ const nextChildIDs = getChildIDs(internalInstance);\n+ internalInstanceToLastKnownChildrenMap.set(\n+ internalInstance,\n+ nextChildIDs\n+ );\n+\n+ let shouldResetChildren = false;\n+ if (prevChildIDs === undefined) {\n+ \/\/ We haven't computed children before.\n+ \/\/ So we'll have to do it now.\n+ \/\/ Next time they'll be cached for comparison\n+ \/\/ TODO: this might not make sense. Revisit.\n+ shouldResetChildren = true;\n+ } else if (nextChildIDs.length > 1) {\n+ if (prevChildIDs.length !== nextChildIDs.length) {\n+ shouldResetChildren = true;\n+ } else {\n+ for (let i = 0; i < prevChildIDs.length; i++) {\n+ if (prevChildIDs[i] !== nextChildIDs[i]) {\n+ shouldResetChildren = true;\n+ break;\n+ }\n+ }\n+ }\n+ }\n+ if (shouldResetChildren) {\n+ ops.push(TREE_OPERATION_REORDER_CHILDREN);\n+ ops.push(getID(internalInstance));\n+ ops.push(nextChildIDs.length);\n+ for (let i = 0; i < nextChildIDs.length; i++) {\n+ ops.push(nextChildIDs[i]);\n+ }\n+ }\n+ });\n+ return ops;\n+ }\n+\n function getStringID(str: string | null): number {\n if (str === null) {\n return 0;\n@@ -652,6 +720,17 @@ export function attach(\n queueFlushPendingEvents();\n }\n \n+ function recordPendingReorder(internalInstance: InternalInstance) {\n+ const id = getID(internalInstance);\n+ pendingReorderIDs.add(id);\n+\n+ if (__DEBUG__) {\n+ console.log('%crecordPendingReorder()', 'color: green', id);\n+ }\n+\n+ queueFlushPendingEvents();\n+ }\n+\n function recordPendingUnmount(internalInstance: InternalInstance) {\n const id = getID(internalInstance);\n \n","body":"","files":[{"path":"src\/__tests__\/legacy\/__snapshots__\/storeLegacy-v15-test.js.snap","status":"M"},{"path":"src\/__tests__\/legacy\/storeLegacy-v15-test.js","status":"M"},{"path":"src\/backend\/legacy\/renderer.js","status":"M"}]}

dillon1000/react

Files changed