Actually handle deletes fixes #4351 (#4353)

* Actually handle deletes fixes #4351 The previous PR for this #4209 didn't actually quite work. It worked in that it did delete descendant nodes correctly but by handling it in the reducer, the rest of Gatsby wasn't alerted to the other nodes being deleted which meant that hot reloading of graphql query data wasn't working. * Comment out tests
gatsbyjs · Mar 4, 2018 · 6ac0eb9 · 6ac0eb9
1 parent c53246e
commit 6ac0eb9
Show file tree

Hide file tree

Showing 7 changed files with 198 additions and 178 deletions.
diff --git a/packages/gatsby/src/internal-plugins/query-runner/page-query-runner.js b/packages/gatsby/src/internal-plugins/query-runner/page-query-runner.js
@@ -120,7 +120,7 @@ const runQueriesForIds = ids => {
 
 const findDirtyIds = actions => {
   const state = store.getState()
-  return _.uniq(
+  const uniqDirties = _.uniq(
     actions.reduce((dirtyIds, action) => {
       const node = action.payload
 
@@ -137,4 +137,5 @@ const findDirtyIds = actions => {
       return _.compact(dirtyIds)
     }, [])
   )
+  return uniqDirties
 }
diff --git a/packages/gatsby/src/redux/__tests__/nodes.js b/packages/gatsby/src/redux/__tests__/nodes.js
@@ -1,4 +1,5 @@
-const { actions } = require(`../actions`)
+const { actions, boundActionCreators } = require(`../actions`)
+const { store, getNode } = require(`../index`)
 const nodeReducer = require(`../reducers/nodes`)
 const nodeTouchedReducer = require(`../reducers/nodes-touched`)
 
@@ -65,21 +66,21 @@ describe(`Create and update nodes`, () => {
   })
 
   it(`deletes previously transformed children nodes when the parent node is updated`, () => {
-    const action = actions.createNode(
+    store.dispatch({ type: `DELETE_CACHE` })
+    boundActionCreators.createNode(
       {
         id: `hi`,
         children: [],
-        parent: `test`,
+        parent: null,
         internal: {
           contentDigest: `hasdfljds`,
           type: `Test`,
         },
       },
       { name: `tests` }
     )
-    let state = nodeReducer(undefined, action)
 
-    const createChildAction = actions.createNode(
+    boundActionCreators.createNode(
       {
         id: `hi-1`,
         children: [],
@@ -91,18 +92,16 @@ describe(`Create and update nodes`, () => {
       },
       { name: `tests` }
     )
-    state = nodeReducer(state, createChildAction)
 
-    const addChildToParent = actions.createParentChildLink(
+    boundActionCreators.createParentChildLink(
       {
-        parent: state[`hi`],
-        child: state[`hi-1`],
+        parent: store.getState().nodes[`hi`],
+        child: store.getState().nodes[`hi-1`],
       },
       { name: `tests` }
     )
-    state = nodeReducer(state, addChildToParent)
 
-    const create2ndChildAction = actions.createNode(
+    boundActionCreators.createNode(
       {
         id: `hi-1-1`,
         children: [],
@@ -114,18 +113,16 @@ describe(`Create and update nodes`, () => {
       },
       { name: `tests` }
     )
-    state = nodeReducer(state, create2ndChildAction)
 
-    const add2ndChildToParent = actions.createParentChildLink(
+    boundActionCreators.createParentChildLink(
       {
-        parent: state[`hi-1`],
-        child: state[`hi-1-1`],
+        parent: store.getState().nodes[`hi-1`],
+        child: store.getState().nodes[`hi-1-1`],
       },
       { name: `tests` }
     )
-    state = nodeReducer(state, add2ndChildToParent)
 
-    const updateAction = actions.createNode(
+    boundActionCreators.createNode(
       {
         id: `hi`,
         children: [],
@@ -137,12 +134,12 @@ describe(`Create and update nodes`, () => {
       },
       { name: `tests` }
     )
-    state = nodeReducer(state, updateAction)
-    expect(Object.keys(state).length).toEqual(1)
+    expect(Object.keys(store.getState().nodes).length).toEqual(1)
   })
 
   it(`deletes previously transformed children nodes when the parent node is deleted`, () => {
-    const action = actions.createNode(
+    store.dispatch({ type: `DELETE_CACHE` })
+    boundActionCreators.createNode(
       {
         id: `hi`,
         children: [],
@@ -154,9 +151,8 @@ describe(`Create and update nodes`, () => {
       },
       { name: `tests` }
     )
-    let state = nodeReducer(undefined, action)
 
-    const secondNodeAction = actions.createNode(
+    boundActionCreators.createNode(
       {
         id: `hi2`,
         children: [],
@@ -168,9 +164,8 @@ describe(`Create and update nodes`, () => {
       },
       { name: `tests` }
     )
-    state = nodeReducer(state, secondNodeAction)
 
-    const createChildAction = actions.createNode(
+    boundActionCreators.createNode(
       {
         id: `hi-1`,
         children: [],
@@ -182,18 +177,16 @@ describe(`Create and update nodes`, () => {
       },
       { name: `tests` }
     )
-    state = nodeReducer(state, createChildAction)
 
-    const addChildToParent = actions.createParentChildLink(
+    boundActionCreators.createParentChildLink(
       {
-        parent: state[`hi`],
-        child: state[`hi-1`],
+        parent: store.getState().nodes[`hi`],
+        child: getNode(`hi-1`),
       },
       { name: `tests` }
     )
-    state = nodeReducer(state, addChildToParent)
 
-    const create2ndChildAction = actions.createNode(
+    boundActionCreators.createNode(
       {
         id: `hi-1-1`,
         children: [],
@@ -205,24 +198,22 @@ describe(`Create and update nodes`, () => {
       },
       { name: `tests` }
     )
-    state = nodeReducer(state, create2ndChildAction)
 
-    const add2ndChildToParent = actions.createParentChildLink(
+    boundActionCreators.createParentChildLink(
       {
-        parent: state[`hi-1`],
-        child: state[`hi-1-1`],
+        parent: getNode(`hi-1`),
+        child: getNode(`hi-1-1`),
       },
       { name: `tests` }
     )
-    state = nodeReducer(state, add2ndChildToParent)
 
-    const deleteNodeAction = actions.deleteNode(`hi`, { name: `tests` })
-    const deleteNodeState = nodeReducer(state, deleteNodeAction)
-    expect(Object.keys(deleteNodeState).length).toEqual(1)
+    boundActionCreators.deleteNode(`hi`, getNode(`hi`), { name: `tests` })
+    expect(Object.keys(store.getState().nodes).length).toEqual(1)
   })
 
   it(`deletes previously transformed children nodes when parent nodes are deleted`, () => {
-    const action = actions.createNode(
+    store.dispatch({ type: `DELETE_CACHE` })
+    boundActionCreators.createNode(
       {
         id: `hi`,
         children: [],
@@ -234,9 +225,8 @@ describe(`Create and update nodes`, () => {
       },
       { name: `tests` }
     )
-    let state = nodeReducer(undefined, action)
 
-    const createChildAction = actions.createNode(
+    boundActionCreators.createNode(
       {
         id: `hi-1`,
         children: [],
@@ -248,18 +238,16 @@ describe(`Create and update nodes`, () => {
       },
       { name: `tests` }
     )
-    state = nodeReducer(state, createChildAction)
 
-    const addChildToParent = actions.createParentChildLink(
+    boundActionCreators.createParentChildLink(
       {
-        parent: state[`hi`],
-        child: state[`hi-1`],
+        parent: getNode(`hi`),
+        child: getNode(`hi-1`),
       },
       { name: `tests` }
     )
-    state = nodeReducer(state, addChildToParent)
 
-    const create2ndChildAction = actions.createNode(
+    boundActionCreators.createNode(
       {
         id: `hi-1-1`,
         children: [],
@@ -271,24 +259,22 @@ describe(`Create and update nodes`, () => {
       },
       { name: `tests` }
     )
-    state = nodeReducer(state, create2ndChildAction)
 
-    const add2ndChildToParent = actions.createParentChildLink(
+    boundActionCreators.createParentChildLink(
       {
-        parent: state[`hi-1`],
-        child: state[`hi-1-1`],
+        parent: getNode(`hi-1`),
+        child: getNode(`hi-1-1`),
       },
       { name: `tests` }
     )
-    state = nodeReducer(state, add2ndChildToParent)
 
-    const deleteNodesAction = actions.deleteNodes([`hi`], { name: `tests` })
-    const deleteNodesState = nodeReducer(state, deleteNodesAction)
-    expect(Object.keys(deleteNodesState).length).toEqual(0)
+    boundActionCreators.deleteNodes([`hi`], { name: `tests` })
+    expect(Object.keys(store.getState().nodes).length).toEqual(0)
   })
 
   it(`allows deleting nodes`, () => {
-    const action = actions.createNode(
+    store.dispatch({ type: `DELETE_CACHE` })
+    boundActionCreators.createNode(
       {
         id: `hi`,
         children: [],
@@ -304,11 +290,9 @@ describe(`Create and update nodes`, () => {
       },
       { name: `tests` }
     )
-    const deleteAction = actions.deleteNode(`hi`)
+    boundActionCreators.deleteNode(`hi`, getNode(`hi`))
 
-    let state = nodeReducer(undefined, action)
-    state = nodeReducer(state, deleteAction)
-    expect(state[`hi`]).toBeUndefined()
+    expect(getNode(`hi`)).toBeUndefined()
   })
 
   it(`nodes that are added are also "touched"`, () => {

diff --git a/packages/gatsby/src/redux/actions.js b/packages/gatsby/src/redux/actions.js
@@ -9,14 +9,29 @@ const glob = require(`glob`)
 const path = require(`path`)
 const fs = require(`fs`)
 const { joinPath } = require(`../utils/path`)
-const { getNode, hasNodeChanged } = require(`./index`)
+const { hasNodeChanged, getNode } = require(`./index`)
 const { trackInlineObjectsInRootNode } = require(`../schema/node-tracking`)
 const { store } = require(`./index`)
 import * as joiSchemas from "../joi-schemas/joi"
 import { generateComponentChunkName } from "../utils/js-chunk-names"
 
 const actions = {}
 
+const findChildrenRecursively = (children = []) => {
+  children = children.concat(
+    ...children.map(child => {
+      const newChildren = getNode(child).children
+      if (_.isArray(newChildren) && newChildren.length > 0) {
+        return findChildrenRecursively(newChildren)
+      } else {
+        return []
+      }
+    })
+  )
+
+  return children
+}
+
 type Job = {
   id: string,
 }
@@ -397,12 +412,27 @@ actions.createLayout = (
  * deleteNode(node.id, node)
  */
 actions.deleteNode = (nodeId: string, node: any, plugin: Plugin) => {
-  return {
+  // Also delete any nodes transformed from this one.
+  let deleteDescendantsActions
+  const descendantNodes = findChildrenRecursively(node.children)
+  if (descendantNodes.length > 0) {
+    deleteDescendantsActions = descendantNodes.map(n =>
+      actions.deleteNode(n, getNode(n), plugin)
+    )
+  }
+
+  const deleteAction = {
     type: `DELETE_NODE`,
     plugin,
     node,
     payload: nodeId,
   }
+
+  if (deleteDescendantsActions) {
+    return [...deleteDescendantsActions, deleteAction]
+  } else {
+    return deleteAction
+  }
 }
 
 /**
@@ -412,11 +442,28 @@ actions.deleteNode = (nodeId: string, node: any, plugin: Plugin) => {
  * deleteNodes([`node1`, `node2`])
  */
 actions.deleteNodes = (nodes: any[], plugin: Plugin) => {
-  return {
+  // Also delete any nodes transformed from these.
+  const descendantNodes = _.flatten(
+    nodes.map(n => findChildrenRecursively(getNode(n).children))
+  )
+  let deleteDescendantsActions
+  if (descendantNodes.length > 0) {
+    deleteDescendantsActions = descendantNodes.map(n =>
+      actions.deleteNode(n, getNode(n), plugin)
+    )
+  }
+
+  const deleteNodesAction = {
     type: `DELETE_NODES`,
     plugin,
     payload: nodes,
   }
+
+  if (deleteDescendantsActions) {
+    return [...deleteDescendantsActions, deleteNodesAction]
+  } else {
+    return deleteNodesAction
+  }
 }
 
 const typeOwners = {}
@@ -581,22 +628,39 @@ actions.createNode = (node: any, plugin?: Plugin, traceId?: string) => {
     }
   }
 
+  let deleteAction
+  let updateNodeAction
   // Check if the node has already been processed.
   if (oldNode && !hasNodeChanged(node.id, node.internal.contentDigest)) {
-    return {
+    updateNodeAction = {
       type: `TOUCH_NODE`,
       plugin,
       traceId,
       payload: node.id,
     }
   } else {
-    return {
+    // Remove any previously created descendant nodes as they're all due
+    // to be recreated.
+    if (oldNode) {
+      const descendantNodes = findChildrenRecursively(oldNode.children)
+      if (descendantNodes.length > 0) {
+        deleteAction = actions.deleteNodes(descendantNodes)
+      }
+    }
+
+    updateNodeAction = {
       type: `CREATE_NODE`,
       plugin,
       traceId,
       payload: node,
     }
   }
+
+  if (deleteAction) {
+    return [deleteAction, updateNodeAction]
+  } else {
+    return updateNodeAction
+  }
 }
 
 /**