Skip to content

Commit

Permalink
refactor(gatsby): simplified materialization a bit (#31882)
Browse files Browse the repository at this point in the history
* 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 <[email protected]>
  • Loading branch information
vladar and gatsbybot authored Jun 30, 2021
1 parent a2224ab commit 4320072
Show file tree
Hide file tree
Showing 2 changed files with 102 additions and 32 deletions.
79 changes: 74 additions & 5 deletions packages/gatsby/src/schema/__tests__/node-model.js
Original file line number Diff line number Diff line change
Expand Up @@ -646,6 +646,7 @@ describe(`NodeModel`, () => {
describe(`materialization`, () => {
let resolveBetterTitleMock
let resolveOtherTitleMock
let resolveSlugMock
beforeEach(async () => {
const nodes = (() => [
{
Expand Down Expand Up @@ -732,6 +733,7 @@ describe(`NodeModel`, () => {
)
resolveBetterTitleMock = jest.fn()
resolveOtherTitleMock = jest.fn()
resolveSlugMock = jest.fn()
store.dispatch({
type: `CREATE_TYPES`,
payload: [
Expand Down Expand Up @@ -795,7 +797,10 @@ describe(`NodeModel`, () => {
},
slug: {
type: `String`,
resolve: source => source.id,
resolve: source => {
resolveSlugMock()
return source.id
},
},
},
}),
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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`, () => {
Expand Down Expand Up @@ -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)
})
})
})
Expand Down
55 changes: 28 additions & 27 deletions packages/gatsby/src/schema/node-model.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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] = []
Expand All @@ -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()
})
})
Expand All @@ -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] = []
Expand Down Expand Up @@ -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,
Expand All @@ -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(
Expand All @@ -484,15 +494,15 @@ class LocalNodeModel {
* @param {Node} node Root Node
*/
trackInlineObjectsInRootNode(node) {
if (!this._trackedRootNodes.has(node.id)) {
if (!this._trackedRootNodes.has(node)) {
addRootNodeToInlineObject(
this._rootNodeMap,
node,
node.id,
true,
new Set()
)
this._trackedRootNodes.add(node.id)
this._trackedRootNodes.add(node)
}
}

Expand Down Expand Up @@ -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) => {
Expand Down

0 comments on commit 4320072

Please sign in to comment.