fix(gatsby): use joi validation result instead of payload (#15379)

* fix(gatsby): use joi validation result instead of payload

* move leading & trailing slash checks to joi

* fix tests

* remove debug info

Author: wardpeet

packages/gatsby/src/joi-schemas/__tests__/joi.js

@@ -1,61 +1,103 @@
 const { gatsbyConfigSchema } = require(`../joi`)
 
 describe(`gatsby config`, () => {
-  it(`strips trailing slashes from url fields`, () => {
+  it(`returns empty pathPrefix when not set`, async () => {
+    const config = {}
+
+    const result = await gatsbyConfigSchema.validate(config)
+    expect(result).toEqual(
+      expect.objectContaining({
+        pathPrefix: ``,
+      })
+    )
+  })
+
+  it(`strips trailing slashes from url fields`, async () => {
     const config = {
       pathPrefix: `/blog///`,
       assetPrefix: `https://cdn.example.com/`,
     }
 
-    expect(gatsbyConfigSchema.validate(config)).resolves.toEqual({
-      pathPrefix: `/blog`,
-      assetPrefix: `https://cdn.example.com`,
-    })
+    const result = await gatsbyConfigSchema.validate(config)
+    expect(result).toEqual(
+      expect.objectContaining({
+        pathPrefix: `/blog`,
+        assetPrefix: `https://cdn.example.com`,
+      })
+    )
   })
 
-  it(`allows assetPrefix to be full URL`, () => {
+  it(`allows assetPrefix to be full URL`, async () => {
     const config = {
-      assetPrefix: `https://cdn.example.com`,
+      assetPrefix: `https://cdn.example.com/`,
     }
 
-    expect(gatsbyConfigSchema.validate(config)).resolves.toEqual(config)
+    const result = await gatsbyConfigSchema.validate(config)
+    expect(result).toEqual(
+      expect.objectContaining({
+        assetPrefix: `https://cdn.example.com`,
+      })
+    )
   })
 
-  it(`allows assetPrefix to be a URL with nested paths`, () => {
+  it(`allows assetPrefix to be a URL with nested paths`, async () => {
     const config = {
       assetPrefix: `https://cdn.example.com/some/nested/path`,
     }
 
-    expect(gatsbyConfigSchema.validate(config)).resolves.toEqual(config)
+    const result = await gatsbyConfigSchema.validate(config)
+    expect(result).toEqual(expect.objectContaining(config))
   })
 
-  it(`allows relative paths for url fields`, () => {
+  it(`allows relative paths for url fields`, async () => {
     const config = {
       pathPrefix: `/blog`,
       assetPrefix: `https://cdn.example.com`,
     }
 
-    expect(gatsbyConfigSchema.validate(config)).resolves.toEqual(config)
+    const result = await gatsbyConfigSchema.validate(config)
+    expect(result).toEqual(expect.objectContaining(config))
   })
 
-  it(`does not allow pathPrefix to be full URL`, () => {
+  it(`strips trailing slash and add leading slash to pathPrefix`, async () => {
+    const config = {
+      pathPrefix: `blog/`,
+      assetPrefix: `https://cdn.example.com/`,
+    }
+
+    const result = await gatsbyConfigSchema.validate(config)
+    expect(result).toEqual(
+      expect.objectContaining({
+        pathPrefix: `/blog`,
+        assetPrefix: `https://cdn.example.com`,
+      })
+    )
+  })
+
+  it(`does not allow pathPrefix to be full URL`, async () => {
+    expect.assertions(1)
     const config = {
       pathPrefix: `https://google.com`,
     }
 
-    expect(
-      gatsbyConfigSchema.validate(config)
-    ).rejects.toThrowErrorMatchingSnapshot()
+    try {
+      await gatsbyConfigSchema.validate(config)
+    } catch (err) {
+      expect(err.message).toMatchSnapshot()
+    }
   })
 
-  it(`throws when relative path used for both assetPrefix and pathPrefix`, () => {
+  it(`throws when relative path used for both assetPrefix and pathPrefix`, async () => {
+    expect.assertions(1)
     const config = {
       assetPrefix: `/assets`,
       pathPrefix: `/blog`,
     }
 
-    expect(
-      gatsbyConfigSchema.validate(config)
-    ).rejects.toThrowErrorMatchingSnapshot()
+    try {
+      await gatsbyConfigSchema.validate(config)
+    } catch (err) {
+      expect(err.message).toMatchSnapshot()
+    }
   })
 })

packages/gatsby/src/joi-schemas/joi.js

@@ -1,21 +1,33 @@
 const Joi = require(`@hapi/joi`)
 
 const stripTrailingSlash = chain => chain.replace(/(\w)\/+$/, `$1`)
+// only add leading slash on relative urls
+const addLeadingSlash = chain =>
+  chain.when(Joi.string().uri({ relativeOnly: true }), {
+    then: chain.replace(/^([^/])/, `/$1`),
+  })
 
 export const gatsbyConfigSchema = Joi.object()
   .keys({
     __experimentalThemes: Joi.array(),
-    polyfill: Joi.boolean(),
+    polyfill: Joi.boolean().default(true),
     assetPrefix: stripTrailingSlash(
       Joi.string().uri({
         allowRelative: true,
       })
     ),
-    pathPrefix: stripTrailingSlash(
-      Joi.string().uri({
-        allowRelative: true,
-        relativeOnly: true,
-      })
+    pathPrefix: addLeadingSlash(
+      stripTrailingSlash(
+        Joi.string()
+          .uri({
+            allowRelative: true,
+            relativeOnly: true,
+          })
+          .default(``)
+          // removes single / value
+          .allow(``)
+          .replace(/^\/$/, ``)
+      )
     ),
     siteMetadata: Joi.object({
       siteUrl: stripTrailingSlash(Joi.string()).uri(),
@@ -35,10 +47,12 @@ export const gatsbyConfigSchema = Joi.object()
         allowRelative: true,
         relativeOnly: true,
       }),
-      pathPrefix: Joi.string().uri({
-        allowRelative: true,
-        relativeOnly: true,
-      }),
+      pathPrefix: Joi.string()
+        .uri({
+          allowRelative: true,
+          relativeOnly: true,
+        })
+        .default(``),
     }),
     {
       then: Joi.object({

packages/gatsby/src/redux/reducers/__tests__/config.js

@@ -1,6 +1,10 @@
 const reducer = require(`../config`)
 
 describe(`config reducer`, () => {
+  beforeEach(() => {
+    jest.resetModules()
+  })
+
   it(`let's you add a config`, () => {
     const action = {
       type: `SET_SITE_CONFIG`,

packages/gatsby/src/redux/reducers/config.js

@@ -1,14 +1,15 @@
-const Joi = require(`@hapi/joi`)
 const chalk = require(`chalk`)
-const _ = require(`lodash`)
 
 const { gatsbyConfigSchema } = require(`../../joi-schemas/joi`)
 
 module.exports = (state = {}, action) => {
   switch (action.type) {
     case `SET_SITE_CONFIG`: {
       // Validate the config.
-      const result = Joi.validate(action.payload, gatsbyConfigSchema)
+      const result = gatsbyConfigSchema.validate(action.payload || {})
+
+      const normalizedPayload = result.value
+
       // TODO use Redux for capturing errors from different
       // parts of Gatsby so a) can capture richer errors and b) be
       // more flexible how to display them.
@@ -17,37 +18,13 @@ module.exports = (state = {}, action) => {
           chalk.blue.bgYellow(`The site's gatsby-config.js failed validation`)
         )
         console.log(chalk.bold.red(result.error))
-        if (action.payload.linkPrefix) {
+        if (normalizedPayload.linkPrefix) {
           console.log(`"linkPrefix" should be changed to "pathPrefix"`)
         }
         throw new Error(`The site's gatsby-config.js failed validation`)
       }
 
-      // Ensure that the pathPrefix (if set) starts with a forward slash
-      // and doesn't end with a slash.
-      if (action.payload && action.payload.pathPrefix) {
-        if (!_.startsWith(action.payload.pathPrefix, `/`)) {
-          action.payload.pathPrefix = `/${action.payload.pathPrefix}`
-        }
-        if (_.endsWith(action.payload.pathPrefix, `/`)) {
-          action.payload.pathPrefix = action.payload.pathPrefix.slice(0, -1)
-        }
-      }
-
-      // If pathPrefix isn't set, set it to an empty string
-      // to avoid it showing up as undefined elsewhere.
-      if (!_.has(action, [`payload`, `pathPrefix`])) {
-        action = _.set(action, [`payload`, `pathPrefix`], ``)
-      }
-
-      // Default polyfill to true.
-      if (!_.has(action, [`payload`, `polyfill`])) {
-        action = _.set(action, [`payload`, `polyfill`], true)
-      }
-
-      return {
-        ...action.payload,
-      }
+      return normalizedPayload
     }
     default:
       return state