fix(gatsby): fix ceateNode hanging in custom resolvers #26716
Conversation
gatsbot
bot
added
the
status: triage needed
| @@ -1,6 +1,7 @@ | |||
| import { IQueryRunningContext } from "./types" | |||
| import { DoneInvokeEvent, assign, ActionFunctionMap } from "xstate" | |||
| import { enqueueFlush } from "../../utils/page-data" | |||
| import { callApi, markNodesDirty } from "../develop/actions" | |||
There was a problem hiding this comment.
Importing from develop actions doesn't feel like the correct thing to do. So I am open to suggestions here!
Gatsby Cloud Build Reportclient-only-paths 🎉 Your build was successful! See the Deploy preview here. Build Details🕐 Build time: 17s PerformanceLighthouse report
|
Gatsby Cloud Build Reportusing-styled-components 🎉 Your build was successful! See the Deploy preview here. Build Details🕐 Build time: 19s PerformanceLighthouse report
|
Gatsby Cloud Build Reportusing-reach-skip-nav 🎉 Your build was successful! See the Deploy preview here. Build Details🕐 Build time: 22s PerformanceLighthouse report
|
Gatsby Cloud Build Reportgatsby 🎉 Your build was successful! See the Deploy preview here. Build Details🕐 Build time: 21m |
vladar
removed
the
status: triage needed
|
Turns out we have specific tests for |
| @@ -14,6 +14,9 @@ export const queryStates: MachineConfig<IQueryRunningContext, any, any> = { | |||
| SOURCE_FILE_CHANGED: { | |||
| target: `extractingQueries`, | |||
| }, | |||
| ADD_NODE_MUTATION: { | |||
| actions: [`markNodesDirty`, `callApi`], | |||
| }, | |||
There was a problem hiding this comment.
Unfortunely this won't do what you expect. As this is a child machine, the action is run against the context of the child machine, so marking nodes as dirty won't do anything.
| @@ -101,6 +101,9 @@ const developConfig: MachineConfig<IBuildContext, any, AnyEventObject> = { | |||
| SOURCE_FILE_CHANGED: { | |||
| actions: [forwardTo(`run-queries`), `markSourceFilesDirty`], | |||
| }, | |||
| ADD_NODE_MUTATION: { | |||
| actions: [forwardTo(`run-queries`)], | |||
| }, | |||
There was a problem hiding this comment.
Rather than forwarding it to the child machine, it should run the action in the current machine. See the initializingData state for an example.
There was a problem hiding this comment.
The trouble here is that this is not supposed to run them immediately: the reason we do this deferral is so that nodes aren't mutated during query runs. Changing this would mean that we'd be back to how it was without any deferral. Now that might be what we want to do after all, but at that point it would be better to remove the whole deferral system.
I think we need a different way of solving this though, which is probably a more granular way of dealing with deferred resolver mutations, i.e. not waiting until waiting, but bailing and drainign earlier.
There was a problem hiding this comment.
Yeah, that's what I thought. This was just a quick and direct attempt to address this problem but I see that it defeats the whole purpose of node deferring. We can definitely have a call/chat tomorrow as I'd like to get more context (haha) on our state charts in general. CC @wardpeet
|
I am gonna close this PR. Looking forward for the follow-up. |
Description
A follow up for #25479 and #26138
This PR fixes a problem with query hanging when
createNodeis called and awaited within a custom resolver.Before this PR
createNodein a custom resolver was deferred until the machine reached thewaitingstate. But it couldn't reach this state because the custom resolver awaited forcreateNodeto complete and so the query could never finish and leave therunningQueriesstate.After this PR
createNodecalls (and other node mutation actions) are not deferred when the machine is in therunningQueriesstate.Caveat: One downside of it is that now if some node is created outside of the query running loop (i.e. in some watcher or subscription) but DURING query running - it won't be deferred.
Related Issues
Fixes #26530