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

chore(gatsby-plugin-sass): add pluginOptionsSchema export #27466

Merged
merged 6 commits into from
Oct 16, 2020
Merged

chore(gatsby-plugin-sass): add pluginOptionsSchema export #27466

merged 6 commits into from
Oct 16, 2020

Conversation

mfrachet
Copy link
Contributor

@mfrachet mfrachet commented Oct 15, 2020

Description

Add plugin schema options in gatsby-plugin-sass

Relates to #27242 and https://www.gatsbyjs.com/plugins/gatsby-plugin-sass/

Notes:

  • Many options come from https://github.com/sass/node-sass
  • I didn't manage to make some special cases using Joi.when() like the following: file or data must be specified (one or the other is strictly required)
  • I'm not overly confident about this plugin options since it owns a lot of options and sub module options 😓

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Oct 15, 2020
@mfrachet mfrachet removed the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Oct 15, 2020
@mfrachet mfrachet marked this pull request as ready for review October 15, 2020 08:58
Copy link
Contributor

@mxstbr mxstbr left a comment

Choose a reason for hiding this comment

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

This looks fantastic to me! I looked through the plugin-sass and node-sass docs and can't find anything wrong with this schema. However, I agree that the documentation is a bit all over the place and it's hard to confirm the correctness of the schema with confidence 😅

.description(`Override the file regex for SASS`),
useResolveUrlLoader: Joi.alternatives().try(
Joi.boolean(),
Joi.object({}).unknown(true)
Copy link
Contributor

Choose a reason for hiding this comment

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

We should type this better in the future, but it's not important enough for this v1 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I totally agree with you :/ . I found this plugin very tough compared to some others 😅

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

Successfully merging this pull request may close these issues.

2 participants