Monorepo Challenges for PHP_CodeSniffer
Recently, I went to, for the second time, a PHP conference in Brazil called PHP Velho Oeste. It’s a yearly conference all about PHP that happens in the south of Brazil. In this conference, it is not common to see WordPress talks, usually Laravel dominates. But WordPress adjacent talks are not uncommon either. This year in particular had one introducing the PHP_CodeSniffer project. Presented by one of its maintainers: Rodrigo Primo.
Rodrigo is Brazilian and works on this open source project sponsored by Automattic. I had bumped into him before in WordCamps abroad. So, while there, I took the opportunity to ask him a few questions about the project, particularly its lack of “monorepo support”.
It was a good conversation. He pointed me to places where folks tried, or are trying, to introduce better monorepo support to the project. But I wanna share briefly how I’m using PHP_CodeSniffer in a big monorepo, and some of the features I miss.
PHP_CodeSniffer.
A few challenges
I work for a consulting agency called Alley, where we build scalable enterprise solutions for media companies, publishers, and nonprofits. Let me share a scenario that’s very similar to one of the monorepo I work on. Not all details are 100% same, but close. Enough to get an idea of the challenges.
Imagine a monorepo that’s used by multiple brands from one corporation. So each brand has a child theme that extend a parent theme. This parent theme is considered “global” code (shared by all brands). Each brand has their own teams. There are also “global”/shared plugins, and per brand plugins. Again, each separate team (in separate continents) manage them.
monorepo/ ├── themes/ │ ├── parent-global-theme/ │ ├── brand-a-child-theme/ │ ├── brand-b-child-theme/ │ └── brand-c-child-theme/ ├── plugins/ │ ├── global/ │ ├── brand-a/ │ ├── brand-b/ │ └── brand-c/ └── docs/
For performance reasons, we do not run phpcs with a per directory configuration, rather we do it from the root of the monorepo instead. Both locally and CI (including via pre-commit checks for modified files only).
Again, we don’t have custom configs per directory. We have a single root config, and from this config we specify which rules apply to what directories. Which rules are disabled per directory, etc.
Having used the one-config per-dir approach before (even before migrating to the current config organization), I’ve found the approach of having a root config to be way simpler, faster, and maintenance-friendly. It’s just easier for everybody to reason about (I’ve found at least), particularly with many different teams working on a big codebase.
Could we have per directory configs? Yes. But I feel there is something clunky with this approach. It can take longer for CI to run (even though I’d agree the performance impact is highly debatable); it is harder to view errors (since they are presented per directory); and it creates a need to “copy (duplicate) rules” all of the place. Where most of them are just “importing” the main config from the parent. One starts to question the usefulness of per directory config when you are not actually creating per-dir custom rules.
So when I approached Rodrigo to ask about better monorepo support, I had two scenarios in mind. The first scenario I run into is that you can’t set different text-domains from a global config to specific paths (sub-directories). The second scenario is about using DiscouragedFunctionsSniff per subdirectory.
Let’s dig into each one.
Different text-domain per subdirectory
Writing code with i18n support is one of the WordPress best practices, even though a site might not have plans for being translated when it is built. The logic is simple: the best time to add support for i18n is when the code is written, not later.
However, if you don’t plan to translate your site right away, the text-domain doesn’t have that much value. Since nobody is actually creating translations from it. Particularly for a big monorepo, with several plugins and child themes, having each plugin/theme with its unique text domain creates more maintenance problems than it is worth it. Sometimes a text-domain from a specific plugin is called A, but another plugin’s text-domain is called C, but a developer could add the text-domain A into the wrong plugin by mistake.
In the case of our monorepo, the way I chose to make this simpler was to create a global text-domain, and a text-domain per brand. Not per theme or plugin. Each brand has its own text-domain.
And that’s one of the things I mentioned to Rodrigo about somethingPHP_CodeSniffer can’t do today, e.g.: a developer working on a plugin from brand A, and by mistake, adds text-domain into brand B, phpcs can’t report that mistake.
PHP_CodeSniffer only knows the text-domains from brand A and B exist, and that they can be used in the whole monorepo. Not that one should not use a specific text-domain in specific directories. Only a per-dir phpcs config can do this today, which we don’t use.
So this monorepo feature is lacking from PHP_CodeSniffer. Creating a custom sniff1 is also an option I need to explore further.
DiscouragedFunctionsSniff per subdirectory
Another scenario that’s frequent is about using DiscouragedFunctionsSniff. Or similar sniffs per directory.
This particular sniff can be used to discourage usage of a specific function, or to suggest a different function should be used instead. Imagine you created a function that replaces one from WordPress core with some caching, or custom logic from your project, and you want to highlight to a developer that the new version should be used instead. That’s a good use case for DiscouragedFunctionsSniff.
Or imagine you are improving your codebase with a new feature (a new global function) that all brands should start using it. And you can’t simply migrate everything at once to this new helper function. Again, DiscouragedFunctionsSniff is a perfect tool for this kind of situation.
Because one could simply block a legacy function from being used per directory (in brand-specific themes first, then plugins, etc). Today, one can’t do that without creating a custom sniff (which is what we did but is not perfect). But ideally, the library could support this sort of behavior.
Coda
There are other similar examples that I don’t remember now, but I can think deeper if needed. But those two examples are enough to share my point.
Having said all that, and while looking at some other issues and pull requests in their Github repo, e.g., 1, 2, 3, 4. I do think “monorepo support” is tricky because different teams have different ways of managing their directory configs.
Some people want to manage phpcs with per directory configs. And others, like me, want root configs but with more per-directory rules flexibility. Obviously, I don’t know the project technically to say if both can be satisfied. But whatever route the project takes, having better support for monorepo is important and very useful.
:)
I’ve yet to carve out time to try a custom sniff to accomplish this. ↩