fix(gatsby): don't throw on warnings in pluginOptionsSchema (#34182)

* fix: don't throw on warnings * fix: typings * fix: more type fixes * fix: tests and add correct test * tests: fix tests based on cahnges * fix: resolve final test issues * cleanup: remove test console.log * add https to github link * test: fix incorect snapshot
gatsbyjs · Jan 13, 2022 · 252f50d · 252f50d
1 parent d061b1c
commit 252f50d
Show file tree

Hide file tree

Showing 13 changed files with 173 additions and 106 deletions.
diff --git a/packages/gatsby-plugin-cxs/src/__tests__/gatsby-node.js b/packages/gatsby-plugin-cxs/src/__tests__/gatsby-node.js
@@ -3,13 +3,17 @@ import { testPluginOptionsSchema } from "gatsby-plugin-utils"
 import { pluginOptionsSchema } from "../gatsby-node"
 
 it(`should provide meaningful errors when fields are invalid`, async () => {
-  const expectedErrors = [`"optionA" is not allowed`]
+  const expectedWarnings = [`"optionA" is not allowed`]
 
-  const { errors } = await testPluginOptionsSchema(pluginOptionsSchema, {
-    optionA: `This options shouldn't exist`,
-  })
-
-  expect(errors).toEqual(expectedErrors)
+  const { warnings, isValid, hasWarnings } = await testPluginOptionsSchema(
+    pluginOptionsSchema,
+    {
+      optionA: `This options shouldn't exist`,
+    }
+  )
+  expect(isValid).toBe(true)
+  expect(hasWarnings).toBe(true)
+  expect(warnings).toEqual(expectedWarnings)
 })
 
 it.each`

diff --git a/packages/gatsby-plugin-flow/src/__tests__/gatsby-node.js b/packages/gatsby-plugin-flow/src/__tests__/gatsby-node.js
@@ -19,17 +19,18 @@ describe(`onCreateBabelConfig`, () => {
 
 describe(`pluginOptionsSchema`, () => {
   it(`should provide meaningful errors when fields are invalid`, async () => {
-    const expectedErrors = [`"optionA" is not allowed`]
+    const expectedWarnings = [`"optionA" is not allowed`]
 
-    const { isValid, errors } = await testPluginOptionsSchema(
+    const { isValid, warnings, hasWarnings } = await testPluginOptionsSchema(
       pluginOptionsSchema,
       {
         optionA: `This option shouldn't exist`,
       }
     )
 
-    expect(isValid).toBe(false)
-    expect(errors).toEqual(expectedErrors)
+    expect(isValid).toBe(true)
+    expect(hasWarnings).toBe(true)
+    expect(warnings).toEqual(expectedWarnings)
   })
 
   it.each`

diff --git a/packages/gatsby-plugin-manifest/src/__tests__/gatsby-node.js b/packages/gatsby-plugin-manifest/src/__tests__/gatsby-node.js
@@ -532,12 +532,11 @@ describe(`Test plugin manifest options`, () => {
 
 describe(`pluginOptionsSchema`, () => {
   it(`validates options correctly`, async () => {
-    expect(await testPluginOptionsSchema(pluginOptionsSchema, manifestOptions))
-      .toMatchInlineSnapshot(`
-      Object {
-        "errors": Array [],
-        "isValid": true,
-      }
-    `)
+    const { isValid } = await testPluginOptionsSchema(
+      pluginOptionsSchema,
+      manifestOptions
+    )
+
+    expect(isValid).toBe(true)
   })
 })
diff --git a/packages/gatsby-plugin-sass/src/__tests__/gatsby-node.js b/packages/gatsby-plugin-sass/src/__tests__/gatsby-node.js
@@ -170,10 +170,14 @@ describe(`pluginOptionsSchema`, () => {
   })
 
   it(`should allow unknown options`, async () => {
-    const { isValid } = await testPluginOptionsSchema(pluginOptionsSchema, {
-      webpackImporter: `unknown option`,
-    })
+    const { isValid, hasWarnings } = await testPluginOptionsSchema(
+      pluginOptionsSchema,
+      {
+        webpackImporter: `unknown option`,
+      }
+    )
 
     expect(isValid).toBe(true)
+    expect(hasWarnings).toBe(true)
   })
 })
diff --git a/packages/gatsby-plugin-sitemap/src/__tests__/options-validation.js b/packages/gatsby-plugin-sitemap/src/__tests__/options-validation.js
@@ -3,25 +3,31 @@ import { testPluginOptionsSchema, Joi } from "gatsby-plugin-utils"
 
 describe(`pluginOptionsSchema`, () => {
   it(`should provide meaningful errors when fields are invalid`, async () => {
-    const expectedErrors = [`"wrong" is not allowed`]
+    const expectedErrors = [`"output" must be a string`]
 
-    const { errors } = await testPluginOptionsSchema(pluginOptionsSchema, {
-      wrong: `test`,
-    })
+    const { isValid, errors } = await testPluginOptionsSchema(
+      pluginOptionsSchema,
+      {
+        output: 123,
+      }
+    )
 
+    expect(isValid).toBe(false)
     expect(errors).toEqual(expectedErrors)
   })
 
-  it(`should provide error for deprecated "exclude" option`, async () => {
-    const expectedErrors = [
-      `As of v4 the \`exclude\` option was renamed to \`excludes\``,
-    ]
+  it(`should provide warning for deprecated "exclude" option`, async () => {
+    const expectedWarnings = [`"exclude" is not allowed`]
 
-    const { errors } = await testPluginOptionsSchema(pluginOptionsSchema, {
-      exclude: [`test`],
-    })
+    const { warnings, hasWarnings } = await testPluginOptionsSchema(
+      pluginOptionsSchema,
+      {
+        exclude: [`test`],
+      }
+    )
 
-    expect(errors).toEqual(expectedErrors)
+    expect(hasWarnings).toBe(true)
+    expect(warnings).toEqual(expectedWarnings)
   })
 
   it(`creates correct defaults`, async () => {
@@ -48,7 +54,7 @@ describe(`pluginOptionsSchema`, () => {
     ${undefined}
     ${{}}
   `(`should validate the schema: $options`, async ({ options }) => {
-    const { isValid } = await testPluginOptionsSchema(
+    const { isValid, errors } = await testPluginOptionsSchema(
       pluginOptionsSchema,
       options
     )

diff --git a/packages/gatsby-plugin-sitemap/src/options-validation.js b/packages/gatsby-plugin-sitemap/src/options-validation.js
@@ -41,7 +41,8 @@ export const pluginOptionsSchema = ({ Joi }) =>
           }
         }`
       )
-      .external(({ query }) => {
+      .external(pluginOptions => {
+        const query = pluginOptions?.query
         if (query) {
           try {
             parseGraphql(query)
@@ -74,9 +75,6 @@ export const pluginOptionsSchema = ({ Joi }) =>
         enter other data types into this array for custom filtering.
         Doing so will require customization of the \`filterPages\` function.`
       ),
-    exclude: Joi.forbidden().messages({
-      "any.unknown": `As of v4 the \`exclude\` option was renamed to \`excludes\``,
-    }),
     resolveSiteUrl: Joi.function()
       .default(() => resolveSiteUrl)
       .description(

diff --git a/packages/gatsby-plugin-twitter/src/__tests__/gatsby-node.js b/packages/gatsby-plugin-twitter/src/__tests__/gatsby-node.js
@@ -3,13 +3,18 @@ import { testPluginOptionsSchema } from "gatsby-plugin-utils"
 import { pluginOptionsSchema } from "../gatsby-node"
 
 it(`should provide meaningful errors when fields are invalid`, async () => {
-  const expectedErrors = [`"optionA" is not allowed`]
+  const expectedWarnings = [`"optionA" is not allowed`]
 
-  const { errors } = await testPluginOptionsSchema(pluginOptionsSchema, {
-    optionA: `This options shouldn't exist`,
-  })
+  const { warnings, isValid, hasWarnings } = await testPluginOptionsSchema(
+    pluginOptionsSchema,
+    {
+      optionA: `This options shouldn't exist`,
+    }
+  )
 
-  expect(errors).toEqual(expectedErrors)
+  expect(isValid).toBe(true)
+  expect(hasWarnings).toBe(true)
+  expect(warnings).toEqual(expectedWarnings)
 })
 
 it.each`

diff --git a/packages/gatsby-plugin-utils/src/__tests__/index.ts b/packages/gatsby-plugin-utils/src/__tests__/index.ts
@@ -1,5 +1,6 @@
 /* eslint-disable @typescript-eslint/explicit-function-return-type */
 import { validateOptionsSchema, Joi } from "../"
+import { testPluginOptionsSchema } from "../test-plugin-options-schema"
 
 it(`validates a basic schema`, async () => {
   const pluginSchema = Joi.object({
@@ -9,9 +10,9 @@ it(`validates a basic schema`, async () => {
   const validOptions = {
     str: `is a string`,
   }
-  expect(await validateOptionsSchema(pluginSchema, validOptions)).toEqual(
-    validOptions
-  )
+
+  const { value } = await validateOptionsSchema(pluginSchema, validOptions)
+  expect(value).toEqual(validOptions)
 
   const invalid = () =>
     validateOptionsSchema(pluginSchema, {
@@ -56,18 +57,38 @@ it(`does not validate async external validation rules when validateExternalRules
   expect(invalid).not.toThrowError()
 })
 
-it(`throws an error on unknown values`, async () => {
+it(`throws an warning on unknown values`, async () => {
   const schema = Joi.object({
     str: Joi.string(),
   })
 
-  const invalid = () =>
-    validateOptionsSchema(schema, {
+  const validWarnings = [`"notInSchema" is not allowed`]
+
+  const { hasWarnings, warnings } = await testPluginOptionsSchema(
+    () => schema,
+    {
       str: `bla`,
       notInSchema: true,
-    })
-
-  expect(invalid()).rejects.toThrowErrorMatchingInlineSnapshot(
-    `"\\"notInSchema\\" is not allowed"`
+    }
   )
+
+  expect(hasWarnings).toBe(true)
+  expect(warnings).toEqual(validWarnings)
+})
+
+it(`populates default values`, async () => {
+  const pluginSchema = Joi.object({
+    str: Joi.string(),
+    default: Joi.string().default(`default`),
+  })
+
+  const validOptions = {
+    str: `is a string`,
+  }
+
+  const { value } = await validateOptionsSchema(pluginSchema, validOptions)
+  expect(value).toEqual({
+    ...validOptions,
+    default: `default`,
+  })
 })
diff --git a/packages/gatsby-plugin-utils/src/test-plugin-options-schema.ts b/packages/gatsby-plugin-utils/src/test-plugin-options-schema.ts
@@ -5,7 +5,9 @@ import { IPluginInfoOptions } from "./types"
 
 interface ITestPluginOptionsSchemaReturnType {
   errors: Array<string>
+  warnings: Array<string>
   isValid: boolean
+  hasWarnings: boolean
 }
 
 export async function testPluginOptionsSchema(
@@ -44,11 +46,22 @@ export async function testPluginOptionsSchema(
   })
 
   try {
-    await validateOptionsSchema(pluginSchema, pluginOptions)
+    const { warning } = await validateOptionsSchema(pluginSchema, pluginOptions)
+
+    const warnings = warning?.details?.map(detail => detail.message) ?? []
+
+    if (warnings?.length > 0) {
+      return {
+        isValid: true,
+        errors: [],
+        hasWarnings: true,
+        warnings,
+      }
+    }
   } catch (e) {
-    const errors = e.details.map(detail => detail.message)
-    return { isValid: false, errors }
+    const errors = e?.details?.map(detail => detail.message) ?? []
+    return { isValid: false, errors, hasWarnings: false, warnings: [] }
   }
 
-  return { isValid: true, errors: [] }
+  return { isValid: true, errors: [], hasWarnings: false, warnings: [] }
 }
diff --git a/packages/gatsby-plugin-utils/src/utils/plugin-options-schema-joi-type.ts b/packages/gatsby-plugin-utils/src/utils/plugin-options-schema-joi-type.ts
@@ -1160,7 +1160,7 @@ interface AnySchema extends SchemaInternals {
    * Warnings are reported separately from errors alongside the result value via the warning key (i.e. `{ value, warning }`).
    * Warning are always included when calling `any.validate()`.
    */
-  warning(code: string, context: Context): this
+  warning(code: string, context?: Context): this
 
   /**
    * Converts the type into an alternatives type where the conditions are merged into the type definition where:

diff --git a/packages/gatsby-plugin-utils/src/validate.ts b/packages/gatsby-plugin-utils/src/validate.ts
@@ -1,5 +1,5 @@
 import { ValidationOptions } from "joi"
-import { ObjectSchema } from "./joi"
+import { ObjectSchema, Joi } from "./joi"
 import { IPluginInfoOptions } from "./types"
 
 const validationOptions: ValidationOptions = {
@@ -10,21 +10,40 @@ const validationOptions: ValidationOptions = {
 
 interface IOptions {
   validateExternalRules?: boolean
+  returnWarnings?: boolean
+}
+
+interface IValidateAsyncResult {
+  value: IPluginInfoOptions
+  warning: {
+    message: string
+    details: Array<{
+      message: string
+      path: Array<string>
+      type: string
+      context: Array<Record<string, unknown>>
+    }>
+  }
 }
 
 export async function validateOptionsSchema(
   pluginSchema: ObjectSchema,
   pluginOptions: IPluginInfoOptions,
   options: IOptions = {
     validateExternalRules: true,
+    returnWarnings: true,
   }
-): Promise<IPluginInfoOptions> {
-  const { validateExternalRules } = options
+): Promise<IValidateAsyncResult> {
+  const { validateExternalRules, returnWarnings } = options
 
-  const value = await pluginSchema.validateAsync(pluginOptions, {
+  const warnOnUnknownSchema = pluginSchema.pattern(
+    /.*/,
+    Joi.any().warning(`any.unknown`)
+  )
+
+  return (await warnOnUnknownSchema.validateAsync(pluginOptions, {
     ...validationOptions,
     externals: validateExternalRules,
-  })
-
-  return value
+    warnings: returnWarnings,
+  })) as Promise<IValidateAsyncResult>
 }
diff --git a/packages/gatsby/src/bootstrap/load-plugins/__tests__/load-plugins.ts b/packages/gatsby/src/bootstrap/load-plugins/__tests__/load-plugins.ts
@@ -454,7 +454,7 @@ describe(`Load plugins`, () => {
       expect((reporter.warn as jest.Mock).mock.calls[0]).toMatchInlineSnapshot(`
         Array [
           "Warning: there are unknown plugin options for \\"gatsby-plugin-google-analytics\\": doesThisExistInTheSchema
-        Please open an issue at ghub.io/gatsby-plugin-google-analytics if you believe this option is valid.",
+        Please open an issue at https://ghub.io/gatsby-plugin-google-analytics if you believe this option is valid.",
         ]
       `)
       expect(mockProcessExit).not.toHaveBeenCalled()