-
Notifications
You must be signed in to change notification settings - Fork 10.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(gatsby): Defer node mutation during querying #25479
Changes from 47 commits
1193d2e
696d5fa
77b7f3b
6f97db6
36e8ef4
07d0741
cd3a909
d2b615b
01280a9
d049022
2dad129
1df526a
26420c4
637c642
4ff46bd
7bc5056
cd82b3f
6f0ab66
2ce7b77
133099d
e7f426e
7da48a6
8f497be
a4ac2dc
6ca668f
b701d7c
d3e7a59
ae0ece9
28e7691
896a2af
186eb0c
c0acfe1
12d5f20
78463b7
8987299
dc3fb42
fa05e0d
4e8609f
425eafc
aadc60b
33dc4fa
50faf93
f9bdf3c
059798b
da0b2ce
e9bf259
2116857
e514a76
9edd938
a32dc5f
b126a99
95e0291
9496046
1cab0be
6966e76
00a975d
087ed94
c807968
69f9a7a
523478a
b814215
38c05f5
98936a8
3f758f5
f12ceec
e8ddb4d
e450d7f
0d672fc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,16 +3,10 @@ import reporter from "gatsby-cli/lib/reporter" | |
import chalk from "chalk" | ||
import telemetry from "gatsby-telemetry" | ||
import express from "express" | ||
import { bootstrapSchemaHotReloader } from "../bootstrap/schema-hot-reloader" | ||
import bootstrapPageHotReloader from "../bootstrap/page-hot-reloader" | ||
import { initTracer } from "../utils/tracer" | ||
import db from "../db" | ||
import { detectPortInUseAndPrompt } from "../utils/detect-port-in-use-and-prompt" | ||
import onExit from "signal-exit" | ||
import queryUtil from "../query" | ||
import queryWatcher from "../query/query-watcher" | ||
import * as requiresWriter from "../bootstrap/requires-writer" | ||
import { waitUntilAllJobsComplete } from "../utils/wait-until-jobs-complete" | ||
import { | ||
userPassesFeedbackRequestHeuristic, | ||
showFeedbackRequest, | ||
|
@@ -22,32 +16,38 @@ import { markWebpackStatusAsPending } from "../utils/webpack-status" | |
|
||
import { IProgram } from "./types" | ||
import { | ||
startWebpackServer, | ||
writeOutRequires, | ||
IBuildContext, | ||
initialize, | ||
postBootstrap, | ||
rebuildSchemaWithSitePage, | ||
writeOutRedirects, | ||
startWebpackServer, | ||
} from "../services" | ||
import { boundActionCreators } from "../redux/actions" | ||
import { ProgramStatus } from "../redux/types" | ||
import { | ||
MachineConfig, | ||
AnyEventObject, | ||
assign, | ||
Machine, | ||
DoneEventObject, | ||
interpret, | ||
Actor, | ||
Interpreter, | ||
forwardTo, | ||
State, | ||
} from "xstate" | ||
import { DataLayerResult, dataLayerMachine } from "../state-machines/data-layer" | ||
import { dataLayerMachine } from "../state-machines/data-layer" | ||
import { IDataLayerContext } from "../state-machines/data-layer/types" | ||
import { globalTracer } from "opentracing" | ||
import { IQueryRunningContext } from "../state-machines/query-running/types" | ||
import { queryRunningMachine } from "../state-machines/query-running" | ||
import { IWaitingContext } from "../state-machines/waiting/types" | ||
import { | ||
ADD_NODE_MUTATION, | ||
runMutationAndMarkDirty, | ||
QUERY_FILE_CHANGED, | ||
WEBHOOK_RECEIVED, | ||
} from "../state-machines/shared-transition-configs" | ||
import { buildActions } from "../state-machines/actions" | ||
import { waitingMachine } from "../state-machines/waiting" | ||
|
||
const tracer = globalTracer() | ||
|
||
|
@@ -85,6 +85,7 @@ process.on(`message`, msg => { | |
}) | ||
|
||
module.exports = async (program: IProgram): Promise<void> => { | ||
reporter.setVerbose(program.verbose) | ||
const bootstrapSpan = tracer.startSpan(`bootstrap`) | ||
|
||
// We want to prompt the feedback request when users quit develop | ||
|
@@ -139,27 +140,43 @@ module.exports = async (program: IProgram): Promise<void> => { | |
src: `initialize`, | ||
onDone: { | ||
target: `initializingDataLayer`, | ||
actions: `assignStoreAndWorkerPool`, | ||
actions: [`assignStoreAndWorkerPool`, `spawnMutationListener`], | ||
}, | ||
}, | ||
}, | ||
initializingDataLayer: { | ||
on: { | ||
ADD_NODE_MUTATION: runMutationAndMarkDirty, | ||
}, | ||
invoke: { | ||
src: `initializeDataLayer`, | ||
data: ({ parentSpan, store }: IBuildContext): IDataLayerContext => { | ||
return { parentSpan, store, firstRun: true } | ||
data: ({ | ||
parentSpan, | ||
store, | ||
firstRun, | ||
webhookBody, | ||
}: IBuildContext): IDataLayerContext => { | ||
return { | ||
parentSpan, | ||
store, | ||
firstRun, | ||
deferNodeMutation: true, | ||
webhookBody, | ||
} | ||
}, | ||
onDone: { | ||
actions: `assignDataLayer`, | ||
actions: [`assignServiceResult`, `clearWebhookBody`], | ||
wardpeet marked this conversation as resolved.
Show resolved
Hide resolved
|
||
target: `finishingBootstrap`, | ||
}, | ||
}, | ||
}, | ||
finishingBootstrap: { | ||
on: { | ||
ADD_NODE_MUTATION: runMutationAndMarkDirty, | ||
WEBHOOK_RECEIVED, | ||
}, | ||
invoke: { | ||
src: async ({ | ||
gatsbyNodeGraphQLFunction, | ||
}: IBuildContext): Promise<void> => { | ||
src: async (): Promise<void> => { | ||
// These were previously in `bootstrap()` but are now | ||
// in part of the state machine that hasn't been added yet | ||
await rebuildSchemaWithSitePage({ parentSpan: bootstrapSpan }) | ||
|
@@ -168,30 +185,29 @@ module.exports = async (program: IProgram): Promise<void> => { | |
|
||
startRedirectListener() | ||
bootstrapSpan.finish() | ||
await postBootstrap({ parentSpan: bootstrapSpan }) | ||
|
||
// These are the parts that weren't in bootstrap | ||
|
||
// Start the createPages hot reloader. | ||
bootstrapPageHotReloader(gatsbyNodeGraphQLFunction) | ||
|
||
// Start the schema hot reloader. | ||
bootstrapSchemaHotReloader() | ||
}, | ||
onDone: { | ||
target: `runningQueries`, | ||
}, | ||
}, | ||
}, | ||
runningQueries: { | ||
on: { | ||
ADD_NODE_MUTATION, | ||
QUERY_FILE_CHANGED: { | ||
actions: forwardTo(`run-queries`), | ||
}, | ||
}, | ||
invoke: { | ||
id: `run-queries`, | ||
src: `runQueries`, | ||
data: ({ | ||
program, | ||
store, | ||
parentSpan, | ||
gatsbyNodeGraphQLFunction, | ||
graphqlRunner, | ||
websocketManager, | ||
firstRun, | ||
}: IBuildContext): IQueryRunningContext => { | ||
return { | ||
|
@@ -201,6 +217,7 @@ module.exports = async (program: IProgram): Promise<void> => { | |
parentSpan, | ||
gatsbyNodeGraphQLFunction, | ||
graphqlRunner, | ||
websocketManager, | ||
} | ||
}, | ||
onDone: { | ||
|
@@ -209,29 +226,87 @@ module.exports = async (program: IProgram): Promise<void> => { | |
}, | ||
}, | ||
doingEverythingElse: { | ||
wardpeet marked this conversation as resolved.
Show resolved
Hide resolved
|
||
on: { | ||
QUERY_FILE_CHANGED, | ||
ADD_NODE_MUTATION, | ||
WEBHOOK_RECEIVED, | ||
}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I find this pattern a bit confusing because:
Can we instead of exporting So for example: +/**
+ * Shared event hander for `QUERY_FILE_CHANGED` event that applies in most of the cases
+ */
-export const QUERY_FILE_CHANGED = {
+export const markQueryFilesDirtySideEffect = {
actions: `markQueryFilesDirty`,
} and on: {
- QUERY_FILE_CHANGED,
+ QUERY_FILE_CHANGED: markQueryFilesDirtySideEffect,
}, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point. Terseness got the better of me. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Other thought is if those are shared, could we use "global transitions" (not sure what they are called in xstate) like in https://xstate.js.org/viz/?gist=610bc765e60b10833e63ade8229860ef where we have general handler on the machine itself (instead of on states) and then for specific states we have overwrites? I do still like explicitness better (tho we might end up in situations where action is not handled at all if we forget about handling those in some states - that's easy to miss) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, that's the best way. I'll do that. |
||
invoke: { | ||
src: async ({ workerPool, store, app }): Promise<void> => { | ||
src: async (): Promise<void> => { | ||
// All the stuff that's not in the state machine yet | ||
|
||
await writeOutRequires({ store }) | ||
boundActionCreators.setProgramStatus( | ||
ProgramStatus.BOOTSTRAP_QUERY_RUNNING_FINISHED | ||
) | ||
|
||
await db.saveState() | ||
|
||
await waitUntilAllJobsComplete() | ||
requiresWriter.startListener() | ||
db.startAutosave() | ||
ascorbic marked this conversation as resolved.
Show resolved
Hide resolved
|
||
queryUtil.startListeningToDevelopQueue({ | ||
graphqlTracing: program.graphqlTracing, | ||
}) | ||
queryWatcher.startWatchDeletePage() | ||
|
||
await startWebpackServer({ program, app, workerPool }) | ||
}, | ||
onDone: [ | ||
{ | ||
target: `startingDevServers`, | ||
cond: ({ compiler }: IBuildContext): boolean => !compiler, | ||
ascorbic marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}, | ||
{ | ||
target: `waiting`, | ||
}, | ||
], | ||
}, | ||
}, | ||
startingDevServers: { | ||
on: { | ||
QUERY_FILE_CHANGED, | ||
ADD_NODE_MUTATION, | ||
}, | ||
invoke: { | ||
src: `startWebpackServer`, | ||
onDone: { | ||
actions: assign<IBuildContext, any>({ firstRun: false }), | ||
target: `waiting`, | ||
actions: `assignServers`, | ||
}, | ||
}, | ||
}, | ||
waiting: { | ||
on: { | ||
WEBHOOK_RECEIVED, | ||
ADD_NODE_MUTATION: { | ||
actions: forwardTo(`waiting`), | ||
}, | ||
QUERY_FILE_CHANGED: { | ||
actions: forwardTo(`waiting`), | ||
}, | ||
EXTRACT_QUERIES_NOW: { | ||
target: `runningQueries`, | ||
}, | ||
}, | ||
invoke: { | ||
id: `waiting`, | ||
src: `waitForMutations`, | ||
data: ({ | ||
store, | ||
nodeMutationBatch = [], | ||
}: IBuildContext): IWaitingContext => { | ||
return { store, nodeMutationBatch } | ||
}, | ||
onDone: { | ||
actions: `assignServiceResult`, | ||
target: `rebuildingPages`, | ||
}, | ||
}, | ||
}, | ||
rebuildingPages: { | ||
on: { | ||
ADD_NODE_MUTATION, | ||
}, | ||
invoke: { | ||
src: `initializeDataLayer`, | ||
data: ({ parentSpan, store }: IBuildContext): IDataLayerContext => { | ||
return { parentSpan, store, firstRun: false, skipSourcing: true } | ||
}, | ||
onDone: { | ||
actions: `assignServiceResult`, | ||
target: `runningQueries`, | ||
}, | ||
}, | ||
}, | ||
|
@@ -244,21 +319,10 @@ module.exports = async (program: IProgram): Promise<void> => { | |
initializeDataLayer: dataLayerMachine, | ||
initialize, | ||
runQueries: queryRunningMachine, | ||
waitForMutations: waitingMachine, | ||
startWebpackServer: startWebpackServer, | ||
}, | ||
actions: { | ||
assignStoreAndWorkerPool: assign<IBuildContext, DoneEventObject>( | ||
(_context, event) => { | ||
const { store, workerPool } = event.data | ||
return { | ||
store, | ||
workerPool, | ||
} | ||
} | ||
), | ||
assignDataLayer: assign<IBuildContext, DoneEventObject>( | ||
(_, { data }): DataLayerResult => data | ||
), | ||
}, | ||
actions: buildActions, | ||
}).withContext({ program, parentSpan: bootstrapSpan, app, firstRun: true }) | ||
) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,7 +8,6 @@ | |
* - Whenever a query changes, re-run all pages that rely on this query. | ||
***/ | ||
|
||
const _ = require(`lodash`) | ||
const chokidar = require(`chokidar`) | ||
|
||
const path = require(`path`) | ||
|
@@ -196,8 +195,7 @@ exports.extractQueries = ({ parentSpan } = {}) => { | |
// During development start watching files to recompile & run | ||
// queries on the fly. | ||
|
||
// TODO: move this into a spawned service, and emit events rather than | ||
// directly triggering the compilation | ||
// TODO: move this into a spawned service | ||
if (process.env.NODE_ENV !== `production`) { | ||
watch(store.getState().program.directory) | ||
} | ||
|
@@ -222,10 +220,6 @@ const watchComponent = componentPath => { | |
} | ||
} | ||
|
||
const debounceCompile = _.debounce(() => { | ||
updateStateAndRunQueries() | ||
}, 100) | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this debounce completely removed or just moved around? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't need to debounce anymore, because we're not actually compiling on these events, just dispatching an event to the state machine to say that we'd like to compile when ready. |
||
const watch = async rootDir => { | ||
if (watcher) return | ||
|
||
|
@@ -238,13 +232,18 @@ const watch = async rootDir => { | |
}) | ||
|
||
watcher = chokidar | ||
.watch([ | ||
slash(path.join(rootDir, `/src/**/*.{js,jsx,ts,tsx}`)), | ||
...packagePaths, | ||
]) | ||
.watch( | ||
[slash(path.join(rootDir, `/src/**/*.{js,jsx,ts,tsx}`)), ...packagePaths], | ||
{ ignoreInitial: true } | ||
) | ||
.on(`change`, path => { | ||
report.pendingActivity({ id: `query-extraction` }) | ||
debounceCompile() | ||
emitter.emit(`QUERY_FILE_CHANGED`, path) | ||
}) | ||
.on(`add`, path => { | ||
emitter.emit(`QUERY_FILE_CHANGED`, path) | ||
}) | ||
.on(`unlink`, path => { | ||
emitter.emit(`QUERY_FILE_CHANGED`, path) | ||
pvdz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}) | ||
wardpeet marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
filesToWatch.forEach(filePath => watcher.add(filePath)) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,18 +1,24 @@ | ||
import { calcInitialDirtyQueryIds, groupQueryIds } from "../query" | ||
import { | ||
calcInitialDirtyQueryIds, | ||
calcDirtyQueryIds, | ||
groupQueryIds, | ||
} from "../query" | ||
import { IGroupedQueryIds } from "./" | ||
import { IQueryRunningContext } from "../state-machines/query-running/types" | ||
import { assertStore } from "../utils/assert-store" | ||
|
||
export async function calculateDirtyQueries({ | ||
store, | ||
firstRun, | ||
}: Partial<IQueryRunningContext>): Promise<{ | ||
queryIds: IGroupedQueryIds | ||
}> { | ||
assertStore(store) | ||
|
||
const state = store.getState() | ||
// TODO: Check filesDirty from context | ||
|
||
const queryIds = calcInitialDirtyQueryIds(state) | ||
const queryIds = firstRun | ||
? calcInitialDirtyQueryIds(state) | ||
: calcDirtyQueryIds(state) | ||
Comment on lines
+20
to
+22
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we do the firstRun guard inside the statemachine instead of here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I think you're right: this should be two different services. |
||
return { queryIds: groupQueryIds(queryIds) } | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mutationlistener feels to me that it should be part of the state machine and not a side-effect. At this point it doesn't show me where or how these things change state of the machine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is part of the state machine. It's spawning it as an actor, which means it will continue to run when it changes state, and it dispatches events to the top level machine when mutations are received