Enhancing a Stylelint plugin (with some TDD love)
Tags: css, testing, tutorial, webdev
Published: 2023-01-06 11:54:44

In [previous](https://dev.to/mbarzeev/enforcing-your-css-standards-with-a-custom-stylelint-plugin-1o8c) [articles](https://dev.to/mbarzeev/testing-your-stylelint-plugin-5ceh) I’ve created a Stylelint plugin with a single rule which can help you enforce your CSS standards. What this rule did was to make sure that a certain CSS property can only be found in certain files.  
The plugin with this rule can be found on [NPM’s public registry](https://www.npmjs.com/package/@pedalboard/stylelint-plugin-craftsmanlint). 

This is all nice but the Stylelint plugin was somewhat limited. You see, I wanted the rule to allow you to validate all the files for forbidden or allowed properties, and more than that - I would like to be able to declare which CSS properties values are acceptable.
Sounds heavy? Let’s try and break it into smaller chunks as goals. I would like to rule to:

* Support validating all the files the linter inspect
* Support passing a property value as a regex
* Bonus - support inspecting specific files with property values.

Setting goals like this is really great because it lets you focus on what you’re really attempting to accomplish, and they also work great with TDD - Test-First or TDD will allow me to define what I expect and then move on to coding it. This will also enable a smooth refactoring where needed, and trust me, we shall need it :) 

> To better understand this article I recommend you read the two articles which came before it - [Enforcing Your CSS Standards with a Custom Stylelint Plugin](https://dev.to/mbarzeev/enforcing-your-css-standards-with-a-custom-stylelint-plugin-1o8c) and [Testing Your Stylelint Plugin](https://dev.to/mbarzeev/testing-your-stylelint-plugin-5ceh).

Also, all the code mentioned here can be found in the [GitHub repository for this plugin](https://github.com/mbarzeev/pedalboard/tree/master/packages/stylelint-plugin-craftsmanlint).
Put your testing helmet on, here we go!

***

## Preparing
In order to make sure our tests are trustworthy, we need to create at least 2 CSS file fixtures that the linter can inspect. Why 2? Because it will help us test the use cases where we want to allow a certain property in one file but forbid it in another.

I call these fixture files `a.css` and `b.css` and they both contain the same CSS definition:

```css
.my-class {
   font-family: 'Courier New', Courier, monospace;
}
 
.my-second-class {
   font-family: Arial;
}
 
.my-third-class {
   font-family: Courier, Arial, monospace;
   color: black;
}
```

The tests will be conducted on both.

## Support validating all the files the linter inspect
Ok, so our first task is to take the rules the linter already supports and have them check all the files instead of specific ones. Currently the files are defined as an Array in the rule definition, like so:

```json
{
           "stylelint-plugin-craftsmanlint/props-in-files": [
               {
                   "font-family": {
                       forbidden: ["a.css"],
                   },
               },
               {
                   severity: "error",
               },
           ],
       },
```

I would like to use a “reserved” keyword that the rule will know to interpret and act upon. I choose “all”, so instead of ` forbidden: [“a.css”]` it will be ` forbidden: [“all”]`. We can take the `color` property, for example, which can be found both in `a.css` and `b.css` and forbid it. This means that the linter will produce 2 results and in them 2 errors.

Let’s write the test for that:

```javascript
it('should forbid a certain CSS property from all inspected files', async () => {
   const config = {
       plugins: ['./index.ts'],
       rules: {
           'stylelint-plugin-craftsmanlint/props-in-files': [
               {
                   color: {
                       forbidden: ['all'],
                   },
               },
               {
                   severity: 'error',
               },
           ],
       },
   };
 
   const {results} = await lint({
       files: ['src/rules/props-in-files/fixtures/a.css', 'src/rules/props-in-files/fixtures/b.css'],
       config,
   });
 
   expect(results).toHaveLength(2);
   const [{errored: erroredA}, {errored: erroredB}] = results;
   expect(erroredA).toEqual(true);
   expect(erroredB).toEqual(true);
});
 
```

Here we’re testing against both fixture files, and we expect 2 results, that each represents an error.

When running this test it obviously fails, let’s fix that - 
In the linter code we check each forbidden file, but here we need to do something else. We would like to check that if the files array contains the word “all” we will not bother to inspect the file name and report immediately:

```javascript
if (forbiddenFiles) {
               shouldReport = forbiddenFiles.includes(ALL_FILES_KEYWORD) || forbiddenFiles.some(isFileInList);
           }
```

Great, now the test passes, and the other ones as well, making sure that I didn’t introduce any regressions on the way. 

Let’s add this logic to the “allowed” scenario as well. The test is basically the same as the “forbidden” scenario, but we’re asserting that we’re not getting errors in the end, so I’m not writing it here, but I will write the code that satisfies it in the linter:

```javascript
if (allowedFiles) {
               shouldReport = !allowedFiles.includes(ALL_FILES_KEYWORD) && !allowedFiles.some(isFileInList);
           }
```
Both tests pass and it’s time for a small refactor - 
We Have an `isFileInList` method which only checks if the file is in the given list, but I think it should also hold the “all “ logic. We have our tests to keep us from regressing, right? 🙂

```javascript
const isFileValid = (inspectedFile: string, index: number, files: string[]) => {
               return files.includes(ALL_FILES_KEYWORD) || file?.includes(inspectedFile);
           };
 
           if (allowedFiles) {
               shouldReport = !allowedFiles.some(isFileValid);
           }
 
           if (forbiddenFiles) {
               shouldReport = forbiddenFiles.some(isFileValid);
           }
```

Ahh… much better.

## Validating a specific property value
Moving on, we get to the point where we want to start looking into the properties values. Our first goal is to forbid a certain property, which has a certain value, from all the inspected files.
Let’s write our test first as the test helps us define how the rule will look like. 

In this following test we set a rule to forbid any usage of `font-family: Arial;` in any file. Note that this regex only checks for Arial alone, and not Arial among other fonts in the same value.
Given the content of the 2 fixture files we’re inspecting, we should have 2 results with 2 errors and only 2 warnings in total, although the fixture files have more ‘font-family’ properties in them:


```javascript
it('should forbid a certain CSS property with a specific value from all inspected files', async () => {
   const config = {
       plugins: ['./index.ts'],
       rules: {
           'stylelint-plugin-craftsmanlint/props-in-files': [
               {
                   'font-family': {
                       valueRegex: /^Arial$/,
                       forbidden: ['all'],
                   },
               },
               {
                   severity: 'error',
               },
           ],
       },
   };
 
   const {results} = await lint({
       files: ['src/rules/props-in-files/fixtures/a.css', 'src/rules/props-in-files/fixtures/b.css'],
       config,
   });
 
   expect(results).toHaveLength(2);
   const [{errored: erroredA, warnings: warningsA}, {errored: erroredB, warnings: warningsB}] = results;
   expect(erroredA).toEqual(true);
   expect(erroredB).toEqual(true);
   expect(warningsA).toHaveLength(1);
   expect(warningsB).toHaveLength(1);
});
```

The test obviously fails, since it has more warnings than just 1 for each fixture file.
Let’s dive into the code and fix this failure - 
We start with adding the regex to our TS policy type definition:

```javascript
type Policy = {
   forbidden?: string[];
   allowed?: string[];
   valueRegex?: RegExp;
};
```

Here is the raw code to make this test pass:

```javascript
const isFileValid = (inspectedFile: string, index: number, files: string[]) => {
               if (files.includes(ALL_FILES_KEYWORD)) {
                   if (valueRegex) {
                       return valueRegex.test(decl.value);
                   }
                   return true;
               }
               return file?.includes(inspectedFile);
           };
```

Let’s refactor it a bit cause, well, it hard to read and not as elegant as I want - 

```javascript
const isFileValid = (inspectedFile: string, index: number, files: string[]) => {
               let result = false;
               if (files.includes(ALL_FILES_KEYWORD)) {
                   result = valueRegex ? valueRegex.test(decl.value) : true;
               } else {
                   result = file?.includes(inspectedFile) as boolean;
               }
               return result;
           };
```

Remember, we only wish to satisfy our tests, not jump ahead of ourselves. 

We do the same for the “allowed” scenario, but checking that now we’re getting 4 warnings, 2 for each fixture file. Here is the test for it:

```javascript
it('should allow a certain CSS property with a specific value in all inspected files', async () => {
   const config = {
       plugins: ['./index.ts'],
       rules: {
           'stylelint-plugin-craftsmanlint/props-in-files': [
               {
                   'font-family': {
                       valueRegex: /^Arial$/,
                       allowed: ['all'],
                   },
               },
               {
                   severity: 'error',
               },
           ],
       },
   };
 
   const {results} = await lint({
       files: ['src/rules/props-in-files/fixtures/a.css', 'src/rules/props-in-files/fixtures/b.css'],
       config,
   });
 
   expect(results).toHaveLength(2);
   const [{errored: erroredA, warnings: warningsA}, {errored: erroredB, warnings: warningsB}] = results;
   expect(erroredA).toEqual(true);
   expect(erroredB).toEqual(true);
   expect(warningsA).toHaveLength(2);
   expect(warningsB).toHaveLength(2);
});
```

The main difference is the “allowed” instruction and the assertion in the end. Gladly our linter code does not require a change in the linter code for this to pass 🙂

## Bonus - Validating a specific property value for specific files
Our linter knows how to check for specific CSS properties and even properties with a specific value for all inspected files, but it does not support specific property values for specific files (oh dear, my head spins…). 
 
Imagine you need to make sure that `font-family: Arial;` is forbidden only in the `a.css` file, but allowed in any other file.
Let’s write a test for that:

```javascript
it('should forbid a certain CSS property with a specific value from a specific file', async () => {
   const config = {
       plugins: ['./index.ts'],
       rules: {
           'stylelint-plugin-craftsmanlint/props-in-files': [
               {
                   'font-family': {
                       valueRegex: /^Arial$/,
                       forbidden: ['a.css'],
                   },
               },
               {
                   severity: 'error',
               },
           ],
       },
   };
 
   const {results} = await lint({
       files: ['src/rules/props-in-files/fixtures/a.css', 'src/rules/props-in-files/fixtures/b.css'],
       config,
   });
 
   expect(results).toHaveLength(2);
   const [{errored: erroredA, warnings: warningsA}, {errored: erroredB, warnings: warningsB}] = results;
   expect(erroredA).toEqual(true);
   expect(erroredB).toEqual(false);
   expect(warningsA).toHaveLength(1);
   expect(warningsB).toHaveLength(0);
});
```

We expect to have only a single warning though both fixture files have `font-family: Arial;`.
Running the test we obviously fail. Here is the code for fixing it before refactoring:

```javascript
const isFileValid = (inspectedFile: string, index: number, files: string[]) => {
               let result = false;
               if (files.includes(ALL_FILES_KEYWORD)) {
                   result = valueRegex ? valueRegex.test(decl.value) : true;
               } else {
                   if (valueRegex) {
                       result = (file?.includes(inspectedFile) as boolean) && valueRegex.test(decl.value);
                   } else {
                       result = file?.includes(inspectedFile) as boolean;
                   }
               }
               return result;
           };
```

And after refactoring:

```javascript
const isFileValid = (inspectedFile: string, index: number, files: string[]) => {
               let result = false;
               const isRegexValueComply = valueRegex ? valueRegex.test(decl.value) : true;
               if (files.includes(ALL_FILES_KEYWORD)) {
                   result = isRegexValueComply;
               } else {
                   result = (file?.includes(inspectedFile) as boolean) && isRegexValueComply;
               }
               return result;
           };
```

For the sake of the challenge I will leave it to you to write the “forbidden” scenario, but you can always find it in the [plugin repo](https://github.com/mbarzeev/pedalboard/tree/master/packages/stylelint-plugin-craftsmanlint).

And there we go - we have a much better Stylelint plugin 🙂

## Wrapping up
Well, if you stuck this far, kudos to you my friend!

What we now have is a Stylelint plugin which allows us to inspect certain properties within our style files in a much more granular manner. Our goals were fully achieved (including the bonus one):
It supports validating all the files the linter inspect
It supports passing a property value as a regex
It supports inspecting specific files with specific property values.

The package can be found under [npm public registry](https://www.npmjs.com/package/@pedalboard/stylelint-plugin-craftsmanlint) and the code is under the [Pedalboard monorepo](https://github.com/mbarzeev/pedalboard/tree/master/packages/stylelint-plugin-craftsmanlint).

As always, if you have any comments or questions please be sure to leave them in the comments section below so that we can all learn from them :) 

*Hey! for more content like the one you've just read check out <a href="https://twitter.com/mattibarzeev?ref_src=twsrc%5Etfw" class="twitter-follow-button" data-show-count="false">@mattibarzeev</a><script async src="https://platform.twitter.com/widgets.js" charset="utf-8"></script> on Twitter* :beers:

<small><small><small>Photo by <a href="https://unsplash.com/@louishansel?utm_source=unsplash&utm_medium=referral&utm_content=creditCopyText">Louis Hansel</a> on <a href="https://unsplash.com/photos/1B8hIFnHVj8?utm_source=unsplash&utm_medium=referral&utm_content=creditCopyText">Unsplash</a></small></small></small>