⚠️ No Changeset found

Latest commit: 2b32c47

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

This PR is packaged and the instant preview is available (2b32c47). View the demo website.

Install it locally:

npm i -D https://pkg.pr.new/stylelint@2b32c47

Hey @adalinesimonian,

This looks quite useful, thanks!

Just wondering why didn't you use an existing package for the benchmark part?
e.g. bench-node
I don't care whether we do or not, I am just curious as to why you chose not to.

Just wondering why didn't you use an existing package for the benchmark part? e.g. bench-node I don't care whether we do or not, I am just curious as to why you chose not to.

The benchmarking that I was looking for, which was basically just a pretty simple rundown of the API and CLI's lint run times, felt straightforward enough that it was not worth it to me to bring in a dependency for it. Most of the fun stuff in this PR is the deterministic generation of the test workspaces, and that also felt like a pretty simple thing to have here.

I think if we wanted to do some more advanced analysis, it might be worth pulling in a more featureful package. But at least for the functionality I needed, this seemed straightforward and simple enough. Potentially more maintenance, but it's not user-facing, and I doubt it'll demand frequent iteration. And a small plus of having it here means one less dependency, ergo one less attack vector for a supply chain attack.

[question] Finally, can we unify the existing benchmark-rule into the new benchmark tool?

Looks like that is a simple benchmark of running some rules against a single file. It's probably possible to do something like that here, but the goal of this script as currently written is to test against a realistic-ish workspace rather than a single file. Technically possible, maybe alongside @Mouvedia's suggestion for custom CSS, but I think that can maybe wait until someone runs into the need for it, i.e. they want to run the benchmark analysis this tool provides but against that custom CSS or against a single file.

[question] After merging this PR, does it make sense to run this benchmark for every PR and post results to PR comments?

There's a lot of variability in the conditions CI experiences which we have little control over, so I think that limits the usefulness of running this in CI. It also, by its nature, is meant to take some considerable time to run, and it's hard to interpret the results meaningfully without enough iterations and without running it against all workspace sizes. Therefore, I think even though we, in theory, could run it in CI against e.g. only small workspaces, that wouldn't give us very meaningful data.

Understood, thank you for the quick answers. My questions didn't intend to block this PR. If we get other ideas after this PR, we could revisit them later. 👍🏼

This looks great, thank you.

I'm also eager not to block this PR so we can move on to reviewing your other pull requests with the impressive performance gains! However, it'd be great find the best home for the code and docs from the start.

We surface the rule benchmark in our docs because performance is one of the aspects of our vision. The more people who are aware and use our benchmarking tools when contributing to Stylelint, the better. Let's do the same for this benchmarking tool by moving the docs to /maintainer-guide/benchmarking.md (and consolidating the rule benchmark docs there, too).

Let's move the code into scripts/benchmarking rather than a top-level perf, as scripts is our typical home for these things.

The rule and system benchmarking can remain separate even if the documentation is consolidated. CONTRIBUTING will remain concise and can link through to the benchmarking page, as it currently does for the benchmarking rules docs.

If you can move the code, I'm happy to push a commit later today for the docs to save you from getting embroiled in this repo's documentation structure. (There are some other restructuring PRs I'd like to open alongside it.)

We surface the rule benchmark in our docs because performance is one of the aspects of our vision. The more people who are aware and use our benchmarking tools when contributing to Stylelint, the better. Let's do the same for this benchmarking tool by moving the docs to /maintainer-guide/benchmarking.md (and consolidating the rule benchmark docs there, too).

Done!

Let's move the code into scripts/benchmarking rather than a top-level perf, as scripts is our typical home for these things.

Done as well.

The rule and system benchmarking can remain separate even if the documentation is consolidated. CONTRIBUTING will remain concise and can link through to the benchmarking page, as it currently does for the benchmarking rules docs.

If you can move the code, I'm happy to push a commit later today for the docs to save you from getting embroiled in this repo's documentation structure. (There are some other restructuring PRs I'd like to open alongside it.)

I put the docs together as well; I put them both on the same page, but gave them different headers. Stole your wording from your PR comment for the intro, thought it was good stuff. 😁 Hope that's at least a good baseline, if you think there's room for improvement, feel free to throw some edits in.