hyperoslo / playbook

How we do things, and why
42 stars 8 forks source link

Code style for Stylus (and CSS in general) #70

Open mathiasbno opened 7 years ago

mathiasbno commented 7 years ago

So I started looking into some code styles and linting for Stylus today. As Peter pointed out, making guidelines for unopinionated language like CSS is hard. But I still believe we'll benefit from some structure across projects.

The readme for codestyles in the playbook already outlines some basic rules, but with the addition of linting this can be done in a much more detailed maner.

The most promising linting tool for css/stylus thus far is Stylint. For now they provide linting relying on a .stylintrc file in your project. That has a set of rules you can customise to your liking, but no imports or advanced rule setting. They have a 2.0 version in the making that will aim to mimic the functionality from ESlint that we already use for javascript. There is no ETA, but the project is being actively developed on, although the last PR was a month ago.

This is the default ruleset:

{
    "blocks": false,
    "brackets": "never",
    "colons": "always",
    "colors": "always",
    "commaSpace": "always",
    "commentSpace": "always",
    "cssLiteral": "never",
    "customProperties": [],
    "depthLimit": false,
    "duplicates": true,
    "efficient": "always",
    "exclude": [],
    "extendPref": false,
    "globalDupe": false,
    "groupOutputByFile": true,
    "indentPref": false,
    "leadingZero": "never",
    "maxErrors": false,
    "maxWarnings": false,
    "mixed": false,
    "mixins": [],
    "namingConvention": false,
    "namingConventionStrict": false,
    "none": "never",
    "noImportant": true,
    "parenSpace": false,
    "placeholders": "always",
    "prefixVarsWithDollar": "always",
    "quotePref": false,
    "reporterOptions": {
        "columns": ["lineData", "severity", "description", "rule"],
        "columnSplitter": "  ",
        "showHeaders": false,
        "truncate": true
    },
    "semicolons": "never",
    "sortOrder": "alphabetical",
    "stackedProperties": "never",
    "trailingWhitespace": "never",
    "universal": false,
    "valid": true,
    "zeroUnits": "never",
    "zIndexNormalize": false
}

And this is the modifications I'm using in my test project:

{
    "exclude": ["node_modules/**/*"],
    "extendPref": "@extend",
    "namingConvention": "lowercase-dash",
    "namingConventionStrict": true,
    "none": "always",
    "parenSpace": "never",
    "placeholders": false,
    "quotePref": "single",
    "semicolons": false,
    "universal": "never",
    "zeroUnits": "never"
}

Note that you have to include your changes into the whole config to get the default functionality. Thats a bit unfortunate.

If you have some input on this matter it will be greatly appreciated. What sort of ruleset should we use, is it possible to convert old projects into this ruleset easely or is there anything besides all of this linting that can be done to keep styles more structured in a project.

For some inspiration and further reading check out Github and Googles css styleguides

jgorset commented 7 years ago

I like this! I think HTML and CSS would benefit a lot from linting. I'm really surprised that Stylint is all there is, and that GitHub and Google didn't develop a linter to go with those guides! If there really aren't any alternatives and Stylint is stale, we might want to take it upon ourselves to ressurect it and even make one for HTML, too!

jgorset commented 7 years ago

And for now perhaps we should adopt Stylint, such as it is.

timkurvers commented 7 years ago

Awesome 👍

Briefly discussed this with @mathiasbno last week, it might be possible to create an npm package which contains our default .stylintrc and symlink it from our project: .stylintrc -> node_modules/stylint-config-hyperoslo/.stylintrc. That would prevent us from overriding certain options without dropping the link, but it might be better than nothing.

sindrenm commented 7 years ago

Not sure if I agree on this:

"semicolons": false

Shouldn't we enforce either using semicolons or not?

sindrenm commented 7 years ago

Apart from that it's looking really good. :+1:

jgorset commented 7 years ago

👍 for enforcing no semicolons.

gauravtiwari commented 7 years ago

Same as @mathiasbno and @jgorset 😄

timkurvers commented 7 years ago

Agree with @sindrenm, enforcing is good – that is, set it to either always or never.

jgorset commented 7 years ago

Okay, I think that's majority vote (and pretty sure @mathiasbno agrees, too) – I'll change it.

jgorset commented 7 years ago

Wait a minute, this isn't a pull request…

sindrenm commented 7 years ago

It's not, no. :wink: