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

Feature request: scss/function-no-unknown #581

Closed
rdebeasi opened this issue Dec 15, 2021 · 10 comments · Fixed by #591
Closed

Feature request: scss/function-no-unknown #581

rdebeasi opened this issue Dec 15, 2021 · 10 comments · Fixed by #591

Comments

@rdebeasi
Copy link

rdebeasi commented Dec 15, 2021

Problem

Sass doesn't catch typos in function names. The docs recommend using a linter to catch these issues:

Because any unknown function will be compiled to CSS, it’s easy to miss when you typo a function name. Consider running a CSS linter on your stylesheet’s output to be notified when this happens!

I'm unable to find any rules in stylelint-scss that catch mistakes like these, however.

Proposed solution

It would be awesome to have a new rule to catch mistakes like these. It would work like scss/at-rule-no-unknown, but for functions. Let's call it scss/function-no-unknown.

Example

Here's an example of a mistake. I've typed invart instead of invert:

@function invert($color, $amount: 100%) {
  $inverse: change-color($color, $hue: hue($color) + 180);
  @return mix($inverse, $color, $amount);
}

.button {
  background-color: invart(#f00)
}

Sass assumes that invart must be a CSS function and writes it into the compiled CSS:

.button {
  background-color: invart(#f00)
}

Our proposed rule would realize that there's no Sass function called invart and also no CSS function called invart. It would throw an error, alerting us to the fact that we've made a typo.

Thank you so much for your time! :)

@kristerkari
Copy link
Collaborator

@rdebeasi Thanks for the proposal! I see this as a bit tricky to implement, because scss/at-rule-no-unknown is linting for a list of @-rules that are already known.

Functions are something that are created by users, so we don't really know if the function exists in another file, so this rule would really only work if the function is placed in the same file.

@rdebeasi
Copy link
Author

Totally makes sense.

If there's anything that I can do to help to make this proposal more practical, just let me know. Thank you!

@gnuletik
Copy link

gnuletik commented Jan 20, 2022

I'm also looking for a way to avoid these kind of easy mistakes.
I'm currently using the following strategy.

  1. Create a second stylelint config file for CSS errors (e.g. stylelint.dist.config.js)
/*
 * Config to check unknown functions inside `dist` folder (compiled CSS)
 * see https://github.com/stylelint-scss/stylelint-scss/issues/581#issuecomment-1017484418
 *  As recommended in SASS docs
 *  > Because any unknown function will be compiled to CSS,
 *  > it’s easy to miss when you typo a function name.
 *  > Consider running a CSS linter on your stylesheet’s output to be notified when this happens!
 *  https://sass-lang.com/documentation/at-rules/function#plain-css-functions
*/
module.exports = {
  rules: {
    "property-no-unknown": true,
    "media-feature-name-no-unknown": true,
    "at-rule-no-unknown": true,
    "selector-pseudo-class-no-unknown": true,
    "selector-pseudo-element-no-unknown": true,
    "unit-no-unknown": true,

    // add any missing functions here
    "function-allowed-list": [
      "scale",
      "scaleY",
      "rgb",
      "rgba",
      "gradient",
      "linear-gradient",
      "color-stop", // inside webkit-linear-gradient
      "calc",
      "translate",
      "translateX",
      "translateY",
      "translateZ",
      "rotate",
      "url",
      "hsla",
      "drop-shadow",
      "rotateX",
      "rotateY",
      "minmax",
      "var",
      "cubic-bezier",
      "saturate",
      "format", // font
      "alpha", // filter
      "counter"
    ]
  }
}
  1. Edit your build script to check your css after build
   "scripts": {
-    "build": "vite build",
+    "build": "vite build && npm run check:css",
+    "check:css": "stylelint --config stylelint.dist.config.js 'dist/**/*.css'"
   },

However, I did not find a list of known CSS functions so you need to manually add them in your config.

@gnuletik
Copy link

For anyone looking for a solution here:

  • As mentionned on stylelint's linked issue, a new rule may be added to list all functions.
  • You can also use stylelint-css-tree-validator to validate CSS syntax (to avoid listing all functions).

@ybiquitous
Copy link
Contributor

Hi there, the Stylelint built-in function-no-unknown rule will support the ignoreFunctions option with the next version. (stylelint/stylelint#5901)

See also stylelint/stylelint#5865 (comment)

@ybiquitous
Copy link
Contributor

I've opened PR #591 but still draft. I have some difficulties with implementation.
I would be happy if someone could help me make progress.

@kristerkari
Copy link
Collaborator

@ybiquitous looks really nice already. What do you need help with?

@ybiquitous
Copy link
Contributor

@kristerkari Thanks. I've left some comments in #591.

@szaleq
Copy link

szaleq commented Mar 9, 2022

@rdebeasi Thanks for the proposal! I see this as a bit tricky to implement, because scss/at-rule-no-unknown is linting for a list of @-rules that are already known.

Functions are something that are created by users, so we don't really know if the function exists in another file, so this rule would really only work if the function is placed in the same file.

When using Sass @use directive, wouldn't it be possible to look for function definitions in the files imported this way?
Example:

// _functions.scss
@function example() {
    // ..
}

// index.scss
@use "./functions" as fn;

.selector {
    color: fn.example();
}

Would it be possible to scan the _functions.scss file while linting index.scss, list all functions defined in this file and add namespace prefix (fn in this example) and then add this list to the list of valid functions for the current file (index.scss)?

@kristerkari
Copy link
Collaborator

Would it be possible to scan the _functions.scss file while linting index.scss, list all functions defined in this file and add namespace prefix (fn in this example) and then add this list to the list of valid functions for the current file (index.scss)?

Doing something like that would most likely make the rule really slow because you would have to be constantly reading the _functions.scss file on fly and then parse/lint it.

Usually going outside of the current file for linting is just a bad idea.

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

Successfully merging a pull request may close this issue.

5 participants