From d09e66000dfd09e058f5d1f773ea77233d8df01a Mon Sep 17 00:00:00 2001 From: Mike Allanson Date: Wed, 2 May 2018 16:45:18 +0100 Subject: [PATCH 1/4] Change deleteNode signature to remove nodeId --- .../src/gatsby-node.js | 4 ++-- .../src/gatsby-node.js | 4 ++-- .../internal-data-bridge/gatsby-node.js | 2 +- .../query-runner/page-query-runner.js | 2 +- packages/gatsby/src/redux/__tests__/nodes.js | 8 +++---- packages/gatsby/src/redux/actions.js | 23 ++++++++++++------- packages/gatsby/src/redux/reducers/nodes.js | 2 +- packages/gatsby/src/utils/source-nodes.js | 2 +- 8 files changed, 27 insertions(+), 20 deletions(-) diff --git a/packages/gatsby-source-contentful/src/gatsby-node.js b/packages/gatsby-source-contentful/src/gatsby-node.js index 1a6ac2c8540a0..fbcfd90a4579d 100644 --- a/packages/gatsby-source-contentful/src/gatsby-node.js +++ b/packages/gatsby-source-contentful/src/gatsby-node.js @@ -68,8 +68,8 @@ exports.sourceNodes = async ( // Remove deleted entries & assets. // TODO figure out if entries referencing now deleted entries/assets // are "updated" so will get the now deleted reference removed. - currentSyncData.deletedEntries.forEach(e => deleteNode(e.sys.id, e.sys)) - currentSyncData.deletedAssets.forEach(e => deleteNode(e.sys.id, e.sys)) + currentSyncData.deletedEntries.forEach(e => deleteNode(e.sys)) + currentSyncData.deletedAssets.forEach(e => deleteNode(e.sys)) const existingNodes = getNodes().filter( n => n.internal.owner === `gatsby-source-contentful` diff --git a/packages/gatsby-source-filesystem/src/gatsby-node.js b/packages/gatsby-source-filesystem/src/gatsby-node.js index 19862f3476f84..5a0bd85866d79 100644 --- a/packages/gatsby-source-filesystem/src/gatsby-node.js +++ b/packages/gatsby-source-filesystem/src/gatsby-node.js @@ -188,7 +188,7 @@ See docs here - https://www.gatsbyjs.org/packages/gatsby-source-filesystem/ // write and then immediately delete temporary files to the file system. if (node) { currentState = fsMachine.transition(currentState.value, `EMIT_FS_EVENT`) - deleteNode(node.id, node) + deleteNode(node) } }) @@ -212,7 +212,7 @@ See docs here - https://www.gatsbyjs.org/packages/gatsby-source-filesystem/ reporter.info(`directory deleted at ${path}`) } const node = getNode(createNodeId(path)) - deleteNode(node.id, node) + deleteNode(node) }) return new Promise((resolve, reject) => { diff --git a/packages/gatsby/src/internal-plugins/internal-data-bridge/gatsby-node.js b/packages/gatsby/src/internal-plugins/internal-data-bridge/gatsby-node.js index fb92ecd4a2210..1a5693dbc95c4 100644 --- a/packages/gatsby/src/internal-plugins/internal-data-bridge/gatsby-node.js +++ b/packages/gatsby/src/internal-plugins/internal-data-bridge/gatsby-node.js @@ -168,5 +168,5 @@ exports.onCreatePage = ({ page, actions }) => { emitter.on(`DELETE_PAGE`, action => { const nodeId = createPageId(action.payload.path) const node = getNode(nodeId) - boundActionCreators.deleteNode(nodeId, node) + boundActionCreators.deleteNode(node) }) 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 index 26f49c98e9124..8dbf993dda41b 100644 --- a/packages/gatsby/src/internal-plugins/query-runner/page-query-runner.js +++ b/packages/gatsby/src/internal-plugins/query-runner/page-query-runner.js @@ -44,7 +44,7 @@ emitter.on(`CREATE_NODE`, action => { }) emitter.on(`DELETE_NODE`, action => { - queuedDirtyActions.push({ payload: action.node }) + queuedDirtyActions.push({ payload: action.payload }) }) const runQueuedActions = async () => { diff --git a/packages/gatsby/src/redux/__tests__/nodes.js b/packages/gatsby/src/redux/__tests__/nodes.js index 908957989f789..0f147e9317975 100644 --- a/packages/gatsby/src/redux/__tests__/nodes.js +++ b/packages/gatsby/src/redux/__tests__/nodes.js @@ -227,7 +227,8 @@ describe(`Create and update nodes`, () => { { name: `tests` } ) ) - store.dispatch(actions.deleteNode(`hi`, getNode(`hi`), { name: `tests` })) + + store.dispatch(actions.deleteNode(getNode(`hi`), { name: `tests` })) expect(Object.keys(store.getState().nodes).length).toEqual(1) }) @@ -315,8 +316,7 @@ describe(`Create and update nodes`, () => { }, { name: `tests` } ) - actions.deleteNode(`hi`, getNode(`hi`)) - + actions.deleteNode(getNode(`hi`)) expect(getNode(`hi`)).toBeUndefined() }) @@ -463,7 +463,7 @@ describe(`Create and update nodes`, () => { }) it(`does not crash when delete node is called on undefined`, () => { - actions.deleteNode(undefined, undefined, { name: `tests` }) + actions.deleteNode(undefined, { name: `tests` }) expect(Object.keys(store.getState().nodes).length).toEqual(0) }) }) diff --git a/packages/gatsby/src/redux/actions.js b/packages/gatsby/src/redux/actions.js index faed501784903..594a3d92e39fb 100644 --- a/packages/gatsby/src/redux/actions.js +++ b/packages/gatsby/src/redux/actions.js @@ -293,12 +293,20 @@ ${reservedFields.map(f => ` * "${f}"`).join(`\n`)} /** * Delete a node - * @param {string} nodeId a node id * @param {object} node the node object * @example - * deleteNode(node.id, node) + * deleteNode(node) */ -actions.deleteNode = (nodeId: string, node: any, plugin: Plugin) => { +actions.deleteNode = (node: any, plugin: Plugin) => { + if (typeof node === `string`) { + console.log( + `The "deleteNode" action no longer accepts a nodeId string. Please pass a full node object instead.` + ) + if (plugin && plugin.name) { + console.log(`"deleteNode" was called by ${plugin.name}`) + } + } + let deleteDescendantsActions // It's possible the file node was never created as sometimes tools will // write and then immediately delete temporary files to the file system. @@ -307,7 +315,7 @@ actions.deleteNode = (nodeId: string, node: any, plugin: Plugin) => { const descendantNodes = findChildrenRecursively(node.children) if (descendantNodes.length > 0) { deleteDescendantsActions = descendantNodes.map(n => - actions.deleteNode(n, getNode(n), plugin) + actions.deleteNode(getNode(n), plugin) ) } } @@ -315,8 +323,7 @@ actions.deleteNode = (nodeId: string, node: any, plugin: Plugin) => { const deleteAction = { type: `DELETE_NODE`, plugin, - node, - payload: nodeId, + payload: node, } if (deleteDescendantsActions) { @@ -334,7 +341,7 @@ actions.deleteNode = (nodeId: string, node: any, plugin: Plugin) => { */ actions.deleteNodes = (nodes: any[], plugin: Plugin) => { console.log( - `The "deleteNodes" is now deprecated and will be removed in Gatsby v3. Please use "deleteNode" instead` + `The "deleteNodes" action is now deprecated and will be removed in Gatsby v3. Please use "deleteNode" instead.` ) if (plugin && plugin.name) { console.log(`"deleteNodes" was called by ${plugin.name}`) @@ -347,7 +354,7 @@ actions.deleteNodes = (nodes: any[], plugin: Plugin) => { let deleteDescendantsActions if (descendantNodes.length > 0) { deleteDescendantsActions = descendantNodes.map(n => - actions.deleteNode(n, getNode(n), plugin) + actions.deleteNode(getNode(n), plugin) ) } diff --git a/packages/gatsby/src/redux/reducers/nodes.js b/packages/gatsby/src/redux/reducers/nodes.js index b8346d049de56..e375682ec4211 100644 --- a/packages/gatsby/src/redux/reducers/nodes.js +++ b/packages/gatsby/src/redux/reducers/nodes.js @@ -22,7 +22,7 @@ module.exports = (state = {}, action) => { return newState case `DELETE_NODE`: { - newState = _.omit(state, action.payload) + newState = _.omit(state, action.payload.id) return newState } diff --git a/packages/gatsby/src/utils/source-nodes.js b/packages/gatsby/src/utils/source-nodes.js index 46dc5c625b4b8..6d60db616cafb 100644 --- a/packages/gatsby/src/utils/source-nodes.js +++ b/packages/gatsby/src/utils/source-nodes.js @@ -67,6 +67,6 @@ module.exports = async () => { }) if (staleNodes.length > 0) { - staleNodes.forEach(n => deleteNode(n.id, n)) + staleNodes.forEach(n => deleteNode(n)) } } From 1e13cf15e9f73da09185f8fea2350d7c81208e50 Mon Sep 17 00:00:00 2001 From: Mike Allanson Date: Thu, 10 May 2018 13:21:24 +0100 Subject: [PATCH 2/4] These tests were failing? --- packages/gatsby/cache-dir/loader.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/gatsby/cache-dir/loader.js b/packages/gatsby/cache-dir/loader.js index 09829da8a7098..b66c7cc299530 100644 --- a/packages/gatsby/cache-dir/loader.js +++ b/packages/gatsby/cache-dir/loader.js @@ -174,8 +174,8 @@ const queue = { }, addPagesArray: newPages => { - if (__PREFIX_PATHS__) { - pathPrefix = `${__PATH_PREFIX__}/` + if (typeof __PREFIX_PATHS__ !== `undefined`) { + pathPrefix = typeof __PATH_PREFIX__ !== `undefined` ? `${__PATH_PREFIX__}/` : `/` } findPage = pageFinderFactory(newPages, pathPrefix.slice(0, -1)) }, From 5bbf282ea4fa5b959e72dc8899865cdb91285463 Mon Sep 17 00:00:00 2001 From: Mike Allanson Date: Thu, 10 May 2018 15:16:10 +0100 Subject: [PATCH 3/4] Deprecate old deleteNode method signature --- .../src/gatsby-node.js | 4 +- .../src/gatsby-node.js | 4 +- .../internal-data-bridge/gatsby-node.js | 2 +- .../__tests__/__snapshots__/nodes.js.snap | 13 + packages/gatsby/src/redux/__tests__/nodes.js | 682 +++++++++--------- packages/gatsby/src/redux/actions.js | 28 +- packages/gatsby/src/utils/source-nodes.js | 2 +- 7 files changed, 385 insertions(+), 350 deletions(-) diff --git a/packages/gatsby-source-contentful/src/gatsby-node.js b/packages/gatsby-source-contentful/src/gatsby-node.js index fbcfd90a4579d..4eca31d173fad 100644 --- a/packages/gatsby-source-contentful/src/gatsby-node.js +++ b/packages/gatsby-source-contentful/src/gatsby-node.js @@ -68,8 +68,8 @@ exports.sourceNodes = async ( // Remove deleted entries & assets. // TODO figure out if entries referencing now deleted entries/assets // are "updated" so will get the now deleted reference removed. - currentSyncData.deletedEntries.forEach(e => deleteNode(e.sys)) - currentSyncData.deletedAssets.forEach(e => deleteNode(e.sys)) + currentSyncData.deletedEntries.forEach(e => deleteNode({ node: e.sys })) + currentSyncData.deletedAssets.forEach(e => deleteNode({ node: e.sys })) const existingNodes = getNodes().filter( n => n.internal.owner === `gatsby-source-contentful` diff --git a/packages/gatsby-source-filesystem/src/gatsby-node.js b/packages/gatsby-source-filesystem/src/gatsby-node.js index 5a0bd85866d79..f6659093bee54 100644 --- a/packages/gatsby-source-filesystem/src/gatsby-node.js +++ b/packages/gatsby-source-filesystem/src/gatsby-node.js @@ -188,7 +188,7 @@ See docs here - https://www.gatsbyjs.org/packages/gatsby-source-filesystem/ // write and then immediately delete temporary files to the file system. if (node) { currentState = fsMachine.transition(currentState.value, `EMIT_FS_EVENT`) - deleteNode(node) + deleteNode({ node }) } }) @@ -212,7 +212,7 @@ See docs here - https://www.gatsbyjs.org/packages/gatsby-source-filesystem/ reporter.info(`directory deleted at ${path}`) } const node = getNode(createNodeId(path)) - deleteNode(node) + deleteNode({ node }) }) return new Promise((resolve, reject) => { diff --git a/packages/gatsby/src/internal-plugins/internal-data-bridge/gatsby-node.js b/packages/gatsby/src/internal-plugins/internal-data-bridge/gatsby-node.js index 1a5693dbc95c4..408f03e850f8e 100644 --- a/packages/gatsby/src/internal-plugins/internal-data-bridge/gatsby-node.js +++ b/packages/gatsby/src/internal-plugins/internal-data-bridge/gatsby-node.js @@ -168,5 +168,5 @@ exports.onCreatePage = ({ page, actions }) => { emitter.on(`DELETE_PAGE`, action => { const nodeId = createPageId(action.payload.path) const node = getNode(nodeId) - boundActionCreators.deleteNode(node) + boundActionCreators.deleteNode({ node }) }) diff --git a/packages/gatsby/src/redux/__tests__/__snapshots__/nodes.js.snap b/packages/gatsby/src/redux/__tests__/__snapshots__/nodes.js.snap index ccb238c7a9c3e..9b98163e97175 100644 --- a/packages/gatsby/src/redux/__tests__/__snapshots__/nodes.js.snap +++ b/packages/gatsby/src/redux/__tests__/__snapshots__/nodes.js.snap @@ -127,3 +127,16 @@ exports[`Create and update nodes throws error if a node sets a value on "fields" \\"name\\": \\"pluginA\\" }" `; + +exports[`Create and update nodes warns when using old deleteNode signature 1`] = ` +Object { + "children": Array [], + "id": "hi", + "internal": Object { + "contentDigest": "hasdfljds", + "owner": "tests", + "type": "Test", + }, + "parent": "test", +} +`; diff --git a/packages/gatsby/src/redux/__tests__/nodes.js b/packages/gatsby/src/redux/__tests__/nodes.js index 0f147e9317975..2404a45b062f2 100644 --- a/packages/gatsby/src/redux/__tests__/nodes.js +++ b/packages/gatsby/src/redux/__tests__/nodes.js @@ -1,63 +1,73 @@ -const { actions } = require(`../actions`) -const { store, getNode } = require(`../index`) +const { + actions, +} = require(`../actions`) +const { + store, + getNode, +} = require(`../index`) const nodeReducer = require(`../reducers/nodes`) const nodeTouchedReducer = require(`../reducers/nodes-touched`) describe(`Create and update nodes`, () => { + beforeEach(() => { + store.dispatch({ + type: `DELETE_CACHE`, + }) + }) + it(`allows creating nodes`, () => { - const action = actions.createNode( - { - id: `hi`, - children: [], - parent: `test`, - internal: { - contentDigest: `hasdfljds`, - type: `Test`, - }, - pickle: true, + const action = actions.createNode({ + id: `hi`, + children: [], + parent: `test`, + internal: { + contentDigest: `hasdfljds`, + type: `Test`, }, - { name: `tests` } - ) + pickle: true, + }, { + name: `tests`, + }) expect(action).toMatchSnapshot() expect(nodeReducer(undefined, action)).toMatchSnapshot() }) it(`allows updating nodes`, () => { - const action = actions.createNode( - { - id: `hi`, - children: [], - parent: `test`, - internal: { - contentDigest: `hasdfljds`, - type: `Test`, - }, - pickle: true, - deep: { - array: [0, 1, { boom: true }], - }, + const action = actions.createNode({ + id: `hi`, + children: [], + parent: `test`, + internal: { + contentDigest: `hasdfljds`, + type: `Test`, }, - { name: `tests` } - ) - const updateAction = actions.createNode( - { - id: `hi`, - children: [], - parent: `test`, - internal: { - contentDigest: `hasdfljds`, - type: `Test`, - }, - pickle: false, - deep: { - array: [1, 2], - }, - deep2: { - boom: `foo`, - }, + pickle: true, + deep: { + array: [0, 1, { + boom: true, + }], }, - { name: `tests` } - ) + }, { + name: `tests`, + }) + const updateAction = actions.createNode({ + id: `hi`, + children: [], + parent: `test`, + internal: { + contentDigest: `hasdfljds`, + type: `Test`, + }, + pickle: false, + deep: { + array: [1, 2], + }, + deep2: { + boom: `foo`, + }, + }, { + name: `tests`, + }) let state = nodeReducer(undefined, action) state = nodeReducer(state, updateAction) expect(state[`hi`].pickle).toEqual(false) @@ -66,349 +76,367 @@ describe(`Create and update nodes`, () => { }) it(`deletes previously transformed children nodes when the parent node is updated`, () => { - store.dispatch({ type: `DELETE_CACHE` }) - store.dispatch( - actions.createNode( - { - id: `hi`, - children: [], - parent: null, - internal: { - contentDigest: `hasdfljds`, - type: `Test`, - }, + actions.createNode({ + id: `hi`, + children: [], + parent: null, + internal: { + contentDigest: `hasdfljds`, + type: `Test`, }, - { name: `tests` } - ) + }, { + name: `tests`, + }) ) store.dispatch( - actions.createNode( - { - id: `hi-1`, - children: [], - parent: `hi`, - internal: { - contentDigest: `hasdfljds-1`, - type: `Test-1`, - }, + actions.createNode({ + id: `hi-1`, + children: [], + parent: `hi`, + internal: { + contentDigest: `hasdfljds-1`, + type: `Test-1`, }, - { name: `tests` } - ) + }, { + name: `tests`, + }) ) store.dispatch( - actions.createParentChildLink( - { - parent: store.getState().nodes[`hi`], - child: store.getState().nodes[`hi-1`], - }, - { name: `tests` } - ) + actions.createParentChildLink({ + parent: store.getState().nodes[`hi`], + child: store.getState().nodes[`hi-1`], + }, { + name: `tests`, + }) ) store.dispatch( - actions.createNode( - { - id: `hi-1-1`, - children: [], - parent: `hi-1`, - internal: { - contentDigest: `hasdfljds-1-1`, - type: `Test-1-1`, - }, + actions.createNode({ + id: `hi-1-1`, + children: [], + parent: `hi-1`, + internal: { + contentDigest: `hasdfljds-1-1`, + type: `Test-1-1`, }, - { name: `tests` } - ) + }, { + name: `tests`, + }) ) store.dispatch( - actions.createParentChildLink( - { - parent: store.getState().nodes[`hi-1`], - child: store.getState().nodes[`hi-1-1`], - }, - { name: `tests` } - ) + actions.createParentChildLink({ + parent: store.getState().nodes[`hi-1`], + child: store.getState().nodes[`hi-1-1`], + }, { + name: `tests`, + }) ) store.dispatch( - actions.createNode( - { - id: `hi`, - children: [], - parent: `test`, - internal: { - contentDigest: `hasdfljds2`, - type: `Test`, - }, + actions.createNode({ + id: `hi`, + children: [], + parent: `test`, + internal: { + contentDigest: `hasdfljds2`, + type: `Test`, }, - { name: `tests` } - ) + }, { + name: `tests`, + }) ) expect(Object.keys(store.getState().nodes).length).toEqual(1) }) it(`deletes previously transformed children nodes when the parent node is deleted`, () => { - store.dispatch({ type: `DELETE_CACHE` }) - store.dispatch( - actions.createNode( - { - id: `hi`, - children: [], - parent: `test`, - internal: { - contentDigest: `hasdfljds`, - type: `Test`, - }, + actions.createNode({ + id: `hi`, + children: [], + parent: `test`, + internal: { + contentDigest: `hasdfljds`, + type: `Test`, }, - { name: `tests` } - ) + }, { + name: `tests`, + }) ) store.dispatch( - actions.createNode( - { - id: `hi2`, - children: [], - parent: `test`, - internal: { - contentDigest: `hasdfljds`, - type: `Test`, - }, + actions.createNode({ + id: `hi2`, + children: [], + parent: `test`, + internal: { + contentDigest: `hasdfljds`, + type: `Test`, }, - { name: `tests` } - ) + }, { + name: `tests`, + }) ) store.dispatch( - actions.createNode( - { - id: `hi-1`, - children: [], - parent: `hi`, - internal: { - contentDigest: `hasdfljds-1`, - type: `Test-1`, - }, + actions.createNode({ + id: `hi-1`, + children: [], + parent: `hi`, + internal: { + contentDigest: `hasdfljds-1`, + type: `Test-1`, }, - { name: `tests` } - ) + }, { + name: `tests`, + }) ) store.dispatch( - actions.createParentChildLink( - { - parent: store.getState().nodes[`hi`], - child: getNode(`hi-1`), - }, - { name: `tests` } - ) + actions.createParentChildLink({ + parent: store.getState().nodes[`hi`], + child: getNode(`hi-1`), + }, { + name: `tests`, + }) ) store.dispatch( - actions.createNode( - { - id: `hi-1-1`, - children: [], - parent: `hi-1`, - internal: { - contentDigest: `hasdfljds-1-1`, - type: `Test-1-1`, - }, + actions.createNode({ + id: `hi-1-1`, + children: [], + parent: `hi-1`, + internal: { + contentDigest: `hasdfljds-1-1`, + type: `Test-1-1`, }, - { name: `tests` } - ) + }, { + name: `tests`, + }) ) store.dispatch( - actions.createParentChildLink( - { - parent: getNode(`hi-1`), - child: getNode(`hi-1-1`), - }, - { name: `tests` } - ) + actions.createParentChildLink({ + parent: getNode(`hi-1`), + child: getNode(`hi-1-1`), + }, { + name: `tests`, + }) ) - store.dispatch(actions.deleteNode(getNode(`hi`), { name: `tests` })) + store.dispatch(actions.deleteNode({ + node: getNode(`hi`), + }, { + name: `tests`, + })) expect(Object.keys(store.getState().nodes).length).toEqual(1) }) it(`deletes previously transformed children nodes when parent nodes are deleted`, () => { - store.dispatch({ type: `DELETE_CACHE` }) - store.dispatch( - actions.createNode( - { - id: `hi`, - children: [], - parent: `test`, - internal: { - contentDigest: `hasdfljds`, - type: `Test`, - }, - }, - { name: `tests` } - ) - ) + store.dispatch(actions.createNode({ + id: `hi`, + children: [], + parent: `test`, + internal: { + contentDigest: `hasdfljds`, + type: `Test`, + }, + }, { + name: `tests`, + })) store.dispatch( - actions.createNode( - { - id: `hi-1`, - children: [], - parent: `hi`, - internal: { - contentDigest: `hasdfljds-1`, - type: `Test-1`, - }, + actions.createNode({ + id: `hi-1`, + children: [], + parent: `hi`, + internal: { + contentDigest: `hasdfljds-1`, + type: `Test-1`, }, - { name: `tests` } - ) + }, { + name: `tests`, + }) ) store.dispatch( - actions.createParentChildLink( - { - parent: getNode(`hi`), - child: getNode(`hi-1`), - }, - { name: `tests` } - ) + actions.createParentChildLink({ + parent: getNode(`hi`), + child: getNode(`hi-1`), + }, { + name: `tests`, + }) ) store.dispatch( - actions.createNode( - { - id: `hi-1-1`, - children: [], - parent: `hi-1`, - internal: { - contentDigest: `hasdfljds-1-1`, - type: `Test-1-1`, - }, + actions.createNode({ + id: `hi-1-1`, + children: [], + parent: `hi-1`, + internal: { + contentDigest: `hasdfljds-1-1`, + type: `Test-1-1`, }, - { name: `tests` } - ) + }, { + name: `tests`, + }) ) store.dispatch( - actions.createParentChildLink( - { - parent: getNode(`hi-1`), - child: getNode(`hi-1-1`), - }, - { name: `tests` } - ) + actions.createParentChildLink({ + parent: getNode(`hi-1`), + child: getNode(`hi-1-1`), + }, { + name: `tests`, + }) ) - store.dispatch(actions.deleteNodes([`hi`], { name: `tests` })) + store.dispatch(actions.deleteNodes([`hi`], { + name: `tests`, + })) expect(Object.keys(store.getState().nodes).length).toEqual(0) }) it(`allows deleting nodes`, () => { - store.dispatch({ type: `DELETE_CACHE` }) - actions.createNode( - { - id: `hi`, - children: [], - parent: `test`, - internal: { - contentDigest: `hasdfljds`, - type: `Test`, - }, - pickle: true, - deep: { - array: [0, 1, { boom: true }], - }, + actions.createNode({ + id: `hi`, + children: [], + parent: `test`, + internal: { + contentDigest: `hasdfljds`, + type: `Test`, }, - { name: `tests` } - ) - actions.deleteNode(getNode(`hi`)) + pickle: true, + deep: { + array: [0, 1, { + boom: true, + }], + }, + }, { + name: `tests`, + }) + actions.deleteNode({ + node: getNode(`hi`), + }) + expect(getNode(`hi`)).toBeUndefined() + }) + + it(`warns when using old deleteNode signature `, () => { + console.warn = jest.fn() + store.dispatch(actions.createNode({ + id: `hi`, + children: [], + parent: `test`, + internal: { + contentDigest: `hasdfljds`, + type: `Test`, + }, + }, { + name: `tests`, + })) + + expect(getNode(`hi`)).toMatchSnapshot() + store.dispatch(actions.deleteNode(`hi`, getNode(`hi`), { + name: `tests`, + })) + expect(getNode(`hi`)).toBeUndefined() + + const deprecationNotice = `Calling "deleteNode" with a nodeId is deprecated. Please pass an object containing a full node instead: deleteNode({ node })` + expect(console.warn).toHaveBeenCalledWith(deprecationNotice) + + console.warn.mockRestore() }) it(`nodes that are added are also "touched"`, () => { - const action = actions.createNode( - { - id: `hi`, - children: [], - parent: `test`, - internal: { - contentDigest: `hasdfljds`, - type: `Test`, - }, - pickle: true, + const action = actions.createNode({ + id: `hi`, + children: [], + parent: `test`, + internal: { + contentDigest: `hasdfljds`, + type: `Test`, }, - { name: `tests` } - ) + pickle: true, + }, { + name: `tests`, + }) let state = nodeTouchedReducer(undefined, action) expect(state[`hi`]).toBe(true) }) it(`allows adding fields to nodes`, () => { - const action = actions.createNode( - { - id: `hi`, - children: [], - parent: `test`, - internal: { - contentDigest: `hasdfljds`, - type: `Test`, - }, - pickle: true, + const action = actions.createNode({ + id: `hi`, + children: [], + parent: `test`, + internal: { + contentDigest: `hasdfljds`, + type: `Test`, }, - { name: `tests` } - ) + pickle: true, + }, { + name: `tests`, + }) let state = nodeReducer(undefined, action) - const addFieldAction = actions.createNodeField( - { - node: state[`hi`], - name: `joy`, - value: `soul's delight`, - }, - { name: `test` } - ) + const addFieldAction = actions.createNodeField({ + node: state[`hi`], + name: `joy`, + value: `soul's delight`, + }, { + name: `test`, + }) state = nodeReducer(state, addFieldAction) expect(state).toMatchSnapshot() }) it(`throws error if a field is updated by a plugin not its owner`, () => { - const action = actions.createNode( - { - id: `hi`, - children: [], - parent: `test`, - internal: { - contentDigest: `hasdfljds`, - type: `Test`, - }, - pickle: true, + const action = actions.createNode({ + id: `hi`, + children: [], + parent: `test`, + internal: { + contentDigest: `hasdfljds`, + type: `Test`, }, - { name: `tests` } - ) + pickle: true, + }, { + name: `tests`, + }) let state = nodeReducer(undefined, action) - const addFieldAction = actions.createNodeField( - { - node: state[`hi`], - name: `joy`, - value: `soul's delight`, - }, - { name: `test` } - ) + const addFieldAction = actions.createNodeField({ + node: state[`hi`], + name: `joy`, + value: `soul's delight`, + }, { + name: `test`, + }) state = nodeReducer(state, addFieldAction) function callActionCreator() { - actions.createNodeField( - { - node: state[`hi`], - name: `joy`, - value: `soul's delight`, - }, - { name: `test2` } - ) + actions.createNodeField({ + node: state[`hi`], + name: `joy`, + value: `soul's delight`, + }, { + name: `test2`, + }) } expect(callActionCreator).toThrowErrorMatchingSnapshot() }) it(`throws error if a node is created by a plugin not its owner`, () => { - actions.createNode( - { - id: `hi`, + actions.createNode({ + id: `hi`, + children: [], + parent: `test`, + internal: { + contentDigest: `hasdfljds`, + type: `mineOnly`, + }, + pickle: true, + }, { + name: `pluginA`, + }) + + function callActionCreator() { + actions.createNode({ + id: `hi2`, children: [], parent: `test`, internal: { @@ -416,24 +444,9 @@ describe(`Create and update nodes`, () => { type: `mineOnly`, }, pickle: true, - }, - { name: `pluginA` } - ) - - function callActionCreator() { - actions.createNode( - { - id: `hi2`, - children: [], - parent: `test`, - internal: { - contentDigest: `hasdfljds`, - type: `mineOnly`, - }, - pickle: true, - }, - { name: `pluginB` } - ) + }, { + name: `pluginB`, + }) } expect(callActionCreator).toThrowErrorMatchingSnapshot() @@ -441,29 +454,30 @@ describe(`Create and update nodes`, () => { it(`throws error if a node sets a value on "fields"`, () => { function callActionCreator() { - actions.createNode( - { - id: `hi`, - children: [], - parent: `test`, - fields: { - test: `I can't do this but I like to test boundaries`, - }, - internal: { - contentDigest: `hasdfljds`, - type: `mineOnly`, - }, - pickle: true, + actions.createNode({ + id: `hi`, + children: [], + parent: `test`, + fields: { + test: `I can't do this but I like to test boundaries`, }, - { name: `pluginA` } - ) + internal: { + contentDigest: `hasdfljds`, + type: `mineOnly`, + }, + pickle: true, + }, { + name: `pluginA`, + }) } expect(callActionCreator).toThrowErrorMatchingSnapshot() }) it(`does not crash when delete node is called on undefined`, () => { - actions.deleteNode(undefined, { name: `tests` }) + actions.deleteNode(undefined, { + name: `tests`, + }) expect(Object.keys(store.getState().nodes).length).toEqual(0) }) }) diff --git a/packages/gatsby/src/redux/actions.js b/packages/gatsby/src/redux/actions.js index 594a3d92e39fb..30ed6e8c121ee 100644 --- a/packages/gatsby/src/redux/actions.js +++ b/packages/gatsby/src/redux/actions.js @@ -293,18 +293,26 @@ ${reservedFields.map(f => ` * "${f}"`).join(`\n`)} /** * Delete a node - * @param {object} node the node object + * @param {object} $0 + * @param {object} $0.node the node object * @example - * deleteNode(node) + * deleteNode({node: node}) */ -actions.deleteNode = (node: any, plugin: Plugin) => { - if (typeof node === `string`) { - console.log( - `The "deleteNode" action no longer accepts a nodeId string. Please pass a full node object instead.` +actions.deleteNode = (options: any, plugin: Plugin, ...args) => { + let node = _.get(options, `node`) + + // Check if using old method signature. Warn about incorrect usage but get + // node from nodeID anyway. + if (typeof options === `string`) { + console.warn( + `Calling "deleteNode" with a nodeId is deprecated. Please pass an object containing a full node instead: deleteNode({ node })` ) - if (plugin && plugin.name) { - console.log(`"deleteNode" was called by ${plugin.name}`) + + if (args[0] && args[0].name) { // `plugin` used to be the third argument + console.log(`"deleteNode" was called by ${args[0].name}`) } + + node = getNode(options) } let deleteDescendantsActions @@ -315,7 +323,7 @@ actions.deleteNode = (node: any, plugin: Plugin) => { const descendantNodes = findChildrenRecursively(node.children) if (descendantNodes.length > 0) { deleteDescendantsActions = descendantNodes.map(n => - actions.deleteNode(getNode(n), plugin) + actions.deleteNode({ node: getNode(n) }, plugin) ) } } @@ -354,7 +362,7 @@ actions.deleteNodes = (nodes: any[], plugin: Plugin) => { let deleteDescendantsActions if (descendantNodes.length > 0) { deleteDescendantsActions = descendantNodes.map(n => - actions.deleteNode(getNode(n), plugin) + actions.deleteNode({ node: getNode(n) }, plugin) ) } diff --git a/packages/gatsby/src/utils/source-nodes.js b/packages/gatsby/src/utils/source-nodes.js index 6d60db616cafb..232776ea020be 100644 --- a/packages/gatsby/src/utils/source-nodes.js +++ b/packages/gatsby/src/utils/source-nodes.js @@ -67,6 +67,6 @@ module.exports = async () => { }) if (staleNodes.length > 0) { - staleNodes.forEach(n => deleteNode(n)) + staleNodes.forEach(node => deleteNode({ node })) } } From 49cedd1f1ca92500451b771a2e20a107591bda3a Mon Sep 17 00:00:00 2001 From: Mike Allanson Date: Thu, 10 May 2018 16:20:37 +0100 Subject: [PATCH 4/4] Change touchNode to accept an object --- .../src/gatsby-node.js | 2 +- .../gatsby-source-drupal/src/gatsby-node.js | 2 +- .../gatsby-source-wordpress/src/normalize.js | 2 +- .../src/gatsby-node.js | 2 +- packages/gatsby/src/redux/actions.js | 22 ++++++++++++++++--- 5 files changed, 23 insertions(+), 7 deletions(-) diff --git a/packages/gatsby-source-contentful/src/gatsby-node.js b/packages/gatsby-source-contentful/src/gatsby-node.js index 4eca31d173fad..e9485350dd791 100644 --- a/packages/gatsby-source-contentful/src/gatsby-node.js +++ b/packages/gatsby-source-contentful/src/gatsby-node.js @@ -74,7 +74,7 @@ exports.sourceNodes = async ( const existingNodes = getNodes().filter( n => n.internal.owner === `gatsby-source-contentful` ) - existingNodes.forEach(n => touchNode(n.id)) + existingNodes.forEach(n => touchNode({ nodeId: n.id })) const assets = currentSyncData.assets diff --git a/packages/gatsby-source-drupal/src/gatsby-node.js b/packages/gatsby-source-drupal/src/gatsby-node.js index 81fdc3ec4a5cb..43996b95c5fc0 100644 --- a/packages/gatsby-source-drupal/src/gatsby-node.js +++ b/packages/gatsby-source-drupal/src/gatsby-node.js @@ -24,7 +24,7 @@ exports.sourceNodes = async ( // Touch existing Drupal nodes so Gatsby doesn't garbage collect them. // _.values(store.getState().nodes) // .filter(n => n.internal.type.slice(0, 8) === `drupal__`) - // .forEach(n => touchNode(n.id)) + // .forEach(n => touchNode({ nodeId: n.id })) // Fetch articles. // console.time(`fetch Drupal data`) diff --git a/packages/gatsby-source-wordpress/src/normalize.js b/packages/gatsby-source-wordpress/src/normalize.js index ab768d3b8821c..4eb55ddcf9c1d 100644 --- a/packages/gatsby-source-wordpress/src/normalize.js +++ b/packages/gatsby-source-wordpress/src/normalize.js @@ -408,7 +408,7 @@ exports.downloadMediaFiles = async ({ // previously created file node to not try to redownload if (cacheMediaData && e.modified === cacheMediaData.modified) { fileNodeID = cacheMediaData.fileNodeID - touchNode(cacheMediaData.fileNodeID) + touchNode({ nodeId: cacheMediaData.fileNodeID }) } // If we don't have cached data, download the file diff --git a/packages/gatsby-transformer-screenshot/src/gatsby-node.js b/packages/gatsby-transformer-screenshot/src/gatsby-node.js index 3eb55f0860e30..90c1c23579fe0 100644 --- a/packages/gatsby-transformer-screenshot/src/gatsby-node.js +++ b/packages/gatsby-transformer-screenshot/src/gatsby-node.js @@ -35,7 +35,7 @@ exports.onPreBootstrap = ( } else { // Screenshot hasn't yet expired, touch the image node // to prevent garbage collection - touchNode(n.screenshotFile___NODE) + touchNode({ nodeId: n.screenshotFile___NODE }) } }) ) diff --git a/packages/gatsby/src/redux/actions.js b/packages/gatsby/src/redux/actions.js index 30ed6e8c121ee..1c3b15f6ea7f9 100644 --- a/packages/gatsby/src/redux/actions.js +++ b/packages/gatsby/src/redux/actions.js @@ -587,11 +587,27 @@ actions.createNode = (node: any, plugin?: Plugin, traceId?: string) => { * nodes from a remote system that can return only nodes that have * updated. The source plugin then touches all the nodes that haven't * updated but still exist so Gatsby knows to keep them. - * @param {string} nodeId The id of a node. + * @param {Object} $0 + * @param {string} $0.nodeId The id of a node * @example - * touchNode(`a-node-id`) + * touchNode({ nodeId: `a-node-id` }) */ -actions.touchNode = (nodeId: string, plugin?: Plugin) => { +actions.touchNode = (options: any, plugin?: Plugin) => { + let nodeId = _.get(options, `nodeId`) + + // Check if using old method signature. Warn about incorrect usage + if (typeof options === `string`) { + console.warn( + `Calling "touchNode" with a nodeId is deprecated. Please pass an object containing a nodeId instead: touchNode({ nodeId: 'a-node-id' })` + ) + + if (plugin && plugin.name) { + console.log(`"touchNode" was called by ${plugin.name}`) + } + + nodeId = options + } + return { type: `TOUCH_NODE`, plugin,