feat(gatsby,gatsby-cli): Slice component validation (#36801)

gatsbyjs · Oct 13, 2022 · 1fd450f · 1fd450f
1 parent 56badff
commit 1fd450f
Show file tree

Hide file tree

Showing 11 changed files with 480 additions and 161 deletions.
diff --git a/packages/gatsby-cli/src/structured-errors/error-map.ts b/packages/gatsby-cli/src/structured-errors/error-map.ts
@@ -430,7 +430,7 @@ const errors = {
       `${
         context.pluginName
       } created a page and didn't pass the path to the component.\n\nThe page object passed to createPage:\n${JSON.stringify(
-        context.pageObject,
+        context.input,
         null,
         4
       )}`,
@@ -461,42 +461,46 @@ const errors = {
       `${
         context.pluginName
       } created a page with a component that doesn't exist.\n\nThe path to the missing component is "${
-        context.component
+        context.componentPath
       }"\n\nThe page object passed to createPage:\n${JSON.stringify(
-        context.pageObject,
+        context.input,
         null,
         4
-      )}\n\nSee the documentation for the "createPage" action — https://www.gatsbyjs.com/docs/reference/config-files/actions#createPage`,
+      )}`,
     level: Level.ERROR,
     category: ErrorCategory.USER,
+    docsUrl: `https://www.gatsbyjs.com/docs/reference/config-files/actions#createPage`,
   },
   "11326": {
     text: (context): string =>
       `${
         context.pluginName
-      } must set the absolute path to the page component when create creating a page.\n\nThe (relative) path you used for the component is "${
-        context.component
+      } must set the absolute path to the page component when creating a page.\n\nThe (relative) path you used for the component is "${
+        context.componentPath
       }"\n\nYou can convert a relative path to an absolute path by requiring the path module and calling path.resolve() e.g.\n\nconst path = require("path")\npath.resolve("${
-        context.component
+        context.componentPath
       }")\n\nThe page object passed to createPage:\n${JSON.stringify(
-        context.pageObject,
+        context.input,
         null,
         4
-      )}\n\nSee the documentation for the "createPage" action — https://www.gatsbyjs.com/docs/reference/config-files/actions#createPage`,
+      )}`,
     level: Level.ERROR,
     category: ErrorCategory.USER,
+    docsUrl: `https://www.gatsbyjs.com/docs/reference/config-files/actions#createPage`,
   },
   "11327": {
     text: (context): string =>
-      `You have an empty file in the "src/pages" directory at "${context.relativePath}". Please remove it or make it a valid component`,
+      `An empty file "${context.componentPath}" was found during page creation. Please remove it or make it a valid component.`,
     level: Level.ERROR,
     category: ErrorCategory.USER,
+    docsUrl: `https://www.gatsbyjs.com/docs/reference/config-files/actions#createPage`,
   },
   "11328": {
     text: (context): string =>
-      `A page component must export a React component for it to be valid. Please make sure this file exports a React component:\n\n${context.fileName}`,
+      `${context.pluginName} created a page without a valid default export.\n\nThe path to the page is "${context.componentPath}". If your page is a named export, please use "export default" instead.`,
     level: Level.ERROR,
     category: ErrorCategory.USER,
+    docsUrl: `https://www.gatsbyjs.com/docs/reference/config-files/actions#createPage`,
   },
   // invalid or deprecated APIs
   "11329": {
@@ -573,7 +577,7 @@ const errors = {
       `${
         context.pluginName
       } created a slice and didn't pass the path to the component.\n\nThe slice object passed to createSlice:\n${JSON.stringify(
-        context.sliceObject,
+        context.input,
         null,
         4
       )}`,
@@ -596,7 +600,56 @@ const errors = {
     // TODO: change domain to gatsbyjs.com when it's released
     docsUrl: `https://v5.gatsbyjs.com/docs/reference/config-files/actions#createSlice`,
   },
-
+  "11335": {
+    text: (context): string =>
+      `${
+        context.pluginName
+      } must set the absolute path to the slice component when creating a slice.\n\nThe (relative) path you used for the component is "${
+        context.componentPath
+      }"\n\nYou can convert a relative path to an absolute path by requiring the path module and calling path.resolve() e.g.\n\nconst path = require("path")\npath.resolve("${
+        context.componentPath
+      }")\n\nThe object passed to createSlice:\n${JSON.stringify(
+        context.input,
+        null,
+        4
+      )}`,
+    level: Level.ERROR,
+    category: ErrorCategory.USER,
+    // TODO: change domain to gatsbyjs.com when it's released
+    docsUrl: `https://v5.gatsbyjs.com/docs/reference/config-files/actions#createSlice`,
+  },
+  "11336": {
+    text: (context): string =>
+      `${
+        context.pluginName
+      } created a slice with a component that doesn't exist.\n\nThe path to the missing component is "${
+        context.componentPath
+      }"\n\nThe slice object passed to createSlice:\n${JSON.stringify(
+        context.input,
+        null,
+        4
+      )}`,
+    level: Level.ERROR,
+    category: ErrorCategory.USER,
+    // TODO: change domain to gatsbyjs.com when it's released
+    docsUrl: `https://v5.gatsbyjs.com/docs/reference/config-files/actions#createSlice`,
+  },
+  "11337": {
+    text: (context): string =>
+      `An empty file "${context.componentPath}" was found during slice creation. Please remove it or make it a valid component.`,
+    level: Level.ERROR,
+    category: ErrorCategory.USER,
+    // TODO: change domain to gatsbyjs.com when it's released
+    docsUrl: `https://v5.gatsbyjs.com/docs/reference/config-files/actions#createSlice`,
+  },
+  "11338": {
+    text: (context): string =>
+      `${context.pluginName} created a slice component without a valid default export.\n\nThe path to the component is "${context.componentPath}". If your component is a named export, please use "export default" instead.`,
+    level: Level.ERROR,
+    category: ErrorCategory.USER,
+    // TODO: change domain to gatsbyjs.com when it's released
+    docsUrl: `https://v5.gatsbyjs.com/docs/reference/config-files/actions#createSlice`,
+  },
   // node object didn't pass validation
   "11467": {
     text: (context): string =>

diff --git a/packages/gatsby/src/redux/__tests__/__snapshots__/pages.ts.snap b/packages/gatsby/src/redux/__tests__/__snapshots__/pages.ts.snap
@@ -4,7 +4,7 @@ exports[`Add pages Fails if component path is missing 1`] = `"A component must b
 
 exports[`Add pages Fails if path is missing 1`] = `"The plugin \\"test\\" must set the page path when creating a page"`;
 
-exports[`Add pages Fails if the component path isn't absolute 1`] = `"The plugin \\"test\\" must set the absolute path to the page component when create creating a page"`;
+exports[`Add pages Fails if the component path isn't absolute 1`] = `"The plugin \\"test\\" must set the absolute path to the page component when creating a page"`;
 
 exports[`Add pages Fails if use a reserved field in the context object 1`] = `
 "The plugin \\"test\\" used reserved field names in the context object when creating a page:

diff --git a/packages/gatsby/src/redux/actions/public.js b/packages/gatsby/src/redux/actions/public.js
@@ -17,7 +17,7 @@ const { hasNodeChanged } = require(`../../utils/nodes`)
 const { getNode, getDataStore } = require(`../../datastore`)
 import sanitizeNode from "../../utils/sanitize-node"
 const { store } = require(`../index`)
-const { validatePageComponent } = require(`../../utils/validate-page-component`)
+const { validateComponent } = require(`../../utils/validate-component`)
 import { nodeSchema } from "../../joi-schemas/joi"
 const { generateComponentChunkName } = require(`../../utils/js-chunk-names`)
 const {
@@ -91,13 +91,13 @@ type JobV2 = {
   args: Object,
 }
 
-type PageInput = {
-  path: string,
-  component: string,
-  context?: Object,
-  ownerNodeId?: string,
-  defer?: boolean,
-  slices: Record<string, string>,
+export interface IPageInput {
+  path: string;
+  component: string;
+  context?: Object;
+  ownerNodeId?: string;
+  defer?: boolean;
+  slices: Record<string, string>;
 }
 
 type PageMode = "SSG" | "DSG" | "SSR"
@@ -138,7 +138,7 @@ type PageDataRemove = {
  * @example
  * deletePage(page)
  */
-actions.deletePage = (page: PageInput) => {
+actions.deletePage = (page: IPageInput) => {
   return {
     type: `DELETE_PAGE`,
     payload: page,
@@ -183,7 +183,7 @@ const reservedFields = [
  * })
  */
 actions.createPage = (
-  page: PageInput,
+  page: IPageInput,
   plugin?: Plugin,
   actionOptions?: ActionOptions
 ) => {
@@ -268,8 +268,8 @@ ${reservedFields.map(f => `  * "${f}"`).join(`\n`)}
       report.panic({
         id: `11322`,
         context: {
+          input: page,
           pluginName: name,
-          pageObject: page,
         },
       })
     } else {
@@ -283,13 +283,21 @@ ${reservedFields.map(f => `  * "${f}"`).join(`\n`)}
     page.component = pageComponentPath
   }
 
-  const { trailingSlash } = store.getState().config
-  const rootPath = store.getState().program.directory
-  const { error, message, panicOnBuild } = validatePageComponent(
-    page,
-    rootPath,
-    name
-  )
+  const { config, program } = store.getState()
+  const { trailingSlash } = config
+  const { directory } = program
+
+  const { error, panicOnBuild } = validateComponent({
+    input: page,
+    pluginName: name,
+    errorIdMap: {
+      noPath: `11322`,
+      notAbsolute: `11326`,
+      doesNotExist: `11325`,
+      empty: `11327`,
+      noDefaultExport: `11328`,
+    },
+  })
 
   if (error) {
     if (isNotTestEnv) {
@@ -299,7 +307,7 @@ ${reservedFields.map(f => `  * "${f}"`).join(`\n`)}
         report.panic(error)
       }
     }
-    return message
+    return `${name} must set the absolute path to the page component when creating a page`
   }
 
   // check if we've processed this component path
@@ -326,7 +334,7 @@ ${reservedFields.map(f => `  * "${f}"`).join(`\n`)}
         trueComponentPath = slash(trueCasePathSync(page.component))
       } catch (e) {
         // systems where user doesn't have access to /
-        const commonDir = getCommonDir(rootPath, page.component)
+        const commonDir = getCommonDir(directory, page.component)
 
         // using `path.win32` to force case insensitive relative path
         const relativePath = slash(

diff --git a/packages/gatsby/src/redux/actions/restricted.ts b/packages/gatsby/src/redux/actions/restricted.ts
@@ -27,6 +27,7 @@ import { generateComponentChunkName } from "../../utils/js-chunk-names"
 import { store } from "../index"
 import normalizePath from "normalize-path"
 import { trackFeatureIsUsed } from "gatsby-telemetry"
+import { validateComponent } from "../../utils/validate-component"
 
 type RestrictionActionNames =
   | "createFieldExtension"
@@ -40,6 +41,12 @@ type SomeActionCreator =
   | ActionCreator<ActionsUnion>
   | ActionCreator<ThunkAction<any, IGatsbyState, any, ActionsUnion>>
 
+export interface ICreateSliceInput {
+  id: string
+  component: string
+  context: Record<string, unknown>
+}
+
 export const actions = {
   /**
    * Add a third-party schema to be merged into main schema. Schema has to be a
@@ -430,11 +437,7 @@ export const actions = {
     },
 
   createSlice: (
-    payload: {
-      id: string
-      component: string
-      context: Record<string, unknown>
-    },
+    payload: ICreateSliceInput,
     plugin: IGatsbyPlugin,
     traceId?: string
   ): ICreateSliceAction => {
@@ -455,20 +458,34 @@ export const actions = {
           },
         })
       }
-      if (!payload.component) {
-        report.panic({
-          id: `11333`,
-          context: {
-            pluginName: name,
-            sliceObject: payload,
-          },
-        })
+
+      const { slices } = store.getState()
+
+      const { error, panicOnBuild } = validateComponent({
+        input: payload,
+        pluginName: name,
+        errorIdMap: {
+          noPath: `11333`,
+          notAbsolute: `11335`,
+          doesNotExist: `11336`,
+          empty: `11337`,
+          noDefaultExport: `11338`,
+        },
+      })
+
+      if (error && process.env.NODE_ENV !== `test`) {
+        if (panicOnBuild) {
+          report.panicOnBuild(error)
+        } else {
+          report.panic(error)
+        }
       }
 
       trackFeatureIsUsed(`SliceAPI`)
+
       const componentPath = normalizePath(payload.component)
 
-      const oldSlice = store.getState().slices.get(payload.id)
+      const oldSlice = slices.get(payload.id)
       const contextModified =
         !!oldSlice && !isEqual(oldSlice.context, payload.context)
       const componentModified =

diff --git a/packages/gatsby/src/utils/__tests__/fixtures/empty.js b/packages/gatsby/src/utils/__tests__/fixtures/empty.js
diff --git a/packages/gatsby/src/utils/__tests__/fixtures/has-default-export-2.js b/packages/gatsby/src/utils/__tests__/fixtures/has-default-export-2.js
@@ -0,0 +1,5 @@
+import * as React from "react"
+
+export default function HasDefaultExport() {
+  return <div>Has default export</div>
+}
diff --git a/packages/gatsby/src/utils/__tests__/fixtures/has-default-export.js b/packages/gatsby/src/utils/__tests__/fixtures/has-default-export.js
@@ -0,0 +1,5 @@
+import * as React from "react"
+
+export default function HasDefaultExport() {
+  return <div>Has default export</div>
+}
diff --git a/packages/gatsby/src/utils/__tests__/fixtures/no-default-export.js b/packages/gatsby/src/utils/__tests__/fixtures/no-default-export.js
@@ -0,0 +1,5 @@
+import * as React from "react"
+
+export function NoDefaultExport() {
+  return <div>No default export</div>
+}