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.

Sign up for GitHub

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): add opt out flag SKIP_PLUGIN_OPTION_VALIDATION to bypass validation #27885

Closed
wants to merge 5 commits into from
Closed

feat(gatsby): add opt out flag SKIP_PLUGIN_OPTION_VALIDATION to bypass validation #27885

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
33846a9
add opt out flag to skip plugin option validation
gillkyle Nov 6, 2020
31a5394
add note to docs about flag
gillkyle Nov 6, 2020
d720f48
Apply suggestions from code review
gillkyle Nov 9, 2020
bb319dc
move warning so validation still happens
gillkyle Nov 9, 2020
e3cad1a
switch assertions in tests
gillkyle Nov 9, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions docs/docs/configuring-usage-with-plugin-options.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,16 @@ describe(`pluginOptionsSchema`, () => {
})
```

### Opting out of plugin option validation

If you are running your site and run into an error trying to develop or build your site due to plugin options failing that are undocumented or seem erroneous, you can add the `SKIP_PLUGIN_OPTION_VALIDATION` flag set to `true`. It can be prepended to your develop and build scripts, included as an [environment variable](/docs/environment-variables/) in a .env file, or included before your `gatsby develop` command like this:

```shell
SKIP_PLUGIN_OPTION_VALIDATION=true gatsby develop
Copy link
Contributor

Choose a reason for hiding this comment

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

Bikeshedding, feel free to ignore/postpone

I'd rather not add this to the docs so explicitly because we don't really want people to use this unless they have a blocking error.

I would prefer if we add that information to the error log if they have an error with an ominous warning, for example something like:

success open and validate gatsby-configs - 0.017s

 ERROR #11331  PLUGIN

Invalid plugin options for "gatsby-plugin-manifest":

- "name" must be a string

If you are certain your configuration is correct, you can disable configuration validation with the SKIP_PLUGIN_OPTION_VALIDATION environment variable. Be careful, this could have unintended side effects!

not finished load plugins - 2.391s

Copy link
Contributor

Choose a reason for hiding this comment

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

Ooh, this is a fun one. Ya, I totally see what you mean. At the same time, how often do people skip error messages and go googling? No strong feelings, just a weird thought exercise.

```

gillkyle marked this conversation as resolved.
Show resolved Hide resolved
Please also [submit an issue](https://github.com/gatsbyjs/gatsby/issues) if you believe the options are failing incorrectly.

## Additional resources

- [Example Gatsby site using plugin options with a local plugin](https://github.com/gatsbyjs/gatsby/tree/master/examples/using-plugin-options)
Expand Down
4 changes: 2 additions & 2 deletions packages/gatsby/src/bootstrap/load-plugins/__tests__/load-plugins.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ describe(`Load plugins`, () => {
},
]
`)
expect(mockProcessExit).toHaveBeenCalledWith(1)
expect(reporter.panic).toHaveBeenCalled()
})

it(`defaults plugin options to the ones defined in the schema`, async () => {
Expand Down Expand Up @@ -362,7 +362,7 @@ describe(`Load plugins`, () => {
},
]
`)
expect(mockProcessExit).toHaveBeenCalledWith(1)
expect(reporter.panic).toHaveBeenCalled()
})
})
})
26 changes: 17 additions & 9 deletions packages/gatsby/src/bootstrap/load-plugins/validate.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,14 +244,16 @@ async function validatePluginsOptions(
rootDir &&
path.relative(rootDir, plugin.parentDir)) ||
null
reporter.error({
id: `11331`,
context: {
configDir,
validationErrors: error.details,
pluginName: plugin.resolve,
},
})
if (!process.env.SKIP_PLUGIN_OPTION_VALIDATION) {
reporter.error({
id: `11331`,
context: {
configDir,
validationErrors: error.details,
pluginName: plugin.resolve,
},
})
}
errors++

return plugin
Expand Down Expand Up @@ -280,7 +282,13 @@ export async function validateConfigPluginsOptions(
config.plugins = plugins

if (errors > 0) {
process.exit(1)
if (process.env.SKIP_PLUGIN_OPTION_VALIDATION) {
reporter.warn(`you have enabled the SKIP_PLUGIN_OPTION_VALIDATION flag,`)
Copy link
Contributor

@mxstbr mxstbr Nov 10, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Nitpick: I'd rather we mention explicitly everytime that there is incorrect configuration, something like:

      reporter.warn(`your gatsby-config.js contains invalid plugin options`)
      reporter.warn(`because you have enabled the SKIP_PLUGIN_OPTION_VALIDATION flag, develop will try continuing`)
      reporter.warn(`but this could have unintended side effects and break your site`)

reporter.warn(`so Gatsby will not verify your plugin configurations.`)
return
} else {
reporter.panic(`Plugins in your gatsby-config failed to pass validation.`)
}
}
}

Expand Down