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

fix(gatsby-source-drupal): Clone nodes before adding back references so Gatsby knows the node has changed (so knows to re-run queries) #33328

Merged
merged 1 commit into from
Sep 29, 2021

Conversation

KyleAMathews
Copy link
Contributor

@KyleAMathews KyleAMathews commented Sep 28, 2021

Big thanks to @kszot-ref for finding the root problem of the issue he was noticing with his Drupal/Gatsby site in #33285 (comment).

Fixes #33284

The bug is fairly subtle. The plugin currently updates backreferences on nodes by
directly mutating them. This means that for already existing nodes, Gatsby wasn't able
to tell that they'd changed because when it tried to compare the "new" version of the node
with the "old" version, they were the same because we'd mutated the old node object directly.

The fix is to always do a deep clone of the node object before making modifications to it
so Gatsby knows the node has changed and re-runs any pages depending on it.

I wonder now how common this issue is in other source plugins.

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Sep 28, 2021
@KyleAMathews KyleAMathews added topic: source-drupal Related to Gatsby's integration with Drupal type: bug An issue or pull request relating to a bug in Gatsby and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Sep 28, 2021
@KyleAMathews KyleAMathews changed the title fix(gatsby-source-drupal): Correctly update nodes with changed back references so queries are re-run fix(gatsby-source-drupal): Clone nodes before mutating so Gatsby knows to re-run queries Sep 29, 2021
@KyleAMathews KyleAMathews changed the title fix(gatsby-source-drupal): Clone nodes before mutating so Gatsby knows to re-run queries fix(gatsby-source-drupal): Clone nodes before adding back references so Gatsby knows to re-run queries Sep 29, 2021
const referencedNode = getNode(nodeID)
let referencedNode
if (mutateNode) {
referencedNode = getNode(nodeID)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the initial sourcing, it does make sense to directly mutate nodes as none of them have been created yet.

@@ -271,23 +299,9 @@ ${JSON.stringify(nodeToUpdate, null, 4)}
)
}
})

// see what nodes are newly referenced, and make sure to call `createNode` on them
const addedReferencedNodes = _.difference(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code isn't needed as it just duplicates what handleReferences does. We now return nodes with updated backreferences from that function so we can eliminate this code.

@KyleAMathews KyleAMathews changed the title fix(gatsby-source-drupal): Clone nodes before adding back references so Gatsby knows to re-run queries fix(gatsby-source-drupal): Clone nodes before adding back references so Gatsby knows the node has changed (so knows to re-run queries) Sep 29, 2021
@@ -9,16 +9,17 @@ const {

const { getOptions } = require(`./plugin-options`)

const backRefsNamesLookup = new WeakMap()
const referencedNodesLookup = new WeakMap()
const backRefsNamesLookup = new Map()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move to regular maps w/ IDs as we're not holding consistent references to the same node object anymore.

@KyleAMathews KyleAMathews merged commit 5196e40 into master Sep 29, 2021
@KyleAMathews KyleAMathews deleted the copy-drupal-nodes branch September 29, 2021 17:02
KyleAMathews added a commit that referenced this pull request Sep 29, 2021
vladar pushed a commit that referenced this pull request Oct 27, 2021
…eferences so queries are re-run (#33328)

(cherry picked from commit 5196e40)
vladar added a commit that referenced this pull request Oct 27, 2021
…eferences so queries are re-run (#33328) (#33699)

(cherry picked from commit 5196e40)

Co-authored-by: Kyle Mathews <[email protected]>
Co-authored-by: Vladimir Razuvaev <[email protected]>
wardpeet pushed a commit to herecydev/gatsby that referenced this pull request Oct 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: source-drupal Related to Gatsby's integration with Drupal type: bug An issue or pull request relating to a bug in Gatsby
Projects
None yet
2 participants