feat: inject source location data to Slice usage and show errors in p…

…laces where user renders them and not in gatsby internals (#36849)

* feat: inject source location data to Slice usage and show errors in places where user renders them and not in gatsby internals

* update snapshot with added line and column numbers when alias/allowEmpty can't be evaluated

* Update packages/gatsby-cli/src/reporter/errors.ts

Co-authored-by: Ty Hopp <tyhopp@users.noreply.github.com>

* update jsdoc description of new babel plugin to match what it's doing instead of description that was copy&paste from StaticImage plugin

* Update packages/gatsby/cache-dir/slice.js

* Normalize renderedByLocation casing

Co-authored-by: Lennart <lekoarts@gmail.com>

Co-authored-by: Ty Hopp <tyhopp@users.noreply.github.com>
Co-authored-by: Lennart <lekoarts@gmail.com>

packages/gatsby-cli/src/reporter/errors.ts

@@ -3,6 +3,8 @@ import stackTrace from "stack-trace"
 import { prepareStackTrace, ErrorWithCodeFrame } from "./prepare-stack-trace"
 import { isNodeInternalModulePath } from "gatsby-core-utils"
 import { IStructuredStackFrame } from "../structured-errors/types"
+import { readFileSync } from "fs-extra"
+import { codeFrameColumns } from "@babel/code-frame"
 
 const packagesToSkip = [`core-js`, `bluebird`, `regenerator-runtime`, `graphql`]
 
@@ -99,31 +101,91 @@ export function getErrorFormatter(): PrettyError {
   return prettyError
 }
 
+type ErrorWithPotentialForcedLocation = Error & {
+  forcedLocation?: {
+    fileName: string
+    lineNumber?: number
+    columnNumber?: number
+    endLineNumber?: number
+    endColumnNumber?: number
+    functionName?: string
+  }
+}
+
 /**
  * Convert a stringified webpack compilation error back into
  * an Error instance so it can be formatted properly
  */
 export function createErrorFromString(
-  errorStr: string = ``,
+  errorOrErrorStack: string | ErrorWithPotentialForcedLocation = ``,
   sourceMapFile: string
 ): ErrorWithCodeFrame {
-  let [message, ...rest] = errorStr.split(/\r\n|[\n\r]/g)
-  // pull the message from the first line then remove the `Error:` prefix
-  // FIXME: when https://github.com/AriaMinaei/pretty-error/pull/49 is merged
+  if (typeof errorOrErrorStack === `string`) {
+    const errorStr = errorOrErrorStack
+    let [message, ...rest] = errorStr.split(/\r\n|[\n\r]/g)
+    // pull the message from the first line then remove the `Error:` prefix
+    // FIXME: when https://github.com/AriaMinaei/pretty-error/pull/49 is merged
 
-  message = message.replace(/^(Error:)/, ``)
+    message = message.replace(/^(Error:)/, ``)
 
-  const error = new Error(message)
+    const error = new Error(message)
 
-  error.stack = [message, rest.join(`\n`)].join(`\n`)
+    error.stack = [message, rest.join(`\n`)].join(`\n`)
 
-  error.name = `WebpackError`
-  try {
-    if (sourceMapFile) {
-      return prepareStackTrace(error, sourceMapFile)
+    error.name = `WebpackError`
+    try {
+      if (sourceMapFile) {
+        return prepareStackTrace(error, sourceMapFile)
+      }
+    } catch (err) {
+      // don't shadow a real error because of a parsing issue
+    }
+    return error
+  } else {
+    if (errorOrErrorStack.forcedLocation) {
+      const forcedLocation = errorOrErrorStack.forcedLocation
+      const error = new Error(errorOrErrorStack.message) as ErrorWithCodeFrame
+      error.stack = `${errorOrErrorStack.message}
+  at ${forcedLocation.functionName ?? `<anonymous>`} (${
+        forcedLocation.fileName
+      }${
+        forcedLocation.lineNumber
+          ? `:${forcedLocation.lineNumber}${
+              forcedLocation.columnNumber
+                ? `:${forcedLocation.columnNumber}`
+                : ``
+            }`
+          : ``
+      })`
+
+      try {
+        const source = readFileSync(forcedLocation.fileName, `utf8`)
+
+        error.codeFrame = codeFrameColumns(
+          source,
+          {
+            start: {
+              line: forcedLocation.lineNumber ?? 0,
+              column: forcedLocation.columnNumber ?? 0,
+            },
+            end: forcedLocation.endColumnNumber
+              ? {
+                  line: forcedLocation.endLineNumber ?? 0,
+                  column: forcedLocation.endColumnNumber ?? 0,
+                }
+              : undefined,
+          },
+          {
+            highlightCode: true,
+          }
+        )
+      } catch (e) {
+        // failed to generate codeframe, we still should show an error so we keep going
+      }
+
+      return error
+    } else {
+      return createErrorFromString(errorOrErrorStack.stack, sourceMapFile)
     }
-  } catch (err) {
-    // don't shadow a real error because of a parsing issue
   }
-  return error
 }

packages/gatsby/cache-dir/fast-refresh-overlay/components/hooks.js

@@ -10,15 +10,34 @@ const initialResponse = {
   sourceContent: null,
 }
 
-export function useStackFrame({ moduleId, lineNumber, columnNumber }) {
-  const url =
+export function useStackFrame({
+  moduleId,
+  lineNumber,
+  columnNumber,
+  skipSourceMap,
+  endLineNumber,
+  endColumnNumber,
+}) {
+  let url =
     `/__original-stack-frame?moduleId=` +
     window.encodeURIComponent(moduleId) +
     `&lineNumber=` +
     window.encodeURIComponent(lineNumber) +
     `&columnNumber=` +
     window.encodeURIComponent(columnNumber)
 
+  if (skipSourceMap) {
+    url += `&skipSourceMap=true`
+  }
+
+  if (endLineNumber) {
+    url += `&endLineNumber=` + window.encodeURIComponent(endLineNumber)
+
+    if (endColumnNumber) {
+      url += `&endColumnNumber=` + window.encodeURIComponent(endColumnNumber)
+    }
+  }
+
   const [response, setResponse] = React.useState(initialResponse)
 
   React.useEffect(() => {

packages/gatsby/cache-dir/fast-refresh-overlay/components/runtime-errors.js

@@ -3,21 +3,35 @@ import ErrorStackParser from "error-stack-parser"
 import { Overlay, Header, HeaderOpenClose, Body } from "./overlay"
 import { useStackFrame } from "./hooks"
 import { CodeFrame } from "./code-frame"
-import { getCodeFrameInformation, openInEditor } from "../utils"
+import { getCodeFrameInformationFromStackTrace, openInEditor } from "../utils"
 import { Accordion, AccordionItem } from "./accordion"
 
-function WrappedAccordionItem({ error, open }) {
+function getCodeFrameInformationFromError(error) {
+  if (error.forcedLocation) {
+    return {
+      skipSourceMap: true,
+      moduleId: error.forcedLocation.fileName,
+      functionName: error.forcedLocation.functionName,
+      lineNumber: error.forcedLocation.lineNumber,
+      columnNumber: error.forcedLocation.columnNumber,
+      endLineNumber: error.forcedLocation.endLineNumber,
+      endColumnNumber: error.forcedLocation.endColumnNumber,
+    }
+  }
+
   const stacktrace = ErrorStackParser.parse(error)
-  const codeFrameInformation = getCodeFrameInformation(stacktrace)
+  return getCodeFrameInformationFromStackTrace(stacktrace)
+}
+
+function WrappedAccordionItem({ error, open }) {
+  const codeFrameInformation = getCodeFrameInformationFromError(error)
 
   const modulePath = codeFrameInformation?.moduleId
-  const lineNumber = codeFrameInformation?.lineNumber
-  const columnNumber = codeFrameInformation?.columnNumber
   const name = codeFrameInformation?.functionName
   // With the introduction of Metadata management the modulePath can have a resourceQuery that needs to be removed first
   const filePath = modulePath.replace(/(\?|&)export=(default|head)$/, ``)
 
-  const res = useStackFrame({ moduleId: modulePath, lineNumber, columnNumber })
+  const res = useStackFrame(codeFrameInformation)
   const line = res.sourcePosition?.line
 
   const Title = () => {

packages/gatsby/cache-dir/fast-refresh-overlay/utils.js

@@ -35,7 +35,7 @@ export function skipSSR() {
   }
 }
 
-export function getCodeFrameInformation(stackTrace) {
+export function getCodeFrameInformationFromStackTrace(stackTrace) {
   const stackFrame = stackTrace.find(stackFrame => {
     const fileName = stackFrame.getFileName()
     return fileName && fileName !== `[native code]` // Quirk of Safari error stack frames

packages/gatsby/cache-dir/slice.js

@@ -20,7 +20,8 @@ export function Slice(props) {
       throw new SlicePropsError(
         slicesContext.renderEnvironment === `browser`,
         internalProps.sliceName,
-        propErrors
+        propErrors,
+        props.__renderedByLocation
       )
     }
 
@@ -61,9 +62,12 @@ export function Slice(props) {
 }
 
 class SlicePropsError extends Error {
-  constructor(inBrowser, sliceName, propErrors) {
+  constructor(inBrowser, sliceName, propErrors, renderedByLocation) {
     const errors = Object.entries(propErrors)
-      .map(([key, value]) => `${key}: "${value}"`)
+      .map(
+        ([key, value]) =>
+          `not serializable "${value}" type passed to "${key}" prop`
+      )
       .join(`, `)
 
     const name = `SlicePropsError`
@@ -81,22 +85,25 @@ class SlicePropsError extends Error {
       stackLines[0] = stackLines[0].trim()
       stack = `\n` + stackLines.join(`\n`)
 
-      // look for any hints for the component name in the stack trace
-      const componentRe = /^at\s+([a-zA-Z0-9]+)/
-      const componentMatch = stackLines[0].match(componentRe)
-      const componentHint = componentMatch ? `in ${componentMatch[1]} ` : ``
-
-      message = `Slice "${sliceName}" was passed props ${componentHint}that are not serializable (${errors}).`
+      message = `Slice "${sliceName}" was passed props that are not serializable (${errors}).`
     } else {
       // we can't really grab any extra info outside of the browser, so just print what we can
-      message = `${name}: Slice "${sliceName}" was passed props that are not serializable (${errors}). Use \`gatsby develop\` to see more information.`
+      message = `${name}: Slice "${sliceName}" was passed props that are not serializable (${errors}).`
       const stackLines = new Error().stack.trim().split(`\n`).slice(2)
       stack = `${message}\n${stackLines.join(`\n`)}`
     }
 
     super(message)
     this.name = name
-    this.stack = stack
+    if (stack) {
+      this.stack = stack
+    } else {
+      Error.captureStackTrace(this, SlicePropsError)
+    }
+
+    if (renderedByLocation) {
+      this.forcedLocation = { ...renderedByLocation, functionName: `Slice` }
+    }
   }
 }
 

packages/gatsby/src/commands/build-html.ts

@@ -556,10 +556,7 @@ export const doBuildPages = async (
   try {
     await renderHTMLQueue(workerPool, activity, rendererPath, pagePaths, stage)
   } catch (error) {
-    const prettyError = createErrorFromString(
-      error.stack,
-      `${rendererPath}.map`
-    )
+    const prettyError = createErrorFromString(error, `${rendererPath}.map`)
 
     const buildError = new BuildHTMLError(prettyError)
     buildError.context = error.context

packages/gatsby/src/query/__tests__/__snapshots__/file-parser.js.snap

@@ -2584,7 +2584,7 @@ Object {
   "pageSlices": null,
   "warnCalls": Array [
     Array [
-      "[Gatsby Slice API] Could not find values in \\"slice-function.js\\" for the following props at build time: alias",
+      "[Gatsby Slice API] Could not find values in \\"slice-function.js:5:10\\" for the following props at build time: alias",
     ],
   ],
 }

packages/gatsby/src/utils/babel-loader-helpers.ts

@@ -115,6 +115,21 @@ export const prepareOptions = (
     )
   }
 
+  if (
+    stage === `develop` ||
+    stage === `build-html` ||
+    stage === `develop-html`
+  ) {
+    requiredPlugins.push(
+      babel.createConfigItem(
+        [resolve(`./babel/babel-plugin-add-slice-placeholder-location`)],
+        {
+          type: `plugin`,
+        }
+      )
+    )
+  }
+
   const requiredPresets: Array<PluginItem> = []
 
   if (stage === `develop`) {

packages/gatsby/src/utils/babel/babel-plugin-add-slice-placeholder-location.ts

@@ -0,0 +1,90 @@
+import { relative } from "path"
+import type { PluginObj, types as BabelTypes, PluginPass } from "@babel/core"
+import { ObjectProperty } from "@babel/types"
+import { store } from "../../redux"
+
+/**
+ * This is a plugin that finds <Slice> placeholder components and injects the __renderedByLocation prop
+ * with filename and location in the file where the placeholder was found. This is later used to provide
+ * more useful error messages when the user props are invalid showing codeframe where user tries to render it
+ * instead of codeframe of the Slice component itself (internals of gatsby) that is not useful for the user.
+ */
+
+export default function addSlicePlaceholderLocation(
+  this: PluginPass,
+  {
+    types: t,
+  }: {
+    types: typeof BabelTypes
+  }
+): PluginObj {
+  return {
+    name: `babel-plugin-add-slice-placeholder-location`,
+    visitor: {
+      JSXOpeningElement(nodePath): void {
+        if (!nodePath.get(`name`).referencesImport(`gatsby`, `Slice`)) {
+          return
+        }
+
+        if (this.file.opts.filename) {
+          const __renderedByLocationProperties: Array<ObjectProperty> = [
+            t.objectProperty(
+              t.identifier(`fileName`),
+              t.stringLiteral(
+                relative(
+                  store.getState().program.directory,
+                  this.file.opts.filename
+                )
+              )
+            ),
+          ]
+
+          if (nodePath.node.loc?.start.line) {
+            __renderedByLocationProperties.push(
+              t.objectProperty(
+                t.identifier(`lineNumber`),
+                t.numericLiteral(nodePath.node.loc.start.line)
+              )
+            )
+
+            if (nodePath.node.loc?.start.column) {
+              __renderedByLocationProperties.push(
+                t.objectProperty(
+                  t.identifier(`columnNumber`),
+                  t.numericLiteral(nodePath.node.loc.start.column + 1)
+                )
+              )
+            }
+
+            if (nodePath.node.loc?.end.line) {
+              __renderedByLocationProperties.push(
+                t.objectProperty(
+                  t.identifier(`endLineNumber`),
+                  t.numericLiteral(nodePath.node.loc.end.line)
+                )
+              )
+
+              if (nodePath.node.loc?.end.column) {
+                __renderedByLocationProperties.push(
+                  t.objectProperty(
+                    t.identifier(`endColumnNumber`),
+                    t.numericLiteral(nodePath.node.loc.end.column + 1)
+                  )
+                )
+              }
+            }
+          }
+
+          const newProp = t.jsxAttribute(
+            t.jsxIdentifier(`__renderedByLocation`),
+            t.jsxExpressionContainer(
+              t.objectExpression(__renderedByLocationProperties)
+            )
+          )
+
+          nodePath.node.attributes.push(newProp)
+        }
+      },
+    },
+  }
+}