From 5eb2ac6e6ba17ac168865ae54f1f79fd1d015705 Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Tue, 16 Nov 2021 12:52:44 +0100 Subject: [PATCH 01/11] feat(gatsby): detect node mutations (enabled by flag or env var) --- packages/gatsby/src/redux/actions/public.js | 3 +- packages/gatsby/src/schema/node-model.js | 14 +- packages/gatsby/src/services/initialize.ts | 5 + packages/gatsby/src/utils/api-runner-node.js | 40 ++++- .../gatsby/src/utils/detect-node-mutations.ts | 159 ++++++++++++++++++ packages/gatsby/src/utils/flags.ts | 10 ++ .../gatsby/src/utils/stack-trace-utils.ts | 23 ++- 7 files changed, 240 insertions(+), 14 deletions(-) create mode 100644 packages/gatsby/src/utils/detect-node-mutations.ts diff --git a/packages/gatsby/src/redux/actions/public.js b/packages/gatsby/src/redux/actions/public.js index fc8b00dc10de2..1823f7a9a1118 100644 --- a/packages/gatsby/src/redux/actions/public.js +++ b/packages/gatsby/src/redux/actions/public.js @@ -28,6 +28,7 @@ const normalizePath = require(`../../utils/normalize-path`).default import { createJobV2FromInternalJob } from "./internal" import { maybeSendJobToMainProcess } from "../../utils/jobs/worker-messaging" import { reportOnce } from "../../utils/report-once" +import { wrapNode } from "../../utils/detect-node-mutations" const isNotTestEnv = process.env.NODE_ENV !== `test` const isTestEnv = process.env.NODE_ENV === `test` @@ -868,7 +869,7 @@ actions.createNode = const { payload: node, traceId, parentSpan } = createNodeAction return apiRunnerNode(`onCreateNode`, { - node, + node: wrapNode(node), traceId, parentSpan, traceTags: { nodeId: node.id, nodeType: node.internal.type }, diff --git a/packages/gatsby/src/schema/node-model.js b/packages/gatsby/src/schema/node-model.js index 46035933ec46d..2f80b8ae2b943 100644 --- a/packages/gatsby/src/schema/node-model.js +++ b/packages/gatsby/src/schema/node-model.js @@ -22,6 +22,7 @@ import { } from "../datastore" import { GatsbyIterable, isIterable } from "../datastore/common/iterable" import { reportOnce } from "../utils/report-once" +import { wrapNode, wrapNodes } from "../utils/detect-node-mutations" type TypeOrTypeName = string | GraphQLOutputType @@ -149,7 +150,7 @@ class LocalNodeModel { this.trackInlineObjectsInRootNode(node) } - return this.trackPageDependencies(result, pageDependencies) + return wrapNode(this.trackPageDependencies(result, pageDependencies)) } /** @@ -180,7 +181,7 @@ class LocalNodeModel { result.forEach(node => this.trackInlineObjectsInRootNode(node)) } - return this.trackPageDependencies(result, pageDependencies) + return wrapNodes(this.trackPageDependencies(result, pageDependencies)) } /** @@ -221,7 +222,7 @@ class LocalNodeModel { typeof type === `string` ? type : type.name } - return this.trackPageDependencies(result, pageDependencies) + return wrapNodes(this.trackPageDependencies(result, pageDependencies)) } /** @@ -346,7 +347,10 @@ class LocalNodeModel { pageDependencies.connectionType = gqlType.name } this.trackPageDependencies(result.entries, pageDependencies) - return result + return { + entries: wrapNodes(result.entries), + totalCount: result.totalCount, + } } /** @@ -383,7 +387,7 @@ class LocalNodeModel { // the query whenever any node of this type changes. pageDependencies.connectionType = gqlType.name } - return this.trackPageDependencies(first, pageDependencies) + return wrapNode(this.trackPageDependencies(first, pageDependencies)) } prepareNodes(type, queryFields, fieldsToResolve) { diff --git a/packages/gatsby/src/services/initialize.ts b/packages/gatsby/src/services/initialize.ts index 3e246ce566431..6f8314a5a78ce 100644 --- a/packages/gatsby/src/services/initialize.ts +++ b/packages/gatsby/src/services/initialize.ts @@ -20,6 +20,7 @@ import { IGatsbyState, IStateProgram } from "../redux/types" import { IBuildContext } from "./types" import { detectLmdbStore } from "../datastore" import { loadConfigAndPlugins } from "../bootstrap/load-config-and-plugins" +import { enableNodeMutationsDetection } from "../utils/detect-node-mutations" interface IPluginResolution { resolve: string @@ -161,6 +162,10 @@ export async function initialize({ } const lmdbStoreIsUsed = detectLmdbStore() + if (process.env.GATSBY_EXPERIMENTAL_DETECT_NODE_MUTATIONS) { + enableNodeMutationsDetection() + } + if (config && config.polyfill) { reporter.warn( `Support for custom Promise polyfills has been removed in Gatsby v2. We only support Babel 7's new automatic polyfilling behavior.` diff --git a/packages/gatsby/src/utils/api-runner-node.js b/packages/gatsby/src/utils/api-runner-node.js index e4f3e1779844b..9b412d2a43234 100644 --- a/packages/gatsby/src/utils/api-runner-node.js +++ b/packages/gatsby/src/utils/api-runner-node.js @@ -30,6 +30,7 @@ const { requireGatsbyPlugin } = require(`./require-gatsby-plugin`) const { getNonGatsbyCodeFrameFormatted } = require(`./stack-trace-utils`) const { trackBuildError, decorateEvent } = require(`gatsby-telemetry`) import errorParser from "./api-runner-error-parser" +import { wrapNode, wrapNodes } from "./detect-node-mutations" if (!process.env.BLUEBIRD_DEBUG && !process.env.BLUEBIRD_LONG_STACK_TRACES) { // Unless specified - disable longStackTraces @@ -40,6 +41,21 @@ if (!process.env.BLUEBIRD_DEBUG && !process.env.BLUEBIRD_LONG_STACK_TRACES) { Promise.config({ longStackTraces: false }) } +const nodeMutationsWrappers = { + getNode(id) { + return wrapNode(getNode(id)) + }, + getNodes() { + return wrapNodes(getNodes()) + }, + getNodesByType(type) { + return wrapNodes(getNodesByType(type)) + }, + getNodeAndSavePathDependency(id) { + return wrapNode(getNodeAndSavePathDependency(id)) + }, +} + // Bind action creators per plugin so we can auto-add // metadata to actions they create. const boundPluginActionCreators = {} @@ -372,6 +388,14 @@ const runAPI = async (plugin, api, args, activity) => { runningActivities.forEach(activity => activity.end()) } + const shouldDetectNodeMutations = [ + `sourceNodes`, + `onCreateNode`, + `createResolvers`, + `createSchemaCustomization`, + `setFieldsOnGraphQLNodeType`, + ].includes(api) + const apiCallArgs = [ { ...args, @@ -383,11 +407,19 @@ const runAPI = async (plugin, api, args, activity) => { store, emitter, getCache, - getNodes, - getNode, - getNodesByType, + getNodes: shouldDetectNodeMutations + ? nodeMutationsWrappers.getNodes + : getNodes, + getNode: shouldDetectNodeMutations + ? nodeMutationsWrappers.getNode + : getNode, + getNodesByType: shouldDetectNodeMutations + ? nodeMutationsWrappers.getNodesByType + : getNodesByType, reporter: extendedLocalReporter, - getNodeAndSavePathDependency, + getNodeAndSavePathDependency: shouldDetectNodeMutations + ? nodeMutationsWrappers.getNodeAndSavePathDependency + : getNodeAndSavePathDependency, cache, createNodeId: namespacedCreateNodeId, createContentDigest, diff --git a/packages/gatsby/src/utils/detect-node-mutations.ts b/packages/gatsby/src/utils/detect-node-mutations.ts new file mode 100644 index 0000000000000..f35611ed898b9 --- /dev/null +++ b/packages/gatsby/src/utils/detect-node-mutations.ts @@ -0,0 +1,159 @@ +import reporter from "gatsby-cli/lib/reporter" +import { getNonGatsbyCodeFrameFormatted } from "./stack-trace-utils" +import type { IGatsbyNode } from "../redux/types" + +const reported = new Set() + +const genericProxy = createProxyHandler() +const nodeInternalProxy = createProxyHandler({ + onGet(key, value) { + if (key === `fieldOwners` || key === `content`) { + // all allowed in here + return value + } + return undefined + }, + onSet(target, key, value) { + if (key === `fieldOwners` || key === `content`) { + target[key] = value + return true + } + return undefined + }, +}) + +const nodeProxy = createProxyHandler({ + onGet(key, value) { + if (key === `internal`) { + return memoizedProxy(value, nodeInternalProxy) + } else if ( + key === `__gatsby_resolved` || + key === `fields` || + key === `children` + ) { + // all allowed in here + return value + } + return undefined + }, + onSet(target, key, value) { + if (key === `__gatsby_resolved` || key === `fields` || key === `children`) { + target[key] = value + return true + } + return undefined + }, +}) + +/** + * Every time we create proxy for object, we store it in WeakMap, + * so that we reuse it for that object instead of creating new Proxy. + * This also ensures reference equality: `memoizedProxy(obj) === memoizedProxy(obj)`. + * If we didn't reuse already created proxy above comparison would return false. + */ +const referenceMap = new WeakMap() +function memoizedProxy(target: T, handler: ProxyHandler): T { + const alreadyWrapped = referenceMap.get(target) + if (alreadyWrapped) { + return alreadyWrapped + } else { + const wrapped = new Proxy(target, handler) + referenceMap.set(target, wrapped) + return wrapped + } +} + +function createProxyHandler({ + onGet, + onSet, +}: { + onGet?: (key: string | symbol, value: any) => any + onSet?: (target: any, key: string | symbol, value: any) => boolean | undefined +} = {}): ProxyHandler { + function set(target, key, value): boolean { + if (onSet) { + const result = onSet(target, key, value) + if (result !== undefined) { + return result + } + } + + const error = new Error(`Stack trace:`) + Error.captureStackTrace(error, set) + + if (error.stack && !reported.has(error.stack)) { + reported.add(error.stack) + const codeFrame = getNonGatsbyCodeFrameFormatted({ + stack: error.stack, + }) + reporter.warn( + `Node mutation detected\n\n${ + codeFrame ? `${codeFrame}\n\n` : `` + }${error.stack.replace(/^$Error:?\s*/, ``)}` + ) + } + return true + } + + function get(target, key): any { + const value = target[key] + + if (onGet) { + const result = onGet(key, value) + if (result !== undefined) { + return result + } + } + + const fieldDescriptor = Object.getOwnPropertyDescriptor(target, key) + if (fieldDescriptor && !fieldDescriptor.writable) { + // this is to prevent errors like: + // ``` + // TypeError: 'get' on proxy: property 'constants' is a read - only and + // non - configurable data property on the proxy target but the proxy + // did not return its actual value + // (expected '[object Object]' but got '[object Object]') + // ``` + return value + } + + if (typeof value === `object` && value !== null) { + return memoizedProxy(value, genericProxy) + } + + return value + } + + return { + get, + set, + } +} + +let shouldWrapNodesInProxies = + !!process.env.GATSBY_EXPERIMENTAL_DETECT_NODE_MUTATIONS +export function enableNodeMutationsDetection(): void { + shouldWrapNodesInProxies = true + + reporter.warn( + `Node mutation detection is enabled. Remember to disable it after you are finished with diagnostic as it will cause build performance degradation.` + ) +} + +export function wrapNode(node: T): T { + if (node && shouldWrapNodesInProxies) { + return memoizedProxy(node, nodeProxy) + } else { + return node + } +} + +export function wrapNodes | undefined>( + nodes: T +): T { + if (nodes && shouldWrapNodesInProxies && nodes.length > 0) { + return nodes.map(node => memoizedProxy(node, nodeProxy)) as T + } else { + return nodes + } +} diff --git a/packages/gatsby/src/utils/flags.ts b/packages/gatsby/src/utils/flags.ts index 408e2b44716c5..bde1afbef59c9 100644 --- a/packages/gatsby/src/utils/flags.ts +++ b/packages/gatsby/src/utils/flags.ts @@ -226,6 +226,16 @@ const activeFlags: Array = [ }, requires: `Requires Node v14.10 or above.`, }, + { + name: `DETECT_NODE_MUTATIONS`, + env: `GATSBY_EXPERIMENTAL_DETECT_NODE_MUTATIONS`, + command: `all`, + telemetryId: `DetectNodeMutations`, + // TODO: description + description: ``, + experimental: false, + testFitness: (): fitnessEnum => true, + }, ] export default activeFlags diff --git a/packages/gatsby/src/utils/stack-trace-utils.ts b/packages/gatsby/src/utils/stack-trace-utils.ts index 87786b8a7844e..d9123fb8b276d 100644 --- a/packages/gatsby/src/utils/stack-trace-utils.ts +++ b/packages/gatsby/src/utils/stack-trace-utils.ts @@ -50,8 +50,18 @@ interface ICodeFrame { export const getNonGatsbyCodeFrame = ({ highlightCode = true, + stack, +}: { + highlightCode?: boolean + stack?: string } = {}): null | ICodeFrame => { - const callSite = getNonGatsbyCallSite() + let callSite + if (stack) { + callSite = stackTrace.parse({ stack, name: ``, message: `` })[0] + } else { + callSite = getNonGatsbyCallSite() + } + if (!callSite) { return null } @@ -80,11 +90,16 @@ export const getNonGatsbyCodeFrame = ({ } } -export const getNonGatsbyCodeFrameFormatted = ({ highlightCode = true } = {}): - | null - | string => { +export const getNonGatsbyCodeFrameFormatted = ({ + highlightCode = true, + stack, +}: { + highlightCode?: boolean + stack?: string +} = {}): null | string => { const possibleCodeFrame = getNonGatsbyCodeFrame({ highlightCode, + stack, }) if (!possibleCodeFrame) { From 93be2c3497441fe947f41f63d98b58949c96304a Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Sat, 20 Nov 2021 12:11:44 +0100 Subject: [PATCH 02/11] tmp docs --- docs/docs/reference/release-notes/migrating-from-v3-to-v4.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/docs/docs/reference/release-notes/migrating-from-v3-to-v4.md b/docs/docs/reference/release-notes/migrating-from-v3-to-v4.md index 83bd84a1f2b20..000ee34407aa1 100644 --- a/docs/docs/reference/release-notes/migrating-from-v3-to-v4.md +++ b/docs/docs/reference/release-notes/migrating-from-v3-to-v4.md @@ -583,9 +583,6 @@ This was never an intended feature of Gatsby and is considered an anti-pattern ( Starting with v4 Gatsby introduces a persisted storage for nodes and thus this pattern will no longer work because nodes are persisted after `createNode` call and all direct mutations after that will be lost. -Unfortunately it is hard to detect it automatically (without sacrificing performance), so we recommend you to -check your code to ensure you don't mutate nodes directly. - Gatsby provides several actions available in `sourceNodes` and `onCreateNode` APIs to use instead: - [createNode](/docs/reference/config-files/actions/#createNode) @@ -594,6 +591,8 @@ Gatsby provides several actions available in `sourceNodes` and `onCreateNode` AP You can use `createNodeField` and the `@link` directive to create the same schema shape. The [`@link` directive](/docs/reference/graphql-data-layer/schema-customization/#foreign-key-fields) accepts a `from` argument that you can use to place your node to the old position (as `createNodeField` places everything under a `fields` key). See the [source plugin guide](/docs/how-to/plugins-and-themes/creating-a-source-plugin/#create-remote-file-node-from-a-url) for more information. Checkout [this PR](https://github.com/gatsbyjs/gatsby/pull/33715) for a real-world migration example. +Gatsby (starting with version `4.3.0`) can automatically detect those mutations but it introduces noticeable performance degradation and it's not enabled by default. To temporarily opt-in - set `DETECT_NODE_MUTATIONS: true` flag in `gatsby-config.js` or run Gatsby with `GATSBY_EXPERIMENTAL_DETECT_NODE_MUTATIONS=true` environment variable (please remember to disable it after you are done). It will log traces so it's easier to locate offending code. + ### `___NODE` convention Please note that the [deprecation of the `___NODE` convention](#___node-convention-is-deprecated) especially affects source plugins and for Gatsby v5 you'll need to update your usage to keep compatibility. From dac96d382c4a6ee48da8ce24f9978c6327f0b4df Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Sun, 5 Dec 2021 11:03:37 +0100 Subject: [PATCH 03/11] rename env var to drop experimental --- packages/gatsby/src/utils/flags.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/gatsby/src/utils/flags.ts b/packages/gatsby/src/utils/flags.ts index bde1afbef59c9..d1c4f74b64a05 100644 --- a/packages/gatsby/src/utils/flags.ts +++ b/packages/gatsby/src/utils/flags.ts @@ -228,7 +228,7 @@ const activeFlags: Array = [ }, { name: `DETECT_NODE_MUTATIONS`, - env: `GATSBY_EXPERIMENTAL_DETECT_NODE_MUTATIONS`, + env: `GATSBY_DETECT_NODE_MUTATIONS`, command: `all`, telemetryId: `DetectNodeMutations`, // TODO: description From 68d551a92f410f353490d4235ffd394a026e00f5 Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Sun, 5 Dec 2021 11:06:45 +0100 Subject: [PATCH 04/11] fix stack trace label not dropping 'Error:' --- packages/gatsby/src/utils/detect-node-mutations.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/gatsby/src/utils/detect-node-mutations.ts b/packages/gatsby/src/utils/detect-node-mutations.ts index f35611ed898b9..4221a5127d4ba 100644 --- a/packages/gatsby/src/utils/detect-node-mutations.ts +++ b/packages/gatsby/src/utils/detect-node-mutations.ts @@ -89,7 +89,7 @@ function createProxyHandler({ reporter.warn( `Node mutation detected\n\n${ codeFrame ? `${codeFrame}\n\n` : `` - }${error.stack.replace(/^$Error:?\s*/, ``)}` + }${error.stack.replace(/^Error:?\s*/, ``)}` ) } return true From 4c08302f3b9ee304c138b6a3d926b08b8bb9dfd5 Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Sun, 5 Dec 2021 11:07:08 +0100 Subject: [PATCH 05/11] tmp of debugging docs --- .../debugging-missing-data.md | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 docs/docs/how-to/local-development/debugging-missing-data.md diff --git a/docs/docs/how-to/local-development/debugging-missing-data.md b/docs/docs/how-to/local-development/debugging-missing-data.md new file mode 100644 index 0000000000000..78fd4c27a1221 --- /dev/null +++ b/docs/docs/how-to/local-development/debugging-missing-data.md @@ -0,0 +1,73 @@ +--- +title: Debugging missing or stale data fields on nodes +--- + +In Gatsby 3 we introduced new data store - `LMDB` (enabled with `LMDB_STORE` and/or `PARALLEL_QUERY_RUNNING` flags). In Gatsby 4 it became the default data store. It allows us to execute data layer related processing outside of main build process and enable us to run queries in multiple processes as well as support additional rendering strategies (DSG and SSR). + +In a lot of cases that change is completely invisible to users, but there are cases where things behave differently. + +Direct node mutations in various API lifecycles are not persisted anymore. In previous Gatsby versions it did work because source of truth for our data layer was directly in Node.js memory, so mutating node was in fact mutating source of truth. Now we edit data we received from database, so unless we explicitly upsert data to database after edits, those edits will not be persisted (even for same the same build). + +Common errors when doing swap to `LMDB` will be that some fields don't exist anymore or are `null`/`undefined` when trying to execute graphql queries. + +Gatsby (starting with version 4.4) can detect those node mutations. Unfortunately it adds measurable overhead so we don't enable it by default, and only let users to opt into it when they see data related issues (particularly when they didn't have this issue before using `LMDB`). To enable diagnostic mode: + +- use truthy environment variable `GATSBY_DETECT_NODE_MUTATIONS`: + ``` + GATSBY_DETECT_NODE_MUTATIONS=1 gatsby build + ``` +- or use `DETECT_NODE_MUTATIONS` config flag: + ```javascript:title=gatsby-config.js + module.exports = { + flags: { + DETECT_NODE_MUTATIONS: true, + }, + } + ``` + +Example diagnostic message you might see: + +``` +warn Node mutation detected + +File /plugins/transform/gatsby-node.js:4:20 + 2 | if (node.internal.type === `Test`) { + 3 | // console.log(`[Mutating node in onCreateNode]`) +> 4 | node.nested.a2 = `edited` + | ^ + 5 | } + 6 | } + 7 | + +Stack trace: + at Object.exports.onCreateNode (/plugins/transform/gatsby-node.js:4:20) + at runAPI (/node_modules/gatsby/src/utils/api-runner-node.js:462:22) + at Promise.catch.decorateEvent.pluginName +(/node_modules/gatsby/src/utils/api-runner-node.js:613:13) + at Promise._execute +(/node_modules/bluebird/js/release/debuggability.js:384:9) + at Promise._resolveFromExecutor +(/node_modules/bluebird/js/release/promise.js:518:18) + at new Promise (/node_modules/bluebird/js/release/promise.js:103:10) + at /node_modules/gatsby/src/utils/api-runner-node.js:611:16 + at tryCatcher (/node_modules/bluebird/js/release/util.js:16:23) + at Object.gotValue (/node_modules/bluebird/js/release/reduce.js:166:18) + at Object.gotAccum (/node_modules/bluebird/js/release/reduce.js:155:25) + at Object.tryCatcher (/node_modules/bluebird/js/release/util.js:16:23) + at Promise._settlePromiseFromHandler +(/node_modules/bluebird/js/release/promise.js:547:31) + at Promise._settlePromise +(/node_modules/bluebird/js/release/promise.js:604:18) + at Promise._settlePromiseCtx +(/node_modules/bluebird/js/release/promise.js:641:10) + at _drainQueueStep (/node_modules/bluebird/js/release/async.js:97:12) + at _drainQueue (/node_modules/bluebird/js/release/async.js:86:9) +``` + +It will point to code that is trying to mutate nodes. Note that it might also point to installed plugins in which case plugin maintainer should be notified about it. + +Be sure to stop using this mode once you find and handle all problematic code paths as it will decrease performance. + +## Migration + +### Mutating a node in `onCreateNode` From 367d74f6bd76e4417e245536c482d36bc0ab046f Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Tue, 7 Dec 2021 11:46:50 +0100 Subject: [PATCH 06/11] add headings to doc and complete example migration for onCreateNode case --- .../debugging-missing-data.md | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/docs/docs/how-to/local-development/debugging-missing-data.md b/docs/docs/how-to/local-development/debugging-missing-data.md index 78fd4c27a1221..1cf908d4b71df 100644 --- a/docs/docs/how-to/local-development/debugging-missing-data.md +++ b/docs/docs/how-to/local-development/debugging-missing-data.md @@ -2,6 +2,8 @@ title: Debugging missing or stale data fields on nodes --- +## Overview of `LMDB` datastore behavior changes + In Gatsby 3 we introduced new data store - `LMDB` (enabled with `LMDB_STORE` and/or `PARALLEL_QUERY_RUNNING` flags). In Gatsby 4 it became the default data store. It allows us to execute data layer related processing outside of main build process and enable us to run queries in multiple processes as well as support additional rendering strategies (DSG and SSR). In a lot of cases that change is completely invisible to users, but there are cases where things behave differently. @@ -10,6 +12,8 @@ Direct node mutations in various API lifecycles are not persisted anymore. In pr Common errors when doing swap to `LMDB` will be that some fields don't exist anymore or are `null`/`undefined` when trying to execute graphql queries. +## Diagnostic mode + Gatsby (starting with version 4.4) can detect those node mutations. Unfortunately it adds measurable overhead so we don't enable it by default, and only let users to opt into it when they see data related issues (particularly when they didn't have this issue before using `LMDB`). To enable diagnostic mode: - use truthy environment variable `GATSBY_DETECT_NODE_MUTATIONS`: @@ -71,3 +75,43 @@ Be sure to stop using this mode once you find and handle all problematic code pa ## Migration ### Mutating a node in `onCreateNode` + +Instead of mutating node directly, `createNodeField` action should be used instead. This way we will update source of truth (actually update node in datastore). `createNodeField` will create that additional field under `fields.fieldName`. If you want to preserve schema shape, so that additional field is on the root of a node you can use schema customization. + +```diff +const { createRemoteFileNode } = require(`gatsby-source-filesystem`) + +exports.onCreateNode = async ({ + node, // the node that was just created +- actions: { createNode }, ++ actions: { createNode, createNodeField }, + createNodeId, + getCache, +}) => { + if (node.internal.type === `SomeNodeType`) { + const fileNode = await createRemoteFileNode({ + // the url of the remote image to generate a node for + url: node.imgUrl, + parentNodeId: node.id, + createNode, + createNodeId, + getCache, + }) + + if (fileNode) { +- node.localFile___NODE = fileNode.id ++ createNodeField({ node, name: 'localFile', value: fileNode.id }) + } + } +} ++ ++exports.createSchemaCustomization = ({ actions }) => { ++ const { createTypes } = actions ++ ++ createTypes(` ++ type SomeNodeType implements Node { ++ localFile: File @link(from: "fields.localFile") ++ } ++ `) ++} +``` From de72e9c6507007e6111324f37c1913e27f70b57d Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Tue, 7 Dec 2021 12:01:09 +0100 Subject: [PATCH 07/11] fix env var names, add flag description --- packages/gatsby/src/services/initialize.ts | 2 +- packages/gatsby/src/utils/detect-node-mutations.ts | 3 +-- packages/gatsby/src/utils/flags.ts | 3 +-- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/gatsby/src/services/initialize.ts b/packages/gatsby/src/services/initialize.ts index 6f8314a5a78ce..40dfe336e23d3 100644 --- a/packages/gatsby/src/services/initialize.ts +++ b/packages/gatsby/src/services/initialize.ts @@ -162,7 +162,7 @@ export async function initialize({ } const lmdbStoreIsUsed = detectLmdbStore() - if (process.env.GATSBY_EXPERIMENTAL_DETECT_NODE_MUTATIONS) { + if (process.env.GATSBY_DETECT_NODE_MUTATIONS) { enableNodeMutationsDetection() } diff --git a/packages/gatsby/src/utils/detect-node-mutations.ts b/packages/gatsby/src/utils/detect-node-mutations.ts index 4221a5127d4ba..619249510de46 100644 --- a/packages/gatsby/src/utils/detect-node-mutations.ts +++ b/packages/gatsby/src/utils/detect-node-mutations.ts @@ -130,8 +130,7 @@ function createProxyHandler({ } } -let shouldWrapNodesInProxies = - !!process.env.GATSBY_EXPERIMENTAL_DETECT_NODE_MUTATIONS +let shouldWrapNodesInProxies = !!process.env.GATSBY_DETECT_NODE_MUTATIONS export function enableNodeMutationsDetection(): void { shouldWrapNodesInProxies = true diff --git a/packages/gatsby/src/utils/flags.ts b/packages/gatsby/src/utils/flags.ts index d1c4f74b64a05..069a37cb68eeb 100644 --- a/packages/gatsby/src/utils/flags.ts +++ b/packages/gatsby/src/utils/flags.ts @@ -231,8 +231,7 @@ const activeFlags: Array = [ env: `GATSBY_DETECT_NODE_MUTATIONS`, command: `all`, telemetryId: `DetectNodeMutations`, - // TODO: description - description: ``, + description: `Diagnostic mode to log any attempts to mutate node directly. Helpful when debugging missing data problems. See http://gatsbyjs.org/docs/how-to/local-development/debugging-missing-data/ for more details.`, experimental: false, testFitness: (): fitnessEnum => true, }, From 1fa9186dcbd88f6fd9a54858068085572f8e1b04 Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Tue, 7 Dec 2021 12:08:07 +0100 Subject: [PATCH 08/11] update v3->v4 migration guide to mention new doc --- docs/docs/reference/release-notes/migrating-from-v3-to-v4.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/docs/reference/release-notes/migrating-from-v3-to-v4.md b/docs/docs/reference/release-notes/migrating-from-v3-to-v4.md index 07c09cba33e3b..4ab72ed62a4a7 100644 --- a/docs/docs/reference/release-notes/migrating-from-v3-to-v4.md +++ b/docs/docs/reference/release-notes/migrating-from-v3-to-v4.md @@ -583,6 +583,8 @@ This was never an intended feature of Gatsby and is considered an anti-pattern ( Starting with v4 Gatsby introduces a persisted storage for nodes and thus this pattern will no longer work because nodes are persisted after `createNode` call and all direct mutations after that will be lost. +Gatsby provides diagnostic mode to detect those direct mutations, unfortunately it has noticeable performance overhead so we don't enable it by default. See [Debugging missing data](http://gatsbyjs.org/docs/how-to/local-development/debugging-missing-data/) for more details on it. + Gatsby provides several actions available in `sourceNodes` and `onCreateNode` APIs to use instead: - [createNode](/docs/reference/config-files/actions/#createNode) @@ -591,8 +593,6 @@ Gatsby provides several actions available in `sourceNodes` and `onCreateNode` AP You can use `createNodeField` and the `@link` directive to create the same schema shape. The [`@link` directive](/docs/reference/graphql-data-layer/schema-customization/#foreign-key-fields) accepts a `from` argument that you can use to place your node to the old position (as `createNodeField` places everything under a `fields` key). See the [source plugin guide](/docs/how-to/plugins-and-themes/creating-a-source-plugin/#create-remote-file-node-from-a-url) for more information. Checkout [this PR](https://github.com/gatsbyjs/gatsby/pull/33715) for a real-world migration example. -Gatsby (starting with version `4.3.0`) can automatically detect those mutations but it introduces noticeable performance degradation and it's not enabled by default. To temporarily opt-in - set `DETECT_NODE_MUTATIONS: true` flag in `gatsby-config.js` or run Gatsby with `GATSBY_EXPERIMENTAL_DETECT_NODE_MUTATIONS=true` environment variable (please remember to disable it after you are done). It will log traces so it's easier to locate offending code. - ### `___NODE` convention Please note that the [deprecation of the `___NODE` convention](#___node-convention-is-deprecated) especially affects source plugins and for Gatsby v5 you'll need to update your usage to keep compatibility. From 4c6cdc46811142b2ea6de2940f34e946cc672ca7 Mon Sep 17 00:00:00 2001 From: Lennart Date: Tue, 7 Dec 2021 15:52:18 +0100 Subject: [PATCH 09/11] update docs --- .../debugging-missing-data.md | 38 ++++++------------- 1 file changed, 11 insertions(+), 27 deletions(-) diff --git a/docs/docs/how-to/local-development/debugging-missing-data.md b/docs/docs/how-to/local-development/debugging-missing-data.md index 1cf908d4b71df..ac4a03809da45 100644 --- a/docs/docs/how-to/local-development/debugging-missing-data.md +++ b/docs/docs/how-to/local-development/debugging-missing-data.md @@ -1,26 +1,26 @@ --- -title: Debugging missing or stale data fields on nodes +title: Debugging Missing or Stale Data Fields on Nodes --- ## Overview of `LMDB` datastore behavior changes -In Gatsby 3 we introduced new data store - `LMDB` (enabled with `LMDB_STORE` and/or `PARALLEL_QUERY_RUNNING` flags). In Gatsby 4 it became the default data store. It allows us to execute data layer related processing outside of main build process and enable us to run queries in multiple processes as well as support additional rendering strategies (DSG and SSR). +In Gatsby 3 we introduced a new data store called `LMDB` (enabled with `LMDB_STORE` and/or `PARALLEL_QUERY_RUNNING` flags). In Gatsby 4 it became the default data store. It allows Gatsby to execute data layer related processing outside of the main build process and enables Gatsby to run queries in multiple processes as well as support additional rendering strategies ([DSG](/docs/reference/rendering-options/deferred-static-generation/) and [SSR](/docs/reference/rendering-options/server-side-rendering/)). In a lot of cases that change is completely invisible to users, but there are cases where things behave differently. -Direct node mutations in various API lifecycles are not persisted anymore. In previous Gatsby versions it did work because source of truth for our data layer was directly in Node.js memory, so mutating node was in fact mutating source of truth. Now we edit data we received from database, so unless we explicitly upsert data to database after edits, those edits will not be persisted (even for same the same build). +Direct node mutations in various API lifecycles are not persisted anymore. In previous Gatsby versions it did work because source of truth for the data layer was directly in Node.js memory, so mutating node was in fact mutating source of truth. Now Gatsby edits data it receives from the database, so unless it explicitly upserts data to this database after edits, those edits will not be persisted (even for same the same build). -Common errors when doing swap to `LMDB` will be that some fields don't exist anymore or are `null`/`undefined` when trying to execute graphql queries. +Common errors when doing swap to `LMDB` will be that some fields don't exist anymore or are `null`/`undefined` when trying to execute GraphQL queries. ## Diagnostic mode -Gatsby (starting with version 4.4) can detect those node mutations. Unfortunately it adds measurable overhead so we don't enable it by default, and only let users to opt into it when they see data related issues (particularly when they didn't have this issue before using `LMDB`). To enable diagnostic mode: +Gatsby (starting with version 4.4) can detect those node mutations. Unfortunately it adds measurable overhead so it isn't enabled by default. You can opt into it when you see data related issues (particularly when you didn't have this issue before using `LMDB`). To enable diagnostic mode: -- use truthy environment variable `GATSBY_DETECT_NODE_MUTATIONS`: +- Use truthy environment variable `GATSBY_DETECT_NODE_MUTATIONS`: ``` GATSBY_DETECT_NODE_MUTATIONS=1 gatsby build ``` -- or use `DETECT_NODE_MUTATIONS` config flag: +- Or use `DETECT_NODE_MUTATIONS` config flag: ```javascript:title=gatsby-config.js module.exports = { flags: { @@ -49,34 +49,18 @@ Stack trace: at Promise.catch.decorateEvent.pluginName (/node_modules/gatsby/src/utils/api-runner-node.js:613:13) at Promise._execute -(/node_modules/bluebird/js/release/debuggability.js:384:9) - at Promise._resolveFromExecutor -(/node_modules/bluebird/js/release/promise.js:518:18) - at new Promise (/node_modules/bluebird/js/release/promise.js:103:10) - at /node_modules/gatsby/src/utils/api-runner-node.js:611:16 - at tryCatcher (/node_modules/bluebird/js/release/util.js:16:23) - at Object.gotValue (/node_modules/bluebird/js/release/reduce.js:166:18) - at Object.gotAccum (/node_modules/bluebird/js/release/reduce.js:155:25) - at Object.tryCatcher (/node_modules/bluebird/js/release/util.js:16:23) - at Promise._settlePromiseFromHandler -(/node_modules/bluebird/js/release/promise.js:547:31) - at Promise._settlePromise -(/node_modules/bluebird/js/release/promise.js:604:18) - at Promise._settlePromiseCtx -(/node_modules/bluebird/js/release/promise.js:641:10) - at _drainQueueStep (/node_modules/bluebird/js/release/async.js:97:12) - at _drainQueue (/node_modules/bluebird/js/release/async.js:86:9) + ... Rest of Stacktrace ``` -It will point to code that is trying to mutate nodes. Note that it might also point to installed plugins in which case plugin maintainer should be notified about it. +It will point you to the code that is trying to mutate nodes. Note that it might also point to installed plugins in which case you should notify the plugin maintainer about it. -Be sure to stop using this mode once you find and handle all problematic code paths as it will decrease performance. +**Please Note:** Be sure to stop using this mode once you find and handle all problematic code paths as it will decrease performance. ## Migration ### Mutating a node in `onCreateNode` -Instead of mutating node directly, `createNodeField` action should be used instead. This way we will update source of truth (actually update node in datastore). `createNodeField` will create that additional field under `fields.fieldName`. If you want to preserve schema shape, so that additional field is on the root of a node you can use schema customization. +Instead of mutating nodes directly, `createNodeField` action should be used instead. This way Gatsby will update the source of truth (to actually update the node in the datastore). `createNodeField` will create that additional field under `fields.fieldName`. If you want to preserve schema shape, so that additional field is on the root of a node, you can use schema customization. ```diff const { createRemoteFileNode } = require(`gatsby-source-filesystem`) From df973192f293d7c1b93166da2a9dded394f04929 Mon Sep 17 00:00:00 2001 From: Lennart Date: Tue, 7 Dec 2021 15:53:02 +0100 Subject: [PATCH 10/11] Update migrating-from-v3-to-v4.md --- docs/docs/reference/release-notes/migrating-from-v3-to-v4.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/docs/reference/release-notes/migrating-from-v3-to-v4.md b/docs/docs/reference/release-notes/migrating-from-v3-to-v4.md index 4ab72ed62a4a7..a74e69eb2380e 100644 --- a/docs/docs/reference/release-notes/migrating-from-v3-to-v4.md +++ b/docs/docs/reference/release-notes/migrating-from-v3-to-v4.md @@ -583,7 +583,7 @@ This was never an intended feature of Gatsby and is considered an anti-pattern ( Starting with v4 Gatsby introduces a persisted storage for nodes and thus this pattern will no longer work because nodes are persisted after `createNode` call and all direct mutations after that will be lost. -Gatsby provides diagnostic mode to detect those direct mutations, unfortunately it has noticeable performance overhead so we don't enable it by default. See [Debugging missing data](http://gatsbyjs.org/docs/how-to/local-development/debugging-missing-data/) for more details on it. +Gatsby provides diagnostic mode to detect those direct mutations, unfortunately it has noticeable performance overhead so we don't enable it by default. See [Debugging missing data](/docs/how-to/local-development/debugging-missing-data/) for more details on it. Gatsby provides several actions available in `sourceNodes` and `onCreateNode` APIs to use instead: From 5cccdc0c0b7d18b071e76e308e20bf84ec50dc22 Mon Sep 17 00:00:00 2001 From: Lennart Date: Tue, 7 Dec 2021 15:58:47 +0100 Subject: [PATCH 11/11] Update flags.ts --- packages/gatsby/src/utils/flags.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/gatsby/src/utils/flags.ts b/packages/gatsby/src/utils/flags.ts index 069a37cb68eeb..b700349955c15 100644 --- a/packages/gatsby/src/utils/flags.ts +++ b/packages/gatsby/src/utils/flags.ts @@ -231,7 +231,7 @@ const activeFlags: Array = [ env: `GATSBY_DETECT_NODE_MUTATIONS`, command: `all`, telemetryId: `DetectNodeMutations`, - description: `Diagnostic mode to log any attempts to mutate node directly. Helpful when debugging missing data problems. See http://gatsbyjs.org/docs/how-to/local-development/debugging-missing-data/ for more details.`, + description: `Diagnostic mode to log any attempts to mutate node directly. Helpful when debugging missing data problems. See https://gatsby.dev/debugging-missing-data for more details.`, experimental: false, testFitness: (): fitnessEnum => true, },