Refactor/speed improvement

[beaunus]

Refactor: Memoize generateClassnamesListSync

This is my first contribution to this repository. Please excuse any mistakes or lack of awareness. 🙏 I suspect that this solution might not be acceptable. I.e. memoizing this function might actually break how the plugin works. 🤔

This fix seems simple enough that the PR itself is the best way to identify the problem and propose a solution.

Description

My team uses eslint-plugin-tailwindcss for a project with ~400 files.

I observed that when we started using this plugin, the execution time for eslint changed dramatically.

Without eslint-plugin-tailwindcss 29 seconds	With eslint-plugin-tailwindcss 684 seconds

Investigation

I wanted to know what function calls are taking the most time. I ran eslint (with the plugin engaged) on a subset of files. I used the Google Chrome profiler.

It seemed clear that the most expensive function calls were readdir. The piece of code within the plugin that called readdir was generateClassnamesListSync.

I added some console logs to see what generateClassnamesListSync was doing.

🎉 generateClassnamesListSync was taking many seconds for each invocation. And, it always returned the same result. Thus, it seemed like memoization would be an easy fix. 👈 Memoization is the essence of this PR.

With the fix, I am able to run eslint on the entire codebase, and it takes about the same amount of time as before.

Type of change

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

There is no logical change whatsoever, so the existing tests should be sufficient.

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published in downstream modules
• I have checked my code and corrected any misspellings

---

If possible, I think this code change should be integrated and released as a patch for all major versions.

[francoismassart]

Hi @beaunus,
Thank you for your contribution. I'll give it a try and if it works as expected I'll publish it as a beta in order to test it out in the wild.

[francoismassart]

@beaunus, I just tried your PR but it'll not keep the "imported" classes list up to date until I restart VSCode...

Similar performance issue(s) were already addressed with this specific part of the plugin, it lead to use REFRESH_RATE in the modified file lib/util/cssFiles.js.

Maybe I could extract the REFRESH_RATE (default is 5s) as an option and you could try to set it to 40_000 (40s) instead of the current 5000 and see how it works for you...

[beaunus]

"Maybe I could extract the REFRESH_RATE (default is 5s) as an option"

Great idea. Thank you.

[francoismassart]

@beaunus , try npm i eslint-plugin-tailwindcss@3.2.0-beta.0 with the cssFilesRefreshRate option

[beaunus]

Thank you @francoismassart .

Is it possible to include the extracted REFRESH_RATE feature on the 1.17 version?

[francoismassart]

@beaunus => eslint-plugin-tailwindcss@2.0.0-beta.0

[francoismassart]

From 1.17.x to 2.x.x is not really a major release, so there is no breaking change but it is simply to match the supported TailwindCSS major version

[francoismassart]

Let me know how it works for you @beaunus

[beaunus]

Let me know how it works for you @beaunus

Thank you @francoismassart . With the extracted cssFilesRefreshRate option, I was only able to get a small performance improvement. I noticed another opportunity for optimization for you to consider.

#93

☝️ With this change, the default 5000 refresh rate is suitable and still leads to a significant performance improvement.

In other words, I propose that we move forward with #93 . Maybe we don't need the extracted cssFilesRefreshRate option.

---

What do you think?