Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 12 commits into from Jan 13, 2022
16 changes: 10 additions & 6 deletions packages/gatsby-plugin-cxs/src/__tests__/gatsby-node.js
Expand Up @@ -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`
Expand Down
9 changes: 5 additions & 4 deletions packages/gatsby-plugin-flow/src/__tests__/gatsby-node.js
Expand Up @@ -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`
Expand Down
13 changes: 6 additions & 7 deletions packages/gatsby-plugin-manifest/src/__tests__/gatsby-node.js
Expand Up @@ -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)
})
})
10 changes: 7 additions & 3 deletions packages/gatsby-plugin-sass/src/__tests__/gatsby-node.js
Expand Up @@ -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)
})
})
32 changes: 19 additions & 13 deletions packages/gatsby-plugin-sitemap/src/__tests__/options-validation.js
Expand Up @@ -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 () => {
Expand All @@ -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
)
Expand Down
6 changes: 2 additions & 4 deletions packages/gatsby-plugin-sitemap/src/options-validation.js
Expand Up @@ -41,7 +41,8 @@ export const pluginOptionsSchema = ({ Joi }) =>
}
}`
)
.external(({ query }) => {
.external(pluginOptions => {
const query = pluginOptions?.query
if (query) {
try {
parseGraphql(query)
Expand Down Expand Up @@ -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(
Expand Down
15 changes: 10 additions & 5 deletions packages/gatsby-plugin-twitter/src/__tests__/gatsby-node.js
Expand Up @@ -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`
Expand Down
41 changes: 31 additions & 10 deletions 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({
Expand All @@ -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, {
Expand Down Expand Up @@ -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`,
})
})
21 changes: 17 additions & 4 deletions packages/gatsby-plugin-utils/src/test-plugin-options-schema.ts
Expand Up @@ -5,7 +5,9 @@ import { IPluginInfoOptions } from "./types"

interface ITestPluginOptionsSchemaReturnType {
errors: Array<string>
warnings: Array<string>
isValid: boolean
hasWarnings: boolean
}

export async function testPluginOptionsSchema(
Expand Down Expand Up @@ -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,
tyhopp marked this conversation as resolved.
Show resolved Hide resolved
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: [] }
}
Expand Up @@ -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:
Expand Down
33 changes: 26 additions & 7 deletions 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 = {
Expand All @@ -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>
}
Expand Up @@ -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()
Expand Down