From 43200722bbbfb011c1b96c6228cf3b6523f34c57 Mon Sep 17 00:00:00 2001 From: Vladimir Razuvaev Date: Wed, 30 Jun 2021 19:06:25 +0700 Subject: [PATCH] refactor(gatsby): simplified materialization a bit (#31882) * do not retain nodes when tracking inline objects * fix tests * simplify materialization a bit * add test demonstrating the edge case with querying by interface Co-authored-by: gatsbybot --- .../gatsby/src/schema/__tests__/node-model.js | 79 +++++++++++++++++-- packages/gatsby/src/schema/node-model.js | 55 ++++++------- 2 files changed, 102 insertions(+), 32 deletions(-) diff --git a/packages/gatsby/src/schema/__tests__/node-model.js b/packages/gatsby/src/schema/__tests__/node-model.js index 5b235720422f6..9fc7a4825cad9 100644 --- a/packages/gatsby/src/schema/__tests__/node-model.js +++ b/packages/gatsby/src/schema/__tests__/node-model.js @@ -646,6 +646,7 @@ describe(`NodeModel`, () => { describe(`materialization`, () => { let resolveBetterTitleMock let resolveOtherTitleMock + let resolveSlugMock beforeEach(async () => { const nodes = (() => [ { @@ -732,6 +733,7 @@ describe(`NodeModel`, () => { ) resolveBetterTitleMock = jest.fn() resolveOtherTitleMock = jest.fn() + resolveSlugMock = jest.fn() store.dispatch({ type: `CREATE_TYPES`, payload: [ @@ -795,7 +797,10 @@ describe(`NodeModel`, () => { }, slug: { type: `String`, - resolve: source => source.id, + resolve: source => { + resolveSlugMock() + return source.id + }, }, }, }), @@ -928,6 +933,70 @@ describe(`NodeModel`, () => { expect(resolveOtherTitleMock.mock.calls.length).toBe(2) }) + it(`should not resolve prepared nodes more than once (with mixed interfaces and node types)`, async () => { + nodeModel.replaceFiltersCache() + await nodeModel.runQuery( + { + query: { filter: { slug: { eq: `id1` } } }, + firstOnly: false, + type: `Test`, + }, + { path: `/` } + ) + expect(resolveSlugMock.mock.calls.length).toBe(2) + expect(resolveBetterTitleMock.mock.calls.length).toBe(0) + nodeModel.replaceFiltersCache() + await nodeModel.runQuery( + { + query: { filter: { slug: { eq: `id1` } } }, + firstOnly: false, + type: `TestInterface`, + }, + { path: `/` } + ) + expect(resolveSlugMock.mock.calls.length).toBe(2) + expect(resolveBetterTitleMock.mock.calls.length).toBe(0) + nodeModel.replaceFiltersCache() + await nodeModel.runQuery( + { + query: { + filter: { slug: { eq: `id1` }, betterTitle: { eq: `foo` } }, + }, + firstOnly: false, + type: `Test`, + }, + { path: `/` } + ) + expect(resolveSlugMock.mock.calls.length).toBe(2) + expect(resolveBetterTitleMock.mock.calls.length).toBe(2) + nodeModel.replaceFiltersCache() + await nodeModel.runQuery( + { + query: { + filter: { slug: { eq: `id1` } }, + }, + firstOnly: false, + type: `TestInterface`, + }, + { path: `/` } + ) + expect(resolveSlugMock.mock.calls.length).toBe(2) + expect(resolveBetterTitleMock.mock.calls.length).toBe(2) + nodeModel.replaceFiltersCache() + await nodeModel.runQuery( + { + query: { + filter: { slug: { eq: `id1` }, betterTitle: { eq: `foo` } }, + }, + firstOnly: true, + type: `Test`, + }, + { path: `/` } + ) + expect(resolveSlugMock.mock.calls.length).toBe(2) + expect(resolveBetterTitleMock.mock.calls.length).toBe(2) + }) + it(`can filter by resolved fields`, async () => { nodeModel.replaceFiltersCache() const result = await nodeModel.runQuery( @@ -1310,8 +1379,8 @@ describe(`NodeModel`, () => { const copiedInlineObject = { ...node.inlineObject } nodeModel.trackInlineObjectsInRootNode(copiedInlineObject) - expect(nodeModel._trackedRootNodes instanceof Set).toBe(true) - expect(nodeModel._trackedRootNodes.has(node.id)).toEqual(true) + expect(nodeModel._trackedRootNodes instanceof WeakSet).toBe(true) + expect(nodeModel._trackedRootNodes.has(node)).toEqual(true) }) }) describe(`not directly on a node`, () => { @@ -1354,8 +1423,8 @@ describe(`NodeModel`, () => { const copiedInlineObject = { ...node.inlineObject } nodeModel.trackInlineObjectsInRootNode(copiedInlineObject) - expect(nodeModel._trackedRootNodes instanceof Set).toBe(true) - expect(nodeModel._trackedRootNodes.has(node.id)).toEqual(true) + expect(nodeModel._trackedRootNodes instanceof WeakSet).toBe(true) + expect(nodeModel._trackedRootNodes.has(node)).toEqual(true) }) }) }) diff --git a/packages/gatsby/src/schema/node-model.js b/packages/gatsby/src/schema/node-model.js index a214557032158..9aeadc1e38b28 100644 --- a/packages/gatsby/src/schema/node-model.js +++ b/packages/gatsby/src/schema/node-model.js @@ -75,7 +75,7 @@ class LocalNodeModel { this.createPageDependencyActionCreator = createPageDependency this._rootNodeMap = new WeakMap() - this._trackedRootNodes = new Set() + this._trackedRootNodes = new WeakSet() this._prepareNodesQueues = {} this._prepareNodesPromises = {} this._preparedNodesCache = new Map() @@ -280,7 +280,10 @@ class LocalNodeModel { nodeTypeNames ) - await this.prepareNodes(gqlType, fields, fieldsToResolve, nodeTypeNames) + for (const nodeTypeName of nodeTypeNames) { + const gqlNodeType = this.schema.getType(nodeTypeName) + await this.prepareNodes(gqlNodeType, fields, fieldsToResolve) + } if (materializationActivity) { materializationActivity.end() @@ -391,7 +394,7 @@ class LocalNodeModel { return this.trackPageDependencies(first, pageDependencies) } - prepareNodes(type, queryFields, fieldsToResolve, nodeTypeNames) { + prepareNodes(type, queryFields, fieldsToResolve) { const typeName = type.name if (!this._prepareNodesQueues[typeName]) { this._prepareNodesQueues[typeName] = [] @@ -405,7 +408,7 @@ class LocalNodeModel { if (!this._prepareNodesPromises[typeName]) { this._prepareNodesPromises[typeName] = new Promise(resolve => { process.nextTick(async () => { - await this._doResolvePrepareNodesQueue(type, nodeTypeNames) + await this._doResolvePrepareNodesQueue(type) resolve() }) }) @@ -414,7 +417,7 @@ class LocalNodeModel { return this._prepareNodesPromises[typeName] } - async _doResolvePrepareNodesQueue(type, nodeTypeNames) { + async _doResolvePrepareNodesQueue(type) { const typeName = type.name const queue = this._prepareNodesQueues[typeName] this._prepareNodesQueues[typeName] = [] @@ -442,7 +445,8 @@ class LocalNodeModel { ) if (!_.isEmpty(actualFieldsToResolve)) { - await saveResolvedNodes(nodeTypeNames, async node => { + const resolvedNodes = new Map() + for (const node of getDataStore().iterateNodesByType(typeName)) { this.trackInlineObjectsInRootNode(node) const resolvedFields = await resolveRecursive( this, @@ -456,8 +460,14 @@ class LocalNodeModel { if (!node.__gatsby_resolved) { node.__gatsby_resolved = {} } - return _.merge(node.__gatsby_resolved, resolvedFields) - }) + resolvedNodes.set( + node.id, + _.merge(node.__gatsby_resolved, resolvedFields) + ) + } + if (resolvedNodes.size) { + await saveResolvedNodes(typeName, resolvedNodes) + } this._preparedNodesCache.set( typeName, _.merge( @@ -484,7 +494,7 @@ class LocalNodeModel { * @param {Node} node Root Node */ trackInlineObjectsInRootNode(node) { - if (!this._trackedRootNodes.has(node.id)) { + if (!this._trackedRootNodes.has(node)) { addRootNodeToInlineObject( this._rootNodeMap, node, @@ -492,7 +502,7 @@ class LocalNodeModel { true, new Set() ) - this._trackedRootNodes.add(node.id) + this._trackedRootNodes.add(node) } } @@ -940,23 +950,14 @@ const addRootNodeToInlineObject = ( } } -const saveResolvedNodes = async (nodeTypeNames, resolver) => { - for (const typeName of nodeTypeNames) { - const resolvedNodes = new Map() - for (const node of getDataStore().iterateNodesByType(typeName)) { - const resolved = await resolver(node) - resolvedNodes.set(node.id, resolved) - } - if (!resolvedNodes.size) continue - - store.dispatch({ - type: `SET_RESOLVED_NODES`, - payload: { - key: typeName, - nodes: resolvedNodes, - }, - }) - } +const saveResolvedNodes = async (typeName, resolvedNodes) => { + store.dispatch({ + type: `SET_RESOLVED_NODES`, + payload: { + key: typeName, + nodes: resolvedNodes, + }, + }) } const deepObjectDifference = (from, to) => {