Skip to content

Commit

Permalink
fix(gatsby): Remove runQuery & getAllNodes from nodeModel (#36859)
Browse files Browse the repository at this point in the history
* update internals & tests

* update benchmarks/examples
  • Loading branch information
LekoArts authored Oct 24, 2022
1 parent 94d18fa commit 412c65f
Show file tree
Hide file tree
Showing 6 changed files with 54 additions and 204 deletions.
8 changes: 4 additions & 4 deletions benchmarks/source-agilitycms/src/agility/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,12 @@ const getLinkedContentList = ({ type, linkedContentFieldName }) => {
const fieldResolver =
{
type: [type],
resolve: (source, args, context, info) => {
const list = context.nodeModel.getAllNodes({ type });
const filteredList = list.filter(
resolve: async (source, args, context, info) => {
const { entries } = await context.nodeModel.findAll({ type })
const filteredList = entries.filter(
item => item.properties.referenceName === source.customFields[linkedContentFieldName].referencename
)
return filteredList;
return Array.from(filteredList);
}
}

Expand Down
35 changes: 15 additions & 20 deletions examples/using-type-definitions/gatsby-node.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,9 @@ exports.createResolvers = ({ createResolvers }) => {
// Create a new root query field.
allAuthorFullNames: {
type: [`String!`],
resolve(source, args, context, info) {
const authors = context.nodeModel.getAllNodes({
type: `AuthorJson`,
})
return authors.map(author => `${author.name}, ${author.firstName}`)
async resolve(source, args, context, info) {
const { entries } = await context.nodeModel.findAll({ type: `AuthorJson` })
return Array.from(entries, author => `${author.name}, ${author.firstName}`)
},
},
// Field resolvers can use all of Gatsby's querying capabilities
Expand All @@ -91,39 +89,36 @@ exports.createResolvers = ({ createResolvers }) => {
// when no type definitions are provided wth `createTypes`.
posts: {
type: [`BlogJson`],
resolve(source, args, context, info) {
async resolve(source, args, context, info) {
// We use an author's `email` as foreign key in `BlogJson.authors`
const fieldValue = source.email

const posts = context.nodeModel.getAllNodes({
type: `BlogJson`,
})
return posts.filter(post =>
const { entries } = await context.nodeModel.findAll({ type: `BlogJson` })
const posts = entries.filter(post =>
(post.authors || []).some(author => author === fieldValue)
)
return Array.from(posts)
},
},
},
BlogJson: {
// Add a resolver to a field defined with `createTypes`.
authors: {
resolve(source, args, context, info) {
async resolve(source, args, context, info) {
const emails = source[info.fieldName]
if (emails == null) return null

const authors = context.nodeModel.getAllNodes({
type: `AuthorJson`,
})
return authors.filter(author => emails.includes(author.email))
const { entries } = await context.nodeModel.findAll({ type: `AuthorJson` })
const authors = entries.filter(author => emails.includes(author.email))
return Array.from(authors)
},
},
comments: {
type: `[CommentJson!]!`,
resolve(source, args, context, info) {
const result = context.nodeModel.getAllNodes({
type: `CommentJson`,
})
return result.filter(({ blog }) => blog === source.id)
async resolve(source, args, context, info) {
const { entries } = await context.nodeModel.findAll({ type: `CommentJson` })
const result = entries.filter(({ blog }) => blog === source.id)
return Array.from(result)
},
},
},
Expand Down
8 changes: 6 additions & 2 deletions packages/gatsby/src/query/__tests__/data-tracking.js
Original file line number Diff line number Diff line change
Expand Up @@ -1340,8 +1340,12 @@ describe(`query caching between builds`, () => {
},
barList: {
type: [`Bar`],
resolve: (value, args, context) =>
context.nodeModel.getAllNodes({ type: `Bar` }),
resolve: async (value, args, context) => {
const { entries } = await context.nodeModel.findAll({
type: `Bar`,
})
return entries
},
},
},
interfaces: [`Node`],
Expand Down
98 changes: 15 additions & 83 deletions packages/gatsby/src/schema/__tests__/node-model.js
Original file line number Diff line number Diff line change
Expand Up @@ -232,81 +232,6 @@ describe(`NodeModel`, () => {
})
})

describe(`getAllNodes`, () => {
it(`returns all nodes`, () => {
const result = nodeModel.getAllNodes()
expect(result.length).toBe(9)
})

it(`returns all nodes of type`, () => {
const result = nodeModel.getAllNodes({ type: `Author` })
expect(result.length).toBe(2)
})

it(`returns all nodes of union type`, () => {
const result = nodeModel.getAllNodes({ type: `AllFiles` })
expect(result.length).toBe(3)
})

it(`returns all nodes of interface type`, () => {
const result = nodeModel.getAllNodes({ type: `TeamMember` })
expect(result.length).toBe(3)
})

it(`creates page dependencies with all connection types`, () => {
nodeModel.getAllNodes({}, { path: `/` })
allNodeTypes.forEach(typeName => {
expect(createPageDependency).toHaveBeenCalledWith({
path: `/`,
connection: typeName,
})
})
expect(createPageDependency).toHaveBeenCalledTimes(allNodeTypes.length)
})

it(`creates page dependencies when called with context and connection type`, () => {
nodeModel
.withContext({ path: `/` })
.getAllNodes({ type: `Post` }, { connectionType: `Post` })
expect(createPageDependency).toHaveBeenCalledTimes(1)
expect(createPageDependency).toHaveBeenCalledWith({
path: `/`,
connection: `Post`,
})
})

it(`creates page dependencies with all connection types when called with context without connection type`, () => {
nodeModel.withContext({ path: `/` }).getAllNodes()
allNodeTypes.forEach(typeName => {
expect(createPageDependency).toHaveBeenCalledWith({
path: `/`,
connection: typeName,
})
})
expect(createPageDependency).toHaveBeenCalledTimes(allNodeTypes.length)
})

it(`allows to opt-out of automatic dependency tracking`, () => {
nodeModel.getAllNodes({}, { path: `/`, track: false })
expect(createPageDependency).not.toHaveBeenCalled()
})

it(`allows to opt-out of automatic dependency tracking with context`, () => {
nodeModel.withContext({ path: `/` }).getAllNodes({}, { track: false })
expect(createPageDependency).not.toHaveBeenCalled()
})

it(`returns empty array when no nodes of type found`, () => {
const result = nodeModel.getAllNodes({ type: `Astronauts` })
expect(result).toEqual([])
})

it(`does not create page dependencies when no matching nodes found`, () => {
nodeModel.getAllNodes({ type: `Astronauts` }, { path: `/` })
expect(createPageDependency).not.toHaveBeenCalled()
})
})

describe(`getTypes`, () => {
it(`returns all node types in the store`, () => {
const result = nodeModel.getTypes()
Expand All @@ -323,7 +248,7 @@ describe(`NodeModel`, () => {
})
})

describe(`runQuery`, () => {
describe(`findOne/findAll`, () => {
it(`returns first result only`, async () => {
const type = `Post`
const query = {
Expand Down Expand Up @@ -1296,8 +1221,9 @@ describe(`NodeModel`, () => {
// }

// we should have resolved fields for all nodes
const { entries } = await nodeModel.findAll({ type: `Test` })
expect(Array.from(resolvedFieldsForTestNodes.keys())).toEqual(
nodeModel.getAllNodes({ type: `Test` }).map(node => node.id)
Array.from(entries, node => node.id)
)

// we should have all fields merged on all nodes
Expand Down Expand Up @@ -1442,22 +1368,28 @@ describe(`NodeModel`, () => {
})

describe(`Tracks nodes read from cache by list`, () => {
it(`Tracks inline objects`, () => {
const node = nodeModel.getAllNodes({ type: `Test` })[0]
it(`Tracks inline objects`, async () => {
const { entries } = await nodeModel.findAll({ type: `Test` })
const nodes = Array.from(entries)
const node = nodes[0]
const inlineObject = node.inlineObject
const trackedRootNode = nodeModel.findRootNodeAncestor(inlineObject)

expect(trackedRootNode).toEqual(node)
})
it(`Tracks inline arrays`, () => {
const node = nodeModel.getAllNodes({ type: `Test` })[0]
it(`Tracks inline arrays`, async () => {
const { entries } = await nodeModel.findAll({ type: `Test` })
const nodes = Array.from(entries)
const node = nodes[0]
const inlineObject = node.inlineArray
const trackedRootNode = nodeModel.findRootNodeAncestor(inlineObject)

expect(trackedRootNode).toEqual(node)
})
it(`Doesn't track copied objects`, () => {
const node = nodeModel.getAllNodes({ type: `Test` })[0]
it(`Doesn't track copied objects`, async () => {
const { entries } = await nodeModel.findAll({ type: `Test` })
const nodes = Array.from(entries)
const node = nodes[0]
const copiedInlineObject = { ...node.inlineObject }
const trackedRootNode =
nodeModel.findRootNodeAncestor(copiedInlineObject)
Expand Down
22 changes: 14 additions & 8 deletions packages/gatsby/src/schema/__tests__/queries.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ describe(`Query schema`, () => {
args[0].createResolvers({
Frontmatter: {
authors: {
resolve(source, args, context, info) {
async resolve(source, args, context, info) {
// NOTE: When using the first field resolver argument (here called
// `source`, also called `parent` or `root`), it is important to
// take care of the fact that the resolver can be called more than once
Expand All @@ -123,9 +123,13 @@ describe(`Query schema`, () => {
) {
return source.authors
}
return context.nodeModel
.getAllNodes({ type: `Author` })
.filter(author => source.authors.includes(author.email))
const { entries } = await context.nodeModel.findAll({
type: `Author`,
})
const authors = entries.filter(author =>
source.authors.includes(author.email)
)
return Array.from(authors)
},
},
},
Expand Down Expand Up @@ -154,10 +158,12 @@ describe(`Query schema`, () => {
Query: {
allAuthorNames: {
type: `[String!]!`,
resolve(source, args, context, info) {
return context.nodeModel
.getAllNodes({ type: `Author` })
.map(author => author.name)
async resolve(source, args, context, info) {
const { entries } = await context.nodeModel.findAll({
type: `Author`,
})

return Array.from(entries, author => author.name)
},
},
},
Expand Down
87 changes: 0 additions & 87 deletions packages/gatsby/src/schema/node-model.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,14 +54,6 @@ export interface NodeModel {
{ ids: Array<string>, type?: TypeOrTypeName },
pageDependencies?: PageDependencies
): Array<any>;
getAllNodes(
{ type?: TypeOrTypeName },
pageDependencies?: PageDependencies
): Array<any>;
runQuery(
args: QueryArguments,
pageDependencies?: PageDependencies
): Promise<any>;
getTypes(): Array<string>;
trackPageDependencies<nodeOrNodes: Node | Node[]>(
result: nodeOrNodes,
Expand Down Expand Up @@ -190,71 +182,6 @@ class LocalNodeModel {
return wrapNodes(this.trackPageDependencies(result, pageDependencies))
}

/**
* Get all nodes in the store, or all nodes of a specified type. Note that
* this adds connectionType tracking by default if type is passed.
*
* @deprecated Since version 4.0 - Use nodeModel.findAll() instead
* @param {Object} args
* @param {(string|GraphQLOutputType)} [args.type] Optional type of the nodes
* @param {PageDependencies} [pageDependencies]
* @returns {Node[]}
*/
getAllNodes(args, pageDependencies = {}) {
// TODO(v5): Remove API
reportOnce(
`nodeModel.getAllNodes() is deprecated. Use nodeModel.findAll() with an empty query instead.`
)
const { type = `Node` } = args || {}

let result
if (type === `Node`) {
result = getNodes()
} else {
const nodeTypeNames = toNodeTypeNames(this.schema, type)
const nodesByType = nodeTypeNames.map(typeName =>
getNodesByType(typeName)
)
const nodes = [].concat(...nodesByType)
result = nodes.filter(Boolean)
}

if (result) {
result.forEach(node => this.trackInlineObjectsInRootNode(node))
}

if (typeof pageDependencies.connectionType === `undefined`) {
pageDependencies.connectionType =
typeof type === `string` ? type : type.name
}

return wrapNodes(this.trackPageDependencies(result, pageDependencies))
}

/**
* Get nodes of a type matching the specified query.
*
* @deprecated Since version 4.0 - Use nodeModel.findAll() or nodeModel.findOne() instead
* @param {Object} args
* @param {Object} args.query Query arguments (`filter` and `sort`)
* @param {(string|GraphQLOutputType)} args.type Type
* @param {boolean} [args.firstOnly] If true, return only first match
* @param {PageDependencies} [pageDependencies]
* @returns {Promise<Node[]>}
*/
async runQuery(args, pageDependencies = {}) {
// TODO(v5): Remove API
reportOnce(
`nodeModel.runQuery() is deprecated. Use nodeModel.findAll() or nodeModel.findOne() instead.`
)
if (args.firstOnly) {
return this.findOne(args, pageDependencies)
}
const { skip, limit, ...query } = args.query
const result = await this.findAll({ ...args, query }, pageDependencies)
return Array.from(result.entries)
}

async _query(args) {
const { query = {}, type, stats, tracer } = args || {}

Expand Down Expand Up @@ -632,20 +559,6 @@ class ContextualNodeModel {
)
}

getAllNodes(args, pageDependencies) {
return this.nodeModel.getAllNodes(
args,
this._getFullDependencies(pageDependencies)
)
}

runQuery(args, pageDependencies) {
return this.nodeModel.runQuery(
args,
this._getFullDependencies(pageDependencies)
)
}

findOne(args, pageDependencies) {
return this.nodeModel.findOne(
args,
Expand Down

0 comments on commit 412c65f

Please sign in to comment.