Regression in #35271

[fshowalter]

Preliminary Checks

• This issue is not a duplicate. Before opening a new issue, please search existing issues: https://github.com/gatsbyjs/gatsby/issues
• This issue is not a question, feature request, RFC, or anything other than a bug report directly related to Gatsby. Please post those things in GitHub Discussions: https://github.com/gatsbyjs/gatsby/discussions

Description

#35271 introduced a regression where a node will be marked as resolved despite only a possible subset of resolvable fields actually being resolved. This is manifesting in my site where I have a query that runs DISTINCT over several resolvable fields. What it looks like is happening, is Gatsby is taking a SINGLE node and using it for the DISTINCT resolutions which in turn sets it into the "resolved" cache, but only the DISTINCT fields will be resolved. Subsequent queries for the non-resolved fields ON THAT NODE will fail, as Gatsby thinks they're already resolved.

Imagine a blog with a series of posts. Your index page query does a DISTINCT on a resolvable field. For a SINGLE post (whichever one Gatsby pulls--not sure how it picks) this field will resolve and Gatsby will add the node to the cache (line 501 in packages/gatsby/src/schema/node-model.js) with that field resolved but any other resolvable fields will be undefined.

Reproduction Link

fshowalter/franksbooklog.com#30

Steps to Reproduce

1. See above PR for failing example. It fails because a single MarkdownRemark node is marked as resolved, but only those fields used in DISTINCT in https://github.com/fshowalter/franksbooklog.com/blob/main/src/components/ReviewsIndexPage/ReviewsIndexPage.tsx are resolved. When further resolutions are needed ala https://github.com/fshowalter/franksbooklog.com/blob/c8ce17796951a234cd5c6477347198ed1e9be827/gatsby/node/schema/WorksJson.ts#L58 queries will fail as the node is marked as resolved with any other fields returning undefined.
2. To resolve, either drop this page, or add a distinct for any nodes you need to query.

Expected Result

A node should not be marked as globally resolved unless all fields are resolved.

Actual Result

A node is marked as resolved but only a subset of its fields are actually resolved.

Environment

System:
    OS: macOS 12.3.1
    CPU: (8) arm64 Apple M1 Pro
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 16.13.2 - /var/folders/py/50_9jf8n09q9wp46n8600pr80000gp/T/yarn--1649350640590-0.10265790193522317/node
    Yarn: 1.22.18 - /var/folders/py/50_9jf8n09q9wp46n8600pr80000gp/T/yarn--1649350640590-0.10265790193522317/yarn
    npm: 8.1.2 - ~/.nvm/versions/node/v16.13.2/bin/npm
  Browsers:
    Chrome: 100.0.4896.75
    Safari: 15.4
  npmPackages:
    gatsby: ^4.11.2 => 4.11.2 
    gatsby-cli: ^4.11.2 => 4.11.2 
    gatsby-plugin-catch-links: ^4.10.0 => 4.10.0 
    gatsby-plugin-feed: ^4.11.1 => 4.11.1 
    gatsby-plugin-image: ^2.11.1 => 2.11.1 
    gatsby-plugin-manifest: ^4.10.1 => 4.10.1 
    gatsby-plugin-preact: ^6.10.1 => 6.10.1 
    gatsby-plugin-react-helmet: ^5.9.0 => 5.9.0 
    gatsby-plugin-sass: ^5.9.0 => 5.9.0 
    gatsby-plugin-sharp: ^4.11.1 => 4.11.1 
    gatsby-plugin-sitemap: ^5.9.0 => 5.9.0 
    gatsby-remark-smartypants: ^5.9.0 => 5.9.0 
    gatsby-source-filesystem: ^4.9.1 => 4.9.1 
    gatsby-transformer-json: ^4.11.0 => 4.11.0 
    gatsby-transformer-remark: 5.9.0 => 5.9.0 
    gatsby-transformer-sharp: ^4.9.0 => 4.9.0

Config Flags

No response

[fshowalter]

I think it may be a race condition where

gatsby/packages/gatsby/src/schema/node-model.js

Line 452 in de86cb7

async _doResolvePrepareNodesQueue(type) {

_doResolvePrepareNodesQueue is executing for the same node twice, but on the second pass the redux store hasn't yet updated, so

gatsby/packages/gatsby/src/schema/node-model.js

Line 481 in de86cb7

const previouslyResolvedNodes = getResolvedNodes(typeName)

getResolvedNodes returns nothing so the merge doesn't happen and the second update clobbers the first. I can't seem to get logging to work inside the reducer to confirm, however. (redux treats any console.log lines in a reducer as a dispatch and errors out)

[pieh]

Thank you for doing research on this @fshowalter, will be looking into this one.

I think you are completely right - previouslyResolvedNodes is set early (before doing async work), so multiple concurrent materialization runs will overwrite previous results.

I can't seem to get logging to work inside the reducer to confirm, however. (redux treats any console.log lines in a reducer as a dispatch and errors out)

Right, sorry about this annoyance. You can use process.stdout.write("your stuff\n") to log in reducers (tho need to be a string, so JSON.stringify or util.inspect is needed to log objects etc.

[fshowalter]

@pieh Thanks. Mystery deepens, adding the logging I don't see any instances of SET_RESOLVED_NODES in the reducer.

[pieh]

@pieh Thanks. Mystery deepens, adding the logging I don't see any instances of SET_RESOLVED_NODES in the reducer.

Try running with CI=1 env var - this disables animation for spinners and progress bars (terminal "rewrites" for spinner/progress bar animations can "overwrite" and delete untracked terminal outputs that are done with process.stdout.write)

[pieh]

I started work on this - for now added failing test (based on research shared in #35366 (comment) )

I think other than fixing concurrency issue with seting resolved fields I will also spend some time on actual DISTINCT (and other aggregation fields we have). Right now it doesn't make sure that __gatsby_resolved field is there before trying to use it:

gatsby/packages/gatsby/src/schema/resolvers.ts

Lines 115 to 140 in de86cb7

	export const distinct: GatsbyResolver<
	IGatsbyConnection<IGatsbyNode>,
	IFieldConnectionArgs
	> = function distinctResolver(source, args): Array<string> {
	const { field } = args
	const { edges } = source
	const values = new Set<string>()
	edges.forEach(({ node }) => {
	const value =
	getValueAt(node, `__gatsby_resolved.${field}`) || getValueAt(node, field)
	if (value === null || value === undefined) {
	return
	}
	if (Array.isArray(value)) {
	value.forEach(subValue =>
	values.add(subValue instanceof Date ? subValue.toISOString() : subValue)
	)
	} else if (value instanceof Date) {
	values.add(value.toISOString())
	} else {
	values.add(value)
	}
	})
	return Array.from(values).sort()
	}

I think we do need to apply similar check as we have in

gatsby/packages/gatsby/src/datastore/in-memory/indexing.ts

Lines 362 to 369 in e24ec76

	if (!node.__gatsby_resolved) {
	const typeName = node.internal.type
	const resolvedNodes = resolvedNodesCache.get(typeName)
	const resolved = resolvedNodes?.get(node.id)
	if (resolved !== undefined) {
	node.__gatsby_resolved = resolved
	}
	}

The setup currently is quite error prone, I'll also try to think if we can use TypeScript to differentiate "raw" IGatsbyNode from a node with resolved fields set (and demand "decorated" node where we rely on __gatsby_resolved

[fshowalter]

Confirmed. I can see the two actions come through the reducer with the same node ids, but different values, and the second clobbers the first. The trick was adding that \n character to the stdout along with the CI env var to flush the buffer.

[fshowalter]

Thank you @pieh, you've made my Gatsby debugging life so much easier 😄

[fshowalter]

@pieh Just checking on this one. Anything I can do to help?