Enhancing a Stylelint plugin (with some TDD love)

Matti Bar-Zeev - Jan 6 '23 - - Dev Community

In previous articles 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.

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 and Testing Your Stylelint Plugin.

Also, all the code mentioned here can be found in the GitHub repository for this plugin.
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:

.my-class {
   font-family: 'Courier New', Courier, monospace;
}

.my-second-class {
   font-family: Arial;
}

.my-third-class {
   font-family: Courier, Arial, monospace;
   color: black;
}
Enter fullscreen mode Exit fullscreen mode

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:

{
           "stylelint-plugin-craftsmanlint/props-in-files": [
               {
                   "font-family": {
                       forbidden: ["a.css"],
                   },
               },
               {
                   severity: "error",
               },
           ],
       },
Enter fullscreen mode Exit fullscreen mode

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:

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);
});

Enter fullscreen mode Exit fullscreen mode

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:

if (forbiddenFiles) {
               shouldReport = forbiddenFiles.includes(ALL_FILES_KEYWORD) || forbiddenFiles.some(isFileInList);
           }
Enter fullscreen mode Exit fullscreen mode

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:

if (allowedFiles) {
               shouldReport = !allowedFiles.includes(ALL_FILES_KEYWORD) && !allowedFiles.some(isFileInList);
           }
Enter fullscreen mode Exit fullscreen mode

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? 🙂

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);
           }
Enter fullscreen mode Exit fullscreen mode

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:

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);
});
Enter fullscreen mode Exit fullscreen mode

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:

type Policy = {
   forbidden?: string[];
   allowed?: string[];
   valueRegex?: RegExp;
};
Enter fullscreen mode Exit fullscreen mode

Here is the raw code to make this test pass:

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);
           };
Enter fullscreen mode Exit fullscreen mode

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

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;
           };
Enter fullscreen mode Exit fullscreen mode

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:

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);
});
Enter fullscreen mode Exit fullscreen mode

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:

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);
});
Enter fullscreen mode Exit fullscreen mode

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:

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;
           };
Enter fullscreen mode Exit fullscreen mode

And after refactoring:

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;
           };
Enter fullscreen mode Exit fullscreen mode

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.

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 and the code is under the Pedalboard monorepo.

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 @mattibarzeev on Twitter 🍻

Photo by Louis Hansel on Unsplash

. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .