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

RFC: Option validation approach for themes and plugins #8

Closed
teikjun opened this issue Jun 15, 2020 · 5 comments
Closed

RFC: Option validation approach for themes and plugins #8

teikjun opened this issue Jun 15, 2020 · 5 comments
Assignees
Labels

Comments

@teikjun
Copy link
Member

teikjun commented Jun 15, 2020

💥 Proposal

Rationale

In view of the migration CLI command in facebook#2267 and the reasons explained in facebook#2878, it would be useful to have option validation for docusaurus.config.js. In this RFC, we hope to get feedback on an approach to validate options for themes and plugins before we start implementing it.

Option validation for plugins

To validate the user’s options for plugins, we can create a validateOptions method for each plugin. This method takes in the user’s plugins options and validates it with Yup.

validateOptions will be called (in packages/docusaurus/src/server/plugins/init.ts ) and it will return an actionable error message for the user if the input does not match the correct format for plugins (or its Typescript types).

Option validation for themes

For themes, we can create a validateOptions method that takes in the user’s themes options, and a validateConfig method that takes in the user’s themeConfig. Similarly, both methods will validate their input with Yup.

validateOptions and validateConfig will be called (in packages/docusaurus/src/server/index.ts) and return an actionable error message if the user’s input does not match the correct format for themes or themeConfig respectively.

Have you read the Contributing Guidelines on issues?

Yes

@anshulrgoyal
Copy link
Member

validateOptions and validateConfig would be static methods so that it can be used in doctor command later.

@anshulrgoyal
Copy link
Member

There are two possible behavior to handle validation error:-

  • Stopping the build and throwing error
  • Ignoring invalid options and using default options to continue the building. Users can be warned about these errors.

@teikjun teikjun changed the title RFC: Option validation for themes and plugins RFC: Option validation approach for themes and plugins Jun 15, 2020
@slorber
Copy link
Collaborator

slorber commented Jun 15, 2020

Hi,

Why open the issue on this fork, and not on the main repo?

validateOptions and validateConfig would be static methods so that it can be used in doctor command later.

What do you mean by static? That it wouldn't be a lifecycle method?

I think we should validate options/config as soon as possible, possibly even before instanciating the plugin.

In client-redirects I do this in the plugin instanciation but sooner could be great.

image

There are two possible behavior to handle validation error:-

I'd rather fail fast by default. We can be less strict later if users complain.

@slorber
Copy link
Collaborator

slorber commented Jun 15, 2020

plugins/init.ts seems a good place for validateOptions

image

Not sure, but I think themes also use this codepath, so it can be good enough for both plugins/themes/presets.

for validateConfig , I'd put it just before the initPlugins call


One thing that could be handy is also provide a way to normalize the plugin options.

Sometimes, we want to allow the user to pass partial options, but for the internal plugin code, it may not be the most convenient to deal with an options object that can be {}, or with types such as string | string[] | undefined.

See for example https://github.com/facebook/docusaurus/blob/master/packages/docusaurus-plugin-client-redirects/src/normalizePluginOptions.ts

I think we should generalize the pattern in some kind of "preprocessing" step, where you validate/transform raw user input into sanitized/normalized/convenient options object.

@teikjun
Copy link
Member Author

teikjun commented Jun 15, 2020

Thank you very much for your advice! :)

What do you mean by static? That it wouldn't be a lifecycle method?

Yes, we were thinking of using a non-lifecycle method.

In client-redirects I do this in the plugin instanciation but sooner could be great.

Got it! We'll try to model the validation and normalization after the client-redirects plugin, and look for a way to validate the plugins even earlier.

Also, we'll put our RFCs on the main repo next time! 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants