feat(gatsby): plugin option validation #27242

mxstbr · 2020-10-02T12:00:16Z

This implements plugin option validation if a plugin has a gatsbyNode.pluginOptionsSchema defined. (see #27166 for the RFC for the pluginOptionsSchema API)

I think we should disable this by default until we have documentation for it, added some more schemas to core plugins and (maybe) have tested it with a few alpha/beta testers! So, this PR adds a feature flag in the form of the GATSBY_EXPERIMENTAL_PLUGIN_OPTION_VALIDATION environment variable.

I added the Google Analytics option schema for testing. Here is an example of what a configuration error looks like in the terminal with these incorrect options:

{
  resolve: `gatsby-plugin-google-analytics`,
  options: {
    anonymize: `not a boolean`
  }
}

[ch15946]

laurieontech

Liking the structured errors!

ascorbic

Are we able to add a definition for GatsbyNode["pluginOptionsSchema"] to gatsby/index.d.ts? I'm assuming we'll need to add our own type for Joi ones we extend it? Do you have any idea how that works in general?

packages/gatsby/src/bootstrap/load-plugins/validate.ts

ascorbic · 2020-10-06T08:35:17Z

Looking good!

wardpeet

Some small nits and questions

packages/gatsby/src/bootstrap/load-plugins/__tests__/load-plugins.ts

packages/gatsby-plugin-google-analytics/src/gatsby-node.js

wardpeet · 2020-10-06T12:10:01Z

packages/gatsby-plugin-utils/src/joi.ts

+    dotenv: {
+      args: [`name`],
+      validate(value, helpers, args): void {
+        if (!args.name) {
+          return helpers.error(
+            `any.dotenv requires the environment variable name`
+          )
+        }
+        return value
+      },
+      method(name): Schema {
+        return this.$_addRule({ name: `dotenv`, args: { name } })
+      },
+    },


I might have forgotten but I'm still not sure why we want this one.

This let's plugin authors mark that a value entered during (a potential) gatsby plugin add command should be stored in the .env file instead of being written to the gatsby-config.js directly. E.g.:

// gatsby-plugin-source-contentful/gatsby-node.js export const pluginOptionsSchema = ({ Joi }) => Joi.object({ accessToken: Joi.string().required().dotenv("CONTENTFUL_ACCESS_TOKEN") })

Then, when we prompt the user to enter their access token we will 1) mask the input in the CLI and 2) write the value to the .env file and process.env.CONTENTFUL_ACCESS_TOKEN to the gatsby-config.js 👍

It's still something I'm not 100% convinced about. The feature sounds really cool, but I fear that env vars can be similar and bring conflicts.

Can we strip it out until we have the need for it/more discussions?

Isn't this the main way we can ensure core -> cloud compatibility? @wardpeet can you explain more about what conflicts you anticipate?

I might not 100% understand it so let's say that I have plugin A that has dotenv('HOST') and a plugin B dotenv('HOST') how would this play out? Also what if for some reason I rather have CONTENTFUL_DELIVERY_TOKEN instead of CONTENTFUL_ACCESS_TOKEN?

What do you mean with core <-> cloud compatibility?

So this isn't for the user to understand, but rather the plugin author. We could solve the duplicate problem by concatenating the plugin name and the option name. So GATSBY_PLUGIN_CONTENTFUL_HOST.

The core <-> compatibility is the idea that you'll always be in a valid state and you'd be ready to set up with cloud since it leverages process.env to hook into CMS keys and the like.

That's why I want to hold off on adding this feature cause it needs more thinking.

I have disabled (i.e. commented out) this extension for now until we decide on a fixed API for it to unblock this PR 👍

MichaelDeBoey · 2020-10-06T16:02:06Z

@mxstbr Will it also be possible to validate that we don't pass any options anymore?
For instance in a previous version of a plugin you could pass some options, but they have been removed in a new major version of the plugin and the plugin author wants to notify the user about that.

mxstbr · 2020-10-07T07:02:54Z

Will it also be possible to validate that we don't pass any options anymore?
For instance in a previous version of a plugin you could pass some options, but they have been removed in a new major version of the plugin and the plugin author wants to notify the user about that.

This'll happen automatically! When you publish a new version with a changed options schema the user will get an error after upgrading telling them that they are using options that don't exist 👍

openscript · 2020-10-10T09:47:32Z

I like the idea! .. but it brought some troubles to me. I'm using gatsby-plugin-ts-config to write typed configuration files. Now I'm forced to define a pluginOptionsSchema property, when I configure my GatsbyNode.

The comment says Currently experimental and not enabled by default. so why is it not typed as optional?

pluginOptionsSchema?(args: PluginOptionsSchemaArgs): ObjectSchema

mxstbr · 2020-10-12T06:09:31Z

Great catch @openscript, it should definitely be optional! Would you mind opening a new PR with that change? 🙏

@context

* Make pluginOptionsSchema optional @context: #27242 * Exclude undefined pluginOptionsSchema * Exclude undefined pluginOptionsSchema from plugin validation

MichaelDeBoey · 2020-10-16T11:39:23Z

packages/gatsby-plugin-google-analytics/src/gatsby-node.js

-exports.onPreInit = ({ reporter }, options) => {
-  if (!options.trackingId) {
-    reporter.warn(
-      `The Google Analytics plugin requires a tracking ID. Did you mean to add it?`
-    )
-  }
-}


@mxstbr Shouldn't we have kept onPreInit in case GATSBY_EXPERIMENTAL_PLUGIN_OPTION_VALIDATION wasn't true? 🤔

If people update gatsby-plugin-google-analytics but don't update gatsby, they won't get any warning anymore.

True, good catch! PR welcome 👍

On it boss! 😉

Here you go: #27495 🙂

kamranayub · 2021-02-25T04:50:59Z

Hey team, working on a Gatsby plugins course and I wanted to feature this plugin options schema support but it appears that it isn't working for a local plugin. 🤔

I still have to dig into this PR/the code and see if I can confirm that's the issue. If so I'll open a new ticket and try and contribute a fix since I'd love to showcase this (non-local plugins are introduced later in the course and the local plugin in my demo is a good use case for options schema).

KyleAMathews · 2021-02-25T05:36:40Z

That'd be sweet!

kamranayub · 2021-02-26T02:20:01Z

Yes, looks like it does fail with a small test case so I will open an issue.

it(`validates local plugin schemas`, async () => {
      await loadPlugins({
        plugins: [
          {
            resolve: require.resolve('./__fixtures__/local-plugin'),
            options: {
              optionalString: 1234
            },
          },
        ],
      })

      expect(reporter.error as jest.Mock).toHaveBeenCalledTimes(1)
      expect(mockProcessExit).toHaveBeenCalledWith(1)
    })

kamranayub · 2021-02-26T05:21:23Z

For whoever would like to review, I have a proposed fix: #29787

@context

* Make pluginOptionsSchema optional @context: gatsbyjs#27242 * Exclude undefined pluginOptionsSchema * Exclude undefined pluginOptionsSchema from plugin validation

gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Oct 2, 2020

LekoArts removed the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Oct 5, 2020

mxstbr marked this pull request as ready for review October 5, 2020 12:53

mxstbr changed the title ~~feat(gatsby): pluginOptionsSchema validation~~ feat(gatsby): plugin option validation Oct 5, 2020

laurieontech reviewed Oct 5, 2020

View reviewed changes

Base automatically changed from gatsby-plugin-utils to master October 6, 2020 06:22

ascorbic reviewed Oct 6, 2020

View reviewed changes

packages/gatsby/src/bootstrap/load-plugins/validate.ts Outdated Show resolved Hide resolved

packages/gatsby/src/bootstrap/load-plugins/validate.ts Outdated Show resolved Hide resolved

packages/gatsby/src/bootstrap/load-plugins/validate.ts Show resolved Hide resolved

mxstbr added 16 commits October 6, 2020 13:40

Add pluginOptionsSchema to google analytics for testing

ebfd73b

Add structured error for plugin option validation errors

728fce6

Validate gatsbyNode.pluginOptionsSchema when defined

ac40351

Add first test, fix some bugs discovered based on it

0c83e02

process.exit on invalid plugin options

d553dc9

Much nicer structured error logging for invalid plugin options

9dceae7

Move TODO comment to correct place

6c660ab

Remove unused import

f68f50d

Add new API to gatsby-node API tests

f0ac922

Warn on invalid plugin schema

d3e9e3c

Gate plugin option validation behind feature flag for now

13c9d95

less formal wording

86317b2

Clean up based on code review

cf5f3fc

Type GatsbyNode.pluginOptionsSchema

4e7fe42

Properly type Joi including our .dotenv() extension

7902cc1

Move custom joi + types to gatsby-plugin-utils

3b96006

mxstbr force-pushed the validation-plugin-options-schema branch from 47db081 to 3b96006 Compare October 6, 2020 11:42

wardpeet reviewed Oct 6, 2020

View reviewed changes

MichaelDeBoey mentioned this pull request Oct 6, 2020

feat(gatsby-plugin-utils): add package and methods to validate plugin options #27164

Merged

Disable the .dotenv extension for now

fd55ec3

mxstbr merged commit 9b01ca7 into master Oct 7, 2020

delete-merged-branch bot deleted the validation-plugin-options-schema branch October 7, 2020 10:02

mxstbr mentioned this pull request Oct 7, 2020

chore(gatsby-source-contentful): move to pluginOptionsSchema #27322

Merged

$@mfrachet$ mfrachet mentioned this pull request Oct 8, 2020

feat(gatsby-plugin-utils): add testPluginOptionsSchema to verify plugin schema options #27334

Merged

wardpeet added the hacktoberfest-accepted label Oct 8, 2020

mxstbr mentioned this pull request Oct 8, 2020

docs: document pluginOptionsSchema #27337

Merged

LekoArts mentioned this pull request Oct 8, 2020

feat: pluginOptionsSchema LekoArts/gatsby-source-potterapi#4

Closed

openscript mentioned this pull request Oct 12, 2020

fix(gatsby): Make pluginOptionsSchema optional #27397

Merged

$@mfrachet$ mfrachet mentioned this pull request Oct 12, 2020

fix(gatsby-plugin-google-analytics): remove required on trackingId #27398

Merged

ascorbic pushed a commit that referenced this pull request Oct 12, 2020

fix(gatsby): Make pluginOptionsSchema optional (#27397)

a67fb02

* Make pluginOptionsSchema optional @context: #27242 * Exclude undefined pluginOptionsSchema * Exclude undefined pluginOptionsSchema from plugin validation

MichaelDeBoey reviewed Oct 16, 2020

View reviewed changes

This was referenced Oct 16, 2020

fix(gatsby-plugin-google-analytics): Fix gatsby-node so we still have a warning on older gatsby versions #27495

Merged

feat: Add support for the new pluginOptionsSchema function in gatsby-node gatsby-uc/gatsby-plugin-pinterest#15

Merged

LekoArts mentioned this pull request Nov 6, 2020

Plugin options error for gatsby-plugin-manifest and gatsby-remark-images #27839

Closed

kamranayub mentioned this pull request Feb 25, 2021

Module 3: Add plugin options schema kamranayub/pluralsight-gatsby-demo-starter#11

Merged

kamranayub mentioned this pull request Feb 26, 2021

[gatsby] validate plugin options does not run against a locally resolved plugin #29785

Closed

kamranayub mentioned this pull request Feb 26, 2021

fix(gatsby): validate local plugin options schema #29787

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(gatsby): plugin option validation #27242

feat(gatsby): plugin option validation #27242

mxstbr commented Oct 2, 2020 •

edited

Loading

laurieontech left a comment

ascorbic left a comment

ascorbic commented Oct 6, 2020

wardpeet left a comment

wardpeet Oct 6, 2020

mxstbr Oct 6, 2020

wardpeet Oct 6, 2020

laurieontech Oct 6, 2020

wardpeet Oct 6, 2020 •

edited

Loading

laurieontech Oct 6, 2020

wardpeet Oct 7, 2020

mxstbr Oct 7, 2020

MichaelDeBoey commented Oct 6, 2020

mxstbr commented Oct 7, 2020

openscript commented Oct 10, 2020

mxstbr commented Oct 12, 2020

MichaelDeBoey Oct 16, 2020

mxstbr Oct 16, 2020

MichaelDeBoey Oct 16, 2020

MichaelDeBoey Oct 16, 2020

kamranayub commented Feb 25, 2021

KyleAMathews commented Feb 25, 2021

kamranayub commented Feb 26, 2021

kamranayub commented Feb 26, 2021 •

edited

Loading

feat(gatsby): plugin option validation #27242

feat(gatsby): plugin option validation #27242

Conversation

mxstbr commented Oct 2, 2020 • edited Loading

laurieontech left a comment

Choose a reason for hiding this comment

ascorbic left a comment

Choose a reason for hiding this comment

ascorbic commented Oct 6, 2020

wardpeet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wardpeet Oct 6, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MichaelDeBoey commented Oct 6, 2020

mxstbr commented Oct 7, 2020

openscript commented Oct 10, 2020

mxstbr commented Oct 12, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kamranayub commented Feb 25, 2021

KyleAMathews commented Feb 25, 2021

kamranayub commented Feb 26, 2021

kamranayub commented Feb 26, 2021 • edited Loading

mxstbr commented Oct 2, 2020 •

edited

Loading

wardpeet Oct 6, 2020 •

edited

Loading

kamranayub commented Feb 26, 2021 •

edited

Loading