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

feat(gatsby-plugin-utils): save validation resulting value to plugin options #27381

Merged
merged 10 commits into from
Oct 19, 2020
5 changes: 4 additions & 1 deletion packages/gatsby-plugin-utils/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,14 @@
"rimraf": "^3.0.2",
"typescript": "^3.9.5"
},
"peerDependencies": {
"gatsby": "^2.24.79"
},
"files": [
"dist/",
"src/"
],
"engines": {
"node": ">=10.13.0"
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { testPluginOptionsSchema } from "../test-plugin-options-schema"
import { ObjectSchema } from "../utils/plugin-options-schema-joi-type"
import { ObjectSchema } from "../joi"

describe(`testPluginOptionsSchema`, () => {
it(`should partially validate one value of a schema`, () => {
Expand Down
31 changes: 16 additions & 15 deletions packages/gatsby-plugin-utils/src/validate.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { BaseValidationOptions } from "joi"
import { ObjectSchema } from "./utils/plugin-options-schema-joi-type"
import { ValidationOptions } from "joi"
import { ObjectSchema } from "./joi"
import { IPluginInfoOptions } from "gatsby"

const validationOptions: BaseValidationOptions = {
const validationOptions: ValidationOptions = {
// Show all errors at once, rather than only the first one every time
abortEarly: false,
cache: true,
Expand All @@ -11,19 +12,19 @@ interface IOptions {
validateExternalRules?: boolean
}

export async function validateOptionsSchema<PluginOptions = object>(
export async function validateOptionsSchema(
pluginSchema: ObjectSchema,
pluginOptions: PluginOptions,
options: IOptions = {}
): Promise<PluginOptions> {
if (options.validateExternalRules === false) {
const result = pluginSchema.validate(pluginOptions, {
...validationOptions,
externals: false,
})
if (result.error) throw result.error
return result.value
pluginOptions: IPluginInfoOptions,
options: IOptions = {
validateExternalRules: true,
}
): Promise<IPluginInfoOptions> {
const { validateExternalRules } = options

return pluginSchema.validateAsync(pluginOptions, validationOptions)
const value = await pluginSchema.validateAsync(pluginOptions, {
...validationOptions,
externals: validateExternalRules,
})

return value
}
2 changes: 2 additions & 0 deletions packages/gatsby/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ export {
withAssetPrefix,
} from "gatsby-link"

export { IPluginInfoOptions } from "./src/bootstrap/load-plugins/types"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line broke TypeScript types as the src folder is not included in the distribution, so you get...

node_modules/gatsby/index.d.ts:29:36 - error TS2307: Cannot find module './src/bootstrap/load-plugins/types' or its corresponding type declarations.

29 export { IPluginInfoOptions } from "./src/bootstrap/load-plugins/types"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you open a new issue for this please? 🙏

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

export const useScrollRestoration: (
key: string
) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,9 @@ Array [
"onCreateWebpackConfig",
],
"pluginOptions": Object {
"allExtensions": false,
"isTSX": true,
"jsxPragma": "React",
"plugins": Array [],
},
"resolve": "",
Expand Down Expand Up @@ -485,6 +488,9 @@ Array [
"onCreateWebpackConfig",
],
"pluginOptions": Object {
"allExtensions": false,
"isTSX": true,
"jsxPragma": "React",
"plugins": Array [],
},
"resolve": "",
Expand Down Expand Up @@ -770,6 +776,9 @@ Array [
"onCreateWebpackConfig",
],
"pluginOptions": Object {
"allExtensions": false,
"isTSX": true,
"jsxPragma": "React",
"plugins": Array [],
},
"resolve": "",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,21 +120,7 @@ describe(`Load plugins`, () => {
expect(plugins).toEqual(
expect.arrayContaining([
expect.objectContaining({
browserAPIs: [],
id: ``,
name: `gatsby-plugin-typescript`,
nodeAPIs: [
`pluginOptionsSchema`,
`resolvableExtensions`,
`onCreateBabelConfig`,
`onCreateWebpackConfig`,
],
pluginOptions: {
plugins: [],
},
resolve: ``,
ssrAPIs: [],
version: `1.0.0`,
}),
])
)
Expand Down Expand Up @@ -169,8 +155,10 @@ describe(`Load plugins`, () => {
`onCreateWebpackConfig`,
],
pluginOptions: {
plugins: [],
allExtensions: false,
isTSX: true,
jsxPragma: `h`,
plugins: [],
},
resolve: ``,
ssrAPIs: [],
Expand Down Expand Up @@ -290,5 +278,34 @@ describe(`Load plugins`, () => {
`)
expect(mockProcessExit).toHaveBeenCalledWith(1)
})

it(`defaults plugin options to the ones defined in the schema`, async () => {
let plugins = await loadPlugins({
plugins: [
{
resolve: `gatsby-plugin-google-analytics`,
options: {
trackingId: `fake`,
},
},
],
})

plugins = replaceFieldsThatCanVary(plugins)

expect(
plugins.find(plugin => plugin.name === `gatsby-plugin-google-analytics`)
.pluginOptions
).toEqual({
// All the options that have defaults are defined
anonymize: false,
exclude: [],
head: false,
pageTransitionDelay: 0,
plugins: [],
respectDNT: false,
trackingId: `fake`,
})
})
})
})
15 changes: 11 additions & 4 deletions packages/gatsby/src/bootstrap/load-plugins/validate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,16 +198,23 @@ export async function validatePluginOptions({
}

try {
// All plugins have "plugins: []"" added to their options in load.ts, even if they
// do not have subplugins. We add plugins to the schema if it does not exist already
// to make sure they pass validation.
if (typeof plugin.pluginOptions === `undefined`) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for some reason pluginOptions can be undefined, so I've handled that case (thanks ts).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have a repro cause it shouldn't

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not specifically. But type script believed it could be. I'm guessing TS is wrong but I didn't try and hunt down where that was coming from. I can try and figure that out and let you know.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that means our gatsby types are wrong I guess ^^

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mxstbr this can be done in a follow-up PR but I would like to see where this comes from cause it means something in our types are wrong

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/** Options passed to the plugin */
pluginOptions?: IPluginInfoOptions
}

return null
}

if (!optionsSchema.describe().keys.plugins) {
mxstbr marked this conversation as resolved.
Show resolved Hide resolved
// All plugins have "plugins: []"" added to their options in load.ts, even if they
// do not have subplugins. We add plugins to the schema if it does not exist already
// to make sure they pass validation.
optionsSchema = optionsSchema.append({
plugins: Joi.array().length(0),
})
}

await validateOptionsSchema(optionsSchema, plugin.pluginOptions)
plugin.pluginOptions = await validateOptionsSchema(
wardpeet marked this conversation as resolved.
Show resolved Hide resolved
optionsSchema,
plugin.pluginOptions
)
} catch (error) {
if (error instanceof Joi.ValidationError) {
reporter.error({
Expand Down