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

Integration with stylelint. #9

Open
ben-eb opened this issue Aug 5, 2016 · 6 comments
Open

Integration with stylelint. #9

ben-eb opened this issue Aug 5, 2016 · 6 comments

Comments

@ben-eb
Copy link
Owner

ben-eb commented Aug 5, 2016

This follows on some thoughts I had in the stylelint chat.

/cc @stylelint/core

So, I believe that the primary use case for this project is to provide meaningful feedback to a developer about incorrect CSS, and so stylelint is the best fit for it. Of course, I have my own use case of being able to check validity of CSS properties before merging, but primarily I think stylelint benefits the most from this work. With this in mind, I think we need to discuss how we can integrate this into stylelint!

I am now thinking of not exposing any of the templating/generation that we do in the background, it's just not necessary for the two main use cases that I've outlined (and, it took me a while to see @davidtheclark's logic for not doing this). Instead, css-values will exist as a module on npm which you can require, and won't do any CSS parsing beyond using postcss-value-parser; you will be able to use it to return a warning message or true if the value passed validation.

import cssValues from 'css-values';

/**
 * @param {string} Property to validate
 * @param {string|valueParser} Value to validate
 */
const valid = cssValues('color', 'blue');
// => true

const invalid = cssValues('color', 'yrllow');
// => {message: '"yrllow" is not a valid value for "color".'}

To facilitate this I think we need to move to a monorepo which houses:

  • The css-values module itself;
  • A stylelint plugin which wraps the css-values functionality;
  • A PostCSS plugin which wraps the css-values functionality;
  • https://github.com/ben-eb/postcss-reduce-initial (a spin-off project which uses the same data);
  • and any future module which depends on code/tests from this project.

One of the areas that I've already identified as needing improvements is the lack of feedback from the module on an invalid value. It seems like something that can be addressed fairly easily by changing the return value (in the invalid case) of a lot of the different validator functions. So, if you'd like to contribute then this is a really good way to do so!

Please let me know if you have any questions/suggestions/feedback, it really helps! 😄

@davidtheclark
Copy link
Collaborator

This sounds like a great plan to me, both in terms of the API for a css-values module and the monorepo organization.

@jeddy3
Copy link

jeddy3 commented Aug 6, 2016

Agreed. Sounds great!

We were anticipating the stylelint plugin being called stylelint-declaration-no-valid. The plugin might need to detect non-standard syntax (variable interpolations, dollar variables etc) and ignore declarations that contains that for their property and/or value. Other than that, I suspect the plugin will simply walk the declarations calling cssValues on each.

@ben-eb
Copy link
Owner Author

ben-eb commented Aug 8, 2016

@jeddy3 Did you mean stylelint-declaration-no-invalid?

Ignoring things like string interpolation and variables in the consuming plugin is probably easiest, but it might be worth adding support still as long as this behaviour was opt-in. That facilitates this use case;

h1 {
  color: hsl($hue, $sat, $light, $alpha); // Error: Too many arguments passed to `hsl`. Expected 3.
}

Although, it shouldn't try to validate mixins that are not part of CSS syntax. I'd like validation to occur on a property/value pair, and leave more sophisticated static analysis up to other plugins.

Personally I am not using Sass syntax so I would welcome support for this feature from somebody who would be willing to maintain this.

@jeddy3
Copy link

jeddy3 commented Aug 8, 2016

@jeddy3 Did you mean stylelint-declaration-no-invalid?

Ha, yes I did :)

but it might be worth adding support still as long as this behaviour was opt-in. That facilitates this use case

That would be nice.

Personally I am not using Sass syntax

Same. Hopefully someone will step up though.

@ben-eb
Copy link
Owner Author

ben-eb commented Aug 31, 2016

0.1.0 is now out on npm if you'd like to have a play with it. 😄

@ben-eb
Copy link
Owner Author

ben-eb commented Aug 31, 2016

#26 is also really relevant for stylelint's use case. 😄

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

No branches or pull requests

3 participants