Skip to content

Commit

Permalink
feat(gatsby, gatsby-source-contentful): add public action to disable …
Browse files Browse the repository at this point in the history
…stale node checks (#37782)

Co-authored-by: Michal Piechowiak <misiek.piechowiak@gmail.com>
  • Loading branch information
TylerBarnes and pieh authored Apr 5, 2023
1 parent c4d2d0f commit fd168e0
Show file tree
Hide file tree
Showing 21 changed files with 603 additions and 147 deletions.
27 changes: 27 additions & 0 deletions docs/docs/node-creation.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,33 @@ When a `source-nodes` plugin runs again, it generally recreates nodes (which aut

Any nodes that aren't touched by the end of the `source-nodes` phase, are deleted. This is performed via a diff between the `nodesTouched` and `nodes` Redux namespaces, in [source-nodes.ts](https://github.com/gatsbyjs/gatsby/blob/master/packages/gatsby/src/utils/source-nodes.ts)

### Opting out of stale node deletion

Stale node deletion is a very expensive process because all nodes in the data store need to be iterated on to check if they're stale or not. Iterating on nodes to check for staleness also requires loading that entire node into memory, so all nodes created by a source plugin are loaded into memory to check for their staleness, even if they're otherwise not needed in memory.

Source plugins can skip this expensive step by calling the `enableStatefulSourceNodes` action.
This will stop Gatsby from checking for stale nodes created by the source plugin that called the action.
This is a major performance improvement for medium and large sites and those sites will need less total memory to build.

When enabling stateful sourcing plugin authors need to be sure their plugins properly handle deleting nodes when they need to be deleted. Since Gatsby is no longer checking for node staleness, data which should no longer exist could stick around.
`enableStatefulSourceNodes` should only be enabled for source plugins that can fully support all delete operations in their data source.

Note that if `enableStatefulSourceNodes` is supported by the user's `gatsby` version, the action should be called every time `sourceNodes` runs.

Example:

```js
import { hasFeature } from "gatsby-plugin-utils"

exports.sourceNodes = ({ actions }) => {
if (hasFeature(`stateful-source-nodes`)) {
actions.enableStatefulSourceNodes()
} else {
// fallback to old behavior where all nodes are iterated on and touchNode is called.
}
}
```

## Changing a node's fields

From a site developer's point of view, nodes are immutable. In the sense that if you change a node object, those changes will not be seen by other parts of Gatsby. To make a change to a node, it must be persisted to Redux via an action.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ describe(`gatsby-node`, () => {
currentNodeMap.delete(node.id)
}),
touchNode: jest.fn(),
enableStatefulSourceNodes: jest.fn(),
}
const schema = {
buildObjectType: jest.fn(() => {
Expand Down
3 changes: 2 additions & 1 deletion packages/gatsby-source-contentful/src/normalize.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ import { getGatsbyVersion } from "gatsby-core-utils"
import { lt, prerelease } from "semver"

const typePrefix = `Contentful`
const makeTypeName = type => _.upperFirst(_.camelCase(`${typePrefix} ${type}`))
export const makeTypeName = type =>
_.upperFirst(_.camelCase(`${typePrefix} ${type}`))

const GATSBY_VERSION_MANIFEST_V2 = `4.3.0`
const gatsbyVersion =
Expand Down
25 changes: 20 additions & 5 deletions packages/gatsby-source-contentful/src/source-nodes.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// @ts-check
import { hasFeature } from "gatsby-plugin-utils/has-feature"
import isOnline from "is-online"
import _ from "lodash"

Expand All @@ -11,6 +12,7 @@ import {
createAssetNodes,
createNodesForContentType,
makeId,
makeTypeName,
} from "./normalize"
import { createPluginConfig } from "./plugin-options"
import { CODES } from "./report"
Expand Down Expand Up @@ -40,7 +42,7 @@ const CONTENT_DIGEST_COUNTER_SEPARATOR = `_COUNT_`
* or the fallback field or the default field.
*/

let isFirstSource = true
let isFirstSourceNodesCallOfCurrentNodeProcess = true
export async function sourceNodes(
{
actions,
Expand All @@ -55,14 +57,26 @@ export async function sourceNodes(
},
pluginOptions
) {
const { createNode, touchNode, deleteNode, unstable_createNodeManifest } =
actions
const hasStatefulSourceNodes = hasFeature(`stateful-source-nodes`)
const needToTouchNodes = !hasStatefulSourceNodes

const {
createNode,
touchNode,
deleteNode,
unstable_createNodeManifest,
enableStatefulSourceNodes,
} = actions

const online = await isOnline()

if (hasStatefulSourceNodes) {
enableStatefulSourceNodes()
}
// Gatsby only checks if a node has been touched on the first sourcing.
// As iterating and touching nodes can grow quite expensive on larger sites with
// 1000s of nodes, we'll skip doing this on subsequent sources.
if (isFirstSource) {
else if (isFirstSourceNodesCallOfCurrentNodeProcess && needToTouchNodes) {
getNodes().forEach(node => {
if (node.internal.owner !== `gatsby-source-contentful`) {
return
Expand All @@ -73,9 +87,10 @@ export async function sourceNodes(
touchNode(getNode(node.fields.localFile))
}
})
isFirstSource = false
}

isFirstSourceNodesCallOfCurrentNodeProcess = false

if (
!online &&
process.env.GATSBY_CONTENTFUL_OFFLINE === `true` &&
Expand Down
6 changes: 6 additions & 0 deletions packages/gatsby/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export type AvailableFeatures =
| "image-cdn"
| "graphql-typegen"
| "content-file-path"
| "stateful-source-nodes"

export {
Link,
Expand Down Expand Up @@ -433,6 +434,11 @@ export interface GatsbyNode<
calllback: PluginCallback<void>
): void | Promise<void>

/**
* Marks the source plugin that called this function as stateful. Gatsby will not check for stale nodes for any plugin that calls this.
*/
enableStatefulSourceNodes?(this: void, plugin?: ActionPlugin)

/**
* Called when a new node is created. Plugins wishing to extend or
* transform nodes created by other plugins should implement this API.
Expand Down
1 change: 1 addition & 0 deletions packages/gatsby/scripts/__tests__/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ it("generates the expected api output", done => {
"graphql-typegen",
"content-file-path",
"slices",
"stateful-source-nodes",
],
"node": Object {
"createPages": Object {},
Expand Down
2 changes: 1 addition & 1 deletion packages/gatsby/scripts/output-api-file.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ async function outputFile() {
}, {})

/** @type {Array<import("../index").AvailableFeatures>} */
output.features = ["image-cdn", "graphql-typegen", "content-file-path", "slices"];
output.features = ["image-cdn", "graphql-typegen", "content-file-path", "slices", "stateful-source-nodes"];

return fs.writeFile(
path.resolve(OUTPUT_FILE_NAME),
Expand Down
99 changes: 99 additions & 0 deletions packages/gatsby/src/datastore/__tests__/nodes.js
Original file line number Diff line number Diff line change
Expand Up @@ -339,4 +339,103 @@ describe(`nodes db tests`, () => {
})
expect(getNodes()).toHaveLength(0)
})

it(`records the node type owner when a node is created`, async () => {
// creating a node
store.dispatch(
actions.createNode(
{
id: `1`,
internal: {
type: `OwnerOneTestTypeOne`,
contentDigest: `ok`,
},
},
{
name: `test-owner-1`,
}
)
)
// and creating a second node in the same type
store.dispatch(
actions.createNode(
{
id: `2`,
internal: {
type: `OwnerOneTestTypeOne`,
contentDigest: `ok`,
},
},
{
name: `test-owner-1`,
}
)
)

// and a third node of a different type but same plugin
store.dispatch(
actions.createNode(
{
id: `3`,
internal: {
type: `OwnerOneTestTypeTwo`,
contentDigest: `ok`,
},
},
{
name: `test-owner-1`,
}
)
)

// fourth node by a different plugin
store.dispatch(
actions.createNode(
{
id: `4`,
internal: {
type: `OwnerTwoTestTypeThree`,
contentDigest: `ok`,
},
},
{
name: `test-owner-2`,
}
)
)

// fifth node by second plugin but the node is deleted. Deleted nodes still have type owners
const nodeFive = {
id: `5`,
internal: {
type: `OwnerTwoTestTypeFour`,
contentDigest: `ok`,
},
}
store.dispatch(
actions.createNode(nodeFive, {
name: `test-owner-2`,
})
)
store.dispatch(
actions.deleteNode(nodeFive, {
name: `test-owner-2`,
})
)
expect(getNode(`5`)).toBeUndefined()

const state = store.getState()

const ownerOne = state.typeOwners.pluginsToTypes.get(`test-owner-1`)
expect(ownerOne).toBeTruthy()
expect(ownerOne.has(`OwnerOneTestTypeOne`)).toBeTrue()
expect(ownerOne.has(`OwnerOneTestTypeTwo`)).toBeTrue()
expect(ownerOne.has(`OwnerTwoTestTypeThree`)).toBeFalse()

const ownerTwo = state.typeOwners.pluginsToTypes.get(`test-owner-2`)
expect(ownerTwo).toBeTruthy()
expect(ownerTwo.has(`OwnerOneTestTypeTwo`)).toBeFalse()
expect(ownerTwo.has(`OwnerTwoTestTypeThree`)).toBeTrue()
expect(ownerTwo.has(`OwnerTwoTestTypeFour`)).toBeTrue()
})
})
7 changes: 7 additions & 0 deletions packages/gatsby/src/query/__tests__/data-tracking.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,13 @@ jest.mock(`gatsby-cli/lib/reporter`, () => {
end: jest.fn(),
}
},
createProgress: () => {
return {
start: jest.fn(),
tick: jest.fn(),
end: jest.fn(),
}
},
phantomActivity: () => {
return {
start: jest.fn(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,13 +123,18 @@ Object {
},
"slices": Map {},
"slicesByTemplate": Map {},
"statefulSourcePlugins": Set {},
"staticQueriesByTemplate": Map {},
"staticQueryComponents": Map {},
"status": Object {
"LAST_NODE_COUNTER": 0,
"PLUGINS_HASH": "",
"plugins": Object {},
},
"typeOwners": Object {
"pluginsToTypes": Map {},
"typesToPlugins": Map {},
},
"webpackCompilationHash": "",
}
`;
7 changes: 3 additions & 4 deletions packages/gatsby/src/redux/__tests__/nodes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@ import { nodesReducer } from "../reducers/nodes"
import { IGatsbyNode } from "../types"
import { nodesTouchedReducer } from "../reducers/nodes-touched"

const dispatch = jest.fn()

const dispatch = jest.spyOn(store, `dispatch`)
type MapObject = Record<string, IGatsbyNode>

const fromMapToObject = (map: Map<string, any>): MapObject => {
Expand All @@ -18,8 +17,8 @@ const fromMapToObject = (map: Map<string, any>): MapObject => {

describe(`Create and update nodes`, (): void => {
beforeEach((): void => {
dispatch.mockClear()
store.dispatch({ type: `DELETE_CACHE` })
dispatch.mockClear()
})

it(`allows creating nodes`, (): void => {
Expand Down Expand Up @@ -80,7 +79,7 @@ describe(`Create and update nodes`, (): void => {
children: [],
parent: `test`,
internal: {
contentDigest: `hasdfljds`,
contentDigest: `hasdfljds2`,
type: `Test`,
},
pickle: false,
Expand Down
Loading

1 comment on commit fd168e0

@nrandell
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there an issue with this new functionality? I suddenly found various errors with incremental builds on our graphcms plugin (https://github.com/bond-london/bond-theme-development/tree/main/packages/simple-gatsby-source-graphcms)

The error was

Error: The plugin "@bond-london/simple-gatsby-source-graphcms" created a node of a type owned by another plugin.
        The node type "File" is owned by "gatsby-source-filesystem".
        If you copy and pasted code from elsewhere, you'll need to pick a new type name

After some debugging, it looks like I was touching previously creating remote file nodes of type 'File' and the setTypeOwner function was complaining that my plugin hadn't created that node.

Looking at my 'touchNode' code, I could see that I wasn't passing in a plugin to touchNode, but doubleBind in api-runner-node.js (

return boundActionCreator(args[0], plugin, actionOptions)
) added the plugin in if I didn't pass one in.

I'm currently fixing/testing this in my source plugin by passing in 'undefined' as the plugin as a fix for bond-london/bond-theme-development#1

But I'm not convinced this is the correct way to fix it as it could break other plugins.

Please sign in to comment.