feat(gatsby): Move page component state & side effect handling to xst…

…ate (#11897) * Inital work to move component query handling to xstate + fix bug with major relay-compilier update * Expand model to include we don't want to immediately extract queries while bootstrapping + initiate extracting queries from machine when new page component is created * Rearrange states so more in order of possible events * Document two program statuses * Move responsibility for running queries for pages to the page component machine * Handle page components changing * Let state machine know when a query has been run and only re-run queries if the page query changes * Handle babel error state when extracing queries * Make name more explicit * Ignore static query results * Clarify comment * empty strings are falsy 🤦‍♂️ * Handled in last commit * Only handle clean ids for static queries (for now until model static queries in a machine as well) * In hacky/temp way (until modeling pages directly), check if a new page has been created and run its query * Mark new actions as private * Move action to bootstrap per @stefanprobst's suggestion * Extract page component machine into new 'machines' directory * Fix check that there's a query in a non-page component * Add xstate as a dependency * Update snapshots * Add initial tests for page component machine * Listen for query queue to empty to know queries are done * Comment out machine logging * Restore usage of getCodeFromRelayError * Don't need to re-set service on Map * Small fixes * Update packages/gatsby/src/redux/reducers/components.js Co-Authored-By: KyleAMathews <mathews.kyle@gmail.com> * Don't run queries until hit query running stage in bootstrap * Switch page components out of bootstrap state right after bootstrap query running is finished so if a stateful page changes, new queries are still run * During bootstrap, only run page queries that are 'clean' (i.e. we haven't seen them before) or 'dirty' (i.e. their tracked nodes changed since the last run) * Handle deleting pages in machine so it knows to re-run its query if re-added * Move simple set context actions to top-level so don't have to repeat across each state (thanks @pieh!) * Simplify logic for running queries for new pages We only want to run page queries in two circumstances: - the query has changed - the data has changed This is true whether in bootstrap or not. This commit simplifies the reducer / machine logic to not distinguish between whether in bootstrap or not. Also fixes the bug @pieh found where we we're re-running queries in bootstrap if the query had changed. * run queries for pages created after bootstrap * Capture more types of graphql extraction errors * DELETE_PAGE action use `component` not `componentPath` * Page components shouldn't control extraction — moving that to them meant changes to static queries weren't being run * Now that we're not tracking query extracting, add event for when babel successfully extracts queries so we can know to leave babe error state * Move graphql/babel event handling to top-level as we should always respond the same way * Move action call to after findGraphQLTags as we cache babel extraction * trivial change * yarn.lock change * Trivial change so can publish * Update from master
gatsbyjs · Mar 26, 2019 · 91086d4 · 91086d4
1 parent 090be3e
commit 91086d4
Show file tree

Hide file tree

Showing 17 changed files with 626 additions and 114 deletions.
diff --git a/packages/gatsby/cache-dir/static-entry.js b/packages/gatsby/cache-dir/static-entry.js
@@ -126,13 +126,13 @@ export default (pagePath, callback) => {
 
   let dataAndContext = {}
   if (page.jsonName in dataPaths) {
-    const pathToJsonData = `../public/` + dataPaths[page.jsonName]
+    const pathToJsonData = join(
+      process.cwd(),
+      `/public/static/d`,
+      `${dataPaths[page.jsonName]}.json`
+    )
     try {
-      dataAndContext = JSON.parse(
-        fs.readFileSync(
-          `${process.cwd()}/public/static/d/${dataPaths[page.jsonName]}.json`
-        )
-      )
+      dataAndContext = JSON.parse(fs.readFileSync(pathToJsonData))
     } catch (e) {
       console.log(`error`, pathToJsonData, e)
       process.exit()

diff --git a/packages/gatsby/package.json b/packages/gatsby/package.json
@@ -131,6 +131,7 @@
     "webpack-hot-middleware": "^2.21.0",
     "webpack-merge": "^4.1.0",
     "webpack-stats-plugin": "^0.1.5",
+    "xstate": "^4.3.2",
     "yaml-loader": "^0.5.0"
   },
   "devDependencies": {

diff --git a/packages/gatsby/src/bootstrap/index.js b/packages/gatsby/src/bootstrap/index.js
@@ -468,9 +468,17 @@ module.exports = async (args: BootstrapArgs) => {
       ).toFixed(2)} queries/second`
     )
   })
-  await runInitialQueries(activity)
+  // HACKY!!! TODO: REMOVE IN NEXT REFACTOR
+  emitter.emit(`START_QUERY_QUEUE`)
+  // END HACKY
+  runInitialQueries(activity)
+  await new Promise(resolve => queryQueue.on(`drain`, resolve))
   activity.end()
 
+  require(`../redux/actions`).boundActionCreators.setProgramStatus(
+    `BOOTSTRAP_QUERY_RUNNING_FINISHED`
+  )
+
   // Write out files.
   activity = report.activityTimer(`write out page data`, {
     parentSpan: bootstrapSpan,
@@ -528,6 +536,9 @@ const finishBootstrap = async bootstrapSpan => {
   report.info(`bootstrap finished - ${process.uptime()} s`)
   report.log(``)
   emitter.emit(`BOOTSTRAP_FINISHED`)
+  require(`../redux/actions`).boundActionCreators.setProgramStatus(
+    `BOOTSTRAP_FINISHED`
+  )
 
   bootstrapSpan.finish()
 }
diff --git a/packages/gatsby/src/bootstrap/load-plugins/__tests__/__snapshots__/load-plugins.js.snap b/packages/gatsby/src/bootstrap/load-plugins/__tests__/__snapshots__/load-plugins.js.snap
@@ -63,9 +63,7 @@ Array [
     "browserAPIs": Array [],
     "id": "84dad27f-1d44-51fc-ac56-4db2e5222995",
     "name": "query-runner",
-    "nodeAPIs": Array [
-      "onCreatePage",
-    ],
+    "nodeAPIs": Array [],
     "pluginOptions": Object {
       "plugins": Array [],
     },
@@ -181,9 +179,7 @@ Array [
     "browserAPIs": Array [],
     "id": "84dad27f-1d44-51fc-ac56-4db2e5222995",
     "name": "query-runner",
-    "nodeAPIs": Array [
-      "onCreatePage",
-    ],
+    "nodeAPIs": Array [],
     "pluginOptions": Object {
       "plugins": Array [],
     },

diff --git a/packages/gatsby/src/internal-plugins/query-runner/file-parser.js b/packages/gatsby/src/internal-plugins/query-runner/file-parser.js
@@ -13,6 +13,7 @@ import type { DocumentNode, DefinitionNode } from "graphql"
 import { babelParseToAst } from "../../utils/babel-parse-to-ast"
 
 const apiRunnerNode = require(`../../utils/api-runner-node`)
+const { boundActionCreators } = require(`../../redux/actions`)
 
 /**
  * Add autogenerated query name if it wasn't defined by user.
@@ -53,22 +54,36 @@ async function parseToAst(filePath, fileStr) {
         break
       } catch (error) {
         report.error(error)
+        boundActionCreators.queryExtractionGraphQLError({
+          componentPath: filePath,
+        })
         continue
       }
     }
     if (ast === undefined) {
       report.error(`Failed to parse preprocessed file ${filePath}`)
+      boundActionCreators.queryExtractionGraphQLError({
+        componentPath: filePath,
+      })
+
+      return null
     }
   } else {
     try {
       ast = babelParseToAst(fileStr, filePath)
     } catch (error) {
+      boundActionCreators.queryExtractionBabelError({
+        componentPath: filePath,
+        error,
+      })
       report.error(
         `There was a problem parsing "${filePath}"; any GraphQL ` +
           `fragments or queries in this file were not processed. \n` +
           `This may indicate a syntax error in the code, or it may be a file type ` +
           `that Gatsby does not know how to parse.`
       )
+
+      return null
     }
   }
 
@@ -289,6 +304,9 @@ export default class FileParser {
       text = await fs.readFile(file, `utf8`)
     } catch (err) {
       report.error(`There was a problem reading the file: ${file}`, err)
+      boundActionCreators.queryExtractionGraphQLError({
+        componentPath: file,
+      })
       return null
     }
 
@@ -303,6 +321,15 @@ export default class FileParser {
       let astDefinitions =
         cache[hash] || (cache[hash] = await findGraphQLTags(file, text))
 
+      // If any AST definitions were extracted, report success.
+      // This can mean there is none or there was a babel error when
+      // we tried to extract the graphql AST.
+      if (astDefinitions.length > 0) {
+        boundActionCreators.queryExtractedBabelSuccess({
+          componentPath: file,
+        })
+      }
+
       return astDefinitions.length
         ? {
             kind: `Document`,
@@ -314,6 +341,9 @@ export default class FileParser {
         `There was a problem parsing the GraphQL query in file: ${file}`,
         err
       )
+      boundActionCreators.queryExtractionGraphQLError({
+        componentPath: file,
+      })
       return null
     }
   }

diff --git a/packages/gatsby/src/internal-plugins/query-runner/gatsby-node.js b/packages/gatsby/src/internal-plugins/query-runner/gatsby-node.js
diff --git a/packages/gatsby/src/internal-plugins/query-runner/graphql-errors.js b/packages/gatsby/src/internal-plugins/query-runner/graphql-errors.js
@@ -55,21 +55,22 @@ function formatError(message: string, filePath: string, codeFrame: string) {
 }
 
 function extractError(error: Error): { message: string, docName: string } {
-  const docRegex = /Invariant Violation: (RelayParser|GraphQLParser): (.*). Source: document `(.*)` file:/g
+  const docRegex = /Error:.(RelayParser|GraphQLParser):(.*)Source: document.`(.*)`.file.*(GraphQL.request.*^\s*$)/gms
   let matches
-  let message = ``,
-    docName = ``
+  let message = ``
+  let docName = ``
+  let codeBlock = ``
   while ((matches = docRegex.exec(error.toString())) !== null) {
     // This is necessary to avoid infinite loops with zero-width matches
     if (matches.index === docRegex.lastIndex) docRegex.lastIndex++
-    ;[, , message, docName] = matches
+    ;[, , message, docName, codeBlock] = matches
   }
 
   if (!message) {
     message = error.toString()
   }
 
-  return { message, docName }
+  return { message, codeBlock, docName }
 }
 
 function findLocation(extractedMessage, def) {
@@ -169,15 +170,18 @@ export function graphqlError(
   nameDefMap: Map<string, any>,
   error: Error | RelayGraphQLError
 ) {
+  let codeBlock
   let { message, docName } = extractError(error)
   let filePath = namePathMap.get(docName)
 
   if (filePath && docName) {
-    return formatError(
+    codeBlock = getCodeFrameFromRelayError(
+      nameDefMap.get(docName),
       message,
-      filePath,
-      getCodeFrameFromRelayError(nameDefMap.get(docName), message, error)
+      error
     )
+    const formattedMessage = formatError(message, filePath, codeBlock)
+    return { formattedMessage, docName, message, codeBlock }
   }
 
   let reportedMessage = `There was an error while compiling your site's GraphQL queries.
@@ -194,5 +198,5 @@ export function graphqlError(
     reportedMessage += `${error.message.slice(21)}\n`
   }
 
-  return reportedMessage
+  return { formattedMessage: reportedMessage, docName, message, codeBlock }
 }
diff --git a/packages/gatsby/src/internal-plugins/query-runner/page-query-runner.js b/packages/gatsby/src/internal-plugins/query-runner/page-query-runner.js
@@ -28,13 +28,17 @@ exports.queueQueryForPathname = pathname => {
 // Afterwards we listen "API_RUNNING_QUEUE_EMPTY" and check
 // for dirty nodes before running queries.
 exports.runInitialQueries = async () => {
-  await runQueries()
-
   active = true
+  await runQueries(true)
   return
 }
 
-const runQueries = async () => {
+const runQueries = async (initial = false) => {
+  // Don't run queries until bootstrap gets to "run graphql queries"
+  if (!active) {
+    return
+  }
+
   // Find paths dependent on dirty nodes
   queuedDirtyActions = _.uniq(queuedDirtyActions, a => a.payload.id)
   const dirtyIds = findDirtyIds(queuedDirtyActions)
@@ -45,11 +49,25 @@ const runQueries = async () => {
   const cleanIds = findIdsWithoutDataDependencies()
 
   // Construct paths for all queries to run
-  const pathnamesToRun = _.uniq([
-    ...runQueriesForPathnamesQueue,
-    ...dirtyIds,
-    ...cleanIds,
-  ])
+  let pathnamesToRun = _.uniq([...dirtyIds, ...cleanIds])
+
+  // If this is the initial run, remove pathnames from `runQueriesForPathnamesQueue`
+  // if they're also not in the dirtyIds or cleanIds.
+  //
+  // We do this because the page component reducer/machine always
+  // adds pages to runQueriesForPathnamesQueue but during bootstrap
+  // we may not want to run those page queries if their data hasn't
+  // changed since the last time we ran Gatsby.
+  let diffedPathnames = [...runQueriesForPathnamesQueue]
+  if (initial) {
+    diffedPathnames = _.intersection(
+      [...runQueriesForPathnamesQueue],
+      pathnamesToRun
+    )
+  }
+
+  // Combine.
+  pathnamesToRun = _.union(diffedPathnames, pathnamesToRun)
 
   runQueriesForPathnamesQueue.clear()
 
@@ -58,6 +76,8 @@ const runQueries = async () => {
   return
 }
 
+exports.runQueries = runQueries
+
 emitter.on(`CREATE_NODE`, action => {
   queuedDirtyActions.push(action)
 })
@@ -66,12 +86,6 @@ emitter.on(`DELETE_NODE`, action => {
   queuedDirtyActions.push({ payload: action.payload })
 })
 
-emitter.on(`CREATE_PAGE`, action => {
-  if (action.contextModified) {
-    exports.queueQueryForPathname(action.payload.path)
-  }
-})
-
 const runQueuedActions = async () => {
   if (active && !running) {
     try {

diff --git a/packages/gatsby/src/internal-plugins/query-runner/query-compiler.js b/packages/gatsby/src/internal-plugins/query-runner/query-compiler.js
@@ -12,6 +12,7 @@ import filterContextForNode from "@gatsbyjs/relay-compiler/lib/filterContextForN
 const _ = require(`lodash`)
 
 import { store } from "../../redux"
+const { boundActionCreators } = require(`../../redux/actions`)
 import FileParser from "./file-parser"
 import GraphQLIRPrinter from "@gatsbyjs/relay-compiler/lib/GraphQLIRPrinter"
 import {
@@ -134,13 +135,17 @@ class Runner {
     const compiledNodes: Queries = new Map()
     const namePathMap = new Map()
     const nameDefMap = new Map()
+    const nameErrorMap = new Map()
     const documents = []
 
     for (let [filePath, doc] of nodes.entries()) {
       let errors = validate(this.schema, doc, validationRules)
 
       if (errors && errors.length) {
         this.reportError(graphqlValidationError(errors, filePath))
+        boundActionCreators.queryExtractionGraphQLError({
+          componentPath: filePath,
+        })
         return compiledNodes
       }
 
@@ -163,8 +168,18 @@ class Runner {
         )
       )
     } catch (error) {
-      this.reportError(graphqlError(namePathMap, nameDefMap, error))
-      return compiledNodes
+      const { formattedMessage, docName, message, codeBlock } = graphqlError(
+        namePathMap,
+        nameDefMap,
+        error
+      )
+      nameErrorMap.set(docName, { formattedMessage, message, codeBlock })
+      boundActionCreators.queryExtractionGraphQLError({
+        componentPath: namePathMap.get(docName),
+        error: formattedMessage,
+      })
+      this.reportError(formattedMessage)
+      return false
     }
 
     // relay-compiler v1.5.0 added "StripUnusedVariablesTransform" to
@@ -191,6 +206,9 @@ class Runner {
             otherNode && nameDefMap.get(otherNode.name)
           )
         )
+        boundActionCreators.queryExtractionGraphQLError({
+          componentPath: filePath,
+        })
         return
       }