Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(gatsby): Remove deleteNode with ID arg #29205

Merged
merged 10 commits into from
Feb 10, 2021
6 changes: 2 additions & 4 deletions docs/docs/creating-a-source-plugin.md
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ exports.createSchemaCustomization = ({ actions }) => {
author: Author @link(from: "author.name" by: "name") // highlight-line
# ... other fields
}

type Author implements Node {
name: String!
post: Post @link // highlight-line
Expand Down Expand Up @@ -446,9 +446,7 @@ exports.sourceNodes = async ({ actions, getNodesByType }, pluginOptions) => {
newData.forEach(newDatum => {
switch (newDatum.status) {
case "deleted":
deleteNode({
node: getNode(createNodeId(`YourSourceType-${newDatum.uuid}`)),
})
deleteNode(getNode(createNodeId(`YourSourceType-${newDatum.uuid}`)))
break
case "created":
case "updated":
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -887,9 +887,7 @@ exports.sourceNodes = async (
const nodeId = createNodeId(`${POST_NODE_TYPE}-${post.id}`)
switch (post.status) {
case "deleted":
deleteNode({
node: getNode(nodeId),
})
deleteNode(getNode(nodeId))
break
case "created":
case "updated":
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,7 @@ exports.sourceNodes = async function sourceNodes({
deleted.forEach(node => {
const existing = getNode(node.id)
if (existing) {
deleteNode({
node: existing,
})
deleteNode(existing)
}
})
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,7 @@ exports.sourceNodes = async function sourceNodes({
deleted.forEach(node => {
const existing = getNode(node.id)
if (existing) {
deleteNode({
node: existing,
})
deleteNode(existing)
}
})
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,9 +179,7 @@ exports.sourceNodes = async function sourceNodes(
const nodeId = createNodeId(`${POST_NODE_TYPE}-${post.id}`)
switch (post.status) {
case "deleted":
deleteNode({
node: getNode(nodeId),
})
deleteNode(getNode(nodeId))
break
case "created":
case "updated":
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ describe(`gatsby-node`, () => {
// don't allow mutations (this isn't traversing so only top level is frozen)
currentNodeMap.set(node.id, Object.freeze(node))
})
actions.deleteNode = ({ node }) => {
actions.deleteNode = node => {
currentNodeMap.delete(node.id)
}
actions.touchNode = jest.fn()
Expand Down
2 changes: 1 addition & 1 deletion packages/gatsby-source-contentful/src/gatsby-node.js
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,7 @@ exports.sourceNodes = async (
localizedNodes.forEach(node => {
// touchNode first, to populate typeOwners & avoid erroring
touchNode({ nodeId: node.id })
deleteNode({ node })
deleteNode(node)
})
}

Expand Down
6 changes: 3 additions & 3 deletions packages/gatsby-source-drupal/src/gatsby-node.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ exports.sourceNodes = async (
return
}
if (action === `delete`) {
actions.deleteNode({ node: getNode(createNodeId(id)) })
actions.deleteNode(getNode(createNodeId(id)))
reporter.log(`Deleted node: ${id}`)
changesActivity.end()
return
Expand Down Expand Up @@ -147,7 +147,7 @@ exports.sourceNodes = async (
let nodesToSync = data.data.entities
for (const nodeSyncData of nodesToSync) {
if (nodeSyncData.action === `delete`) {
actions.deleteNode({ node: getNode(createNodeId(nodeSyncData.id)) })
actions.deleteNode(getNode(createNodeId(nodeSyncData.id)))
} else {
// The data could be a single Drupal entity or an array of Drupal
// entities to update.
Expand Down Expand Up @@ -385,7 +385,7 @@ exports.onCreateDevServer = (
)
}
if (action === `delete`) {
actions.deleteNode({ node: getNode(createNodeId(id)) })
actions.deleteNode(getNode(createNodeId(id)))
return reporter.log(`Deleted node: ${id}`)
}
const nodeToUpdate = JSON.parse(JSON.parse(req.body)).data
Expand Down
2 changes: 1 addition & 1 deletion packages/gatsby-source-filesystem/src/gatsby-node.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ const createFSMachine = (
// It's possible the node was never created as sometimes tools will
// write and then immediately delete temporary files to the file system.
if (node) {
deleteNode({ node })
deleteNode(node)
}
}

Expand Down
7 changes: 1 addition & 6 deletions packages/gatsby/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1091,10 +1091,6 @@ interface ActionPlugin {
name: string
}

interface DeleteNodeArgs {
node: Node
}

interface CreateNodeFieldArgs {
node: Node
name: string
Expand Down Expand Up @@ -1128,9 +1124,8 @@ export interface Actions {

/** @see https://www.gatsbyjs.org/docs/actions/#deletePage */
deleteNode(
options: { node: Node },
node: NodeInput,
plugin?: ActionPlugin,
option?: ActionOptions
): void

/** @see https://www.gatsbyjs.org/docs/actions/#createNode */
Expand Down
41 changes: 16 additions & 25 deletions packages/gatsby/src/db/__tests__/nodes.js
Original file line number Diff line number Diff line change
Expand Up @@ -188,14 +188,9 @@ describe(`nodes db tests`, () => {
)
)
store.dispatch(
actions.deleteNode(
{
node: getNode(`hi`),
},
{
name: `tests`,
}
)
actions.deleteNode(getNode(`hi`), {
name: `tests`,
})
)
expect(getNodes()).toHaveLength(1)
})
Expand Down Expand Up @@ -272,12 +267,9 @@ describe(`nodes db tests`, () => {
)
)
store.dispatch(
actions.deleteNode(
{ node: getNode(`hi`) },
{
name: `tests`,
}
)
actions.deleteNode(getNode(`hi`), {
name: `tests`,
})
)
expect(getNodes()).toHaveLength(0)
})
Expand Down Expand Up @@ -309,11 +301,7 @@ describe(`nodes db tests`, () => {
}
)
)
store.dispatch(
actions.deleteNode({
node: getNode(`hi`),
})
)
store.dispatch(actions.deleteNode(getNode(`hi`)))
expect(getNode(`hi`)).toBeUndefined()
})

Expand All @@ -336,14 +324,17 @@ describe(`nodes db tests`, () => {
)
expect(getNode(`hi`)).toMatchObject({ id: `hi` })
store.dispatch(
actions.deleteNode(`hi`, getNode(`hi`), {
name: `tests`,
})
actions.deleteNode(
{ node: 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 }). ` +
`Calling "deleteNode" with {node} is deprecated. Please pass ` +
`the node directly to the function: deleteNode(node) ` +
`"deleteNode" was called by tests`
expect(report.warn).toHaveBeenCalledWith(deprecationNotice)
})
Expand All @@ -367,7 +358,7 @@ describe(`nodes db tests`, () => {
)
)
store.dispatch(
actions.deleteNode(`hi`, getNode(`hi`), {
actions.deleteNode(getNode(`hi`), {
name: `tests`,
})
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,5 +197,5 @@ exports.onCreatePage = ({ createContentDigest, page, actions }) => {
emitter.on(`DELETE_PAGE`, action => {
const nodeId = createPageId(action.payload.path)
const node = getNode(nodeId)
boundActionCreators.deleteNode({ node })
boundActionCreators.deleteNode(node)
})
44 changes: 21 additions & 23 deletions packages/gatsby/src/redux/actions/public.js
Original file line number Diff line number Diff line change
Expand Up @@ -425,53 +425,50 @@ ${reservedFields.map(f => ` * "${f}"`).join(`\n`)}

/**
* Delete a node
* @param {object} $0
* @param {object} $0.node the node object
* @param {object} node A node object. See the "createNode" action for more information about the node object details.
* @example
* deleteNode({node: node})
* deleteNode(node)
*/
actions.deleteNode = (options: any, plugin: Plugin, args: any) => {
actions.deleteNode = (node: any, plugin?: Plugin) => {
let id

// Check if using old method signature. Warn about incorrect usage but get
// node from nodeID anyway.
if (typeof options === `string`) {
// TODO(v4): Remove this deprecation warning and only allow deleteNode(node)
if (node && node.node) {
let msg =
`Calling "deleteNode" with a nodeId is deprecated. Please pass an ` +
`object containing a full node instead: deleteNode({ node }).`
if (args && args.name) {
// `plugin` used to be the third argument
plugin = args
`Calling "deleteNode" with {node} is deprecated. Please pass ` +
`the node directly to the function: deleteNode(node)`

if (plugin && plugin.name) {
msg = msg + ` "deleteNode" was called by ${plugin.name}`
}
report.warn(msg)

id = options
id = node.node.id
} else {
id = options && options.node && options.node.id
id = node && node.id
}

// Always get node from the store, as the node we get as an arg
// might already have been deleted.
const node = getNode(id)
const internalNode = getNode(id)
if (plugin) {
const pluginName = plugin.name

if (
node &&
typeOwners[node.internal.type] &&
typeOwners[node.internal.type] !== pluginName
internalNode &&
typeOwners[internalNode.internal.type] &&
typeOwners[internalNode.internal.type] !== pluginName
)
throw new Error(stripIndent`
The plugin "${pluginName}" deleted a node of a type owned by another plugin.

The node type "${node.internal.type}" is owned by "${
typeOwners[node.internal.type]
The node type "${internalNode.internal.type}" is owned by "${
typeOwners[internalNode.internal.type]
}".

The node object passed to "deleteNode":

${JSON.stringify(node, null, 4)}
${JSON.stringify(internalNode, null, 4)}

The plugin deleting the node:

Expand All @@ -487,12 +484,13 @@ actions.deleteNode = (options: any, plugin: Plugin, args: any) => {
}
}

const deleteAction = createDeleteAction(node)
const deleteAction = createDeleteAction(internalNode)

// It's possible the file node was never created as sometimes tools will
// write and then immediately delete temporary files to the file system.
const deleteDescendantsActions =
node && findChildren(node.children).map(getNode).map(createDeleteAction)
internalNode &&
findChildren(internalNode.children).map(getNode).map(createDeleteAction)

if (deleteDescendantsActions && deleteDescendantsActions.length) {
return [...deleteDescendantsActions, deleteAction]
Expand Down
2 changes: 1 addition & 1 deletion packages/gatsby/src/utils/source-nodes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ function deleteStaleNodes(state: IGatsbyState, nodes: Array<Node>): void {
const staleNodes = getStaleNodes(state, nodes)

if (staleNodes.length > 0) {
staleNodes.forEach(node => deleteNode({ node }))
staleNodes.forEach(node => deleteNode(node))
}
}

Expand Down