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(v2): better error message for invalid plugin config #3979

Merged
merged 3 commits into from
Dec 31, 2020

Conversation

9oelM
Copy link
Contributor

@9oelM 9oelM commented Dec 31, 2020

Motivation

As reported by #3934, passing an invalid type of plugin is ignored silently.

  1. To resolve that issue, this PR adds some lines of code to throw error when invalid type of plugin is detected.
  2. This PR also improves tests on validating plugins based on ConfigSchema and the typing of PluginSchema itself to an ordered one. Any type of plugin that has a wrong signature but somehow bypasses the check in Adds back support for the "threeColumn" layout to GridBlock.js #1 should be detected by schema validation.

Have you read the Contributing Guidelines on pull requests?

yes

Test Plan

Tested locally by manually injecting a function into plugins like so:

  const plugins: InitPlugin[] = [...pluginConfigs, function() {}]
    .map((pluginItem) => {
    ...

It would give an error:

Screen Shot 2020-12-31 at 12 03 12 PM

Also tested without invalid plugin. No errors. It actually should be proven by checks passing on this PR.

Will close #3934 if merged

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Dec 31, 2020
@github-actions
Copy link

Size Change: 0 B

Total Size: 156 kB

ℹ️ View Unchanged
Filename Size Change
website/build/blog/2017/12/14/introducing-docusaurus/index.html 20.7 kB 0 B
website/build/docs/introduction/index.html 180 B 0 B
website/build/index.html 5.82 kB 0 B
website/build/main.********.js 112 kB 0 B
website/build/styles.********.css 17.6 kB 0 B

compressed-size-action

@netlify
Copy link

netlify bot commented Dec 31, 2020

✔️ Deploy preview for docusaurus-2 ready!
Built without sensitive environment variables

🔨 Explore the source changes: 6505865

🔍 Inspect the deploy logs: https://app.netlify.com/sites/docusaurus-2/deploys/5fed3fd5270be3000819238e

😎 Browse the preview: https://deploy-preview-3979--docusaurus-2.netlify.app

@github-actions
Copy link

⚡️ Lighthouse report for the changes in this PR:

Category Score
🟠 Performance 84
🟢 Accessibility 99
🟢 Best practices 100
🟢 SEO 100
🟢 PWA 95

Lighthouse ran on https://deploy-preview-3979--docusaurus-2.netlify.app/classic/

@slorber slorber changed the title Invalid plugin prevention feat(v2): better error message for invalid plugin config Dec 31, 2020
@slorber slorber added the pr: new feature This PR adds a new API or behavior. label Dec 31, 2020
@slorber
Copy link
Collaborator

slorber commented Dec 31, 2020

Thanks, LGTM

However this will not close the related issue, as we still don't accept plugins as plain functions.

I mean

function MyPlugin() {
  return {name: "my-plugin"}
}

module.exports {
  plugins: [
    MyPlugin,
   [MyPlugin,{}],
  ],
}

@slorber slorber merged commit 869e118 into facebook:master Dec 31, 2020
@lex111 lex111 added this to the v2.0.0-alpha.71 milestone Mar 1, 2021
This was referenced Mar 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA pr: new feature This PR adds a new API or behavior.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Passing a plain function as a plugin is ignored silently
4 participants