-
-
Notifications
You must be signed in to change notification settings - Fork 944
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for profiling rule performance via the TIMING
environment variable
#8108
Add support for profiling rule performance via the TIMING
environment variable
#8108
Conversation
🦋 Changeset detectedLatest commit: 8b83074 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
This looks cool. Normally you ask before taking a status: ask to implement issue. |
Co-authored-by: Masafumi Koba <473530+ybiquitous@users.noreply.github.com>
Co-authored-by: Gary Gozlan <Mouvedia@users.noreply.github.com>
Co-authored-by: Gary Gozlan <Mouvedia@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
I'm reconsidering the |
If you have a separate npm script for each type of lint it shouldn't matter.
I think that it doesn't matter. But with the flag we will have to start discussing whether we add its configuration property and Node.js option counterparts. |
Make sense. Agree with choosing the |
This is not to be implemented in this PR but I gotta ask, what about supporting |
That sounds good, although it should not be a blocker of this PR 👍🏼 Probably, supporting multiple rules would be fine, e.g., |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the changes. Overall LGTM, but I've left a few suggestions to improve the code maintainability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks LGTM 👍🏼
I probably recommend asking @jeddy3 to review it again 😃
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
@ryo-manba, thank you for persevering with the requested changes. The feature is better because of them.
@Mouvedia and @ybiquitous, thank you for your attention to detail regarding code quality. Your efforts here and in other PRs have benefited the entire codebase!
TIMING
environment variable
@ybiquitous @jeddy3 Edit: |
| datasource | package | from | to | | ---------- | --------- | ------- | ------- | | npm | stylelint | 16.12.0 | 16.13.0 | ## [v16.13.0](https://github.com/stylelint/stylelint/blob/HEAD/CHANGELOG.md#16130---2025-01-12) It adds 3 rules to help you write error-free at-rules and 2 rules to warn you about deprecated CSS features. We've turned these rules on in our [standard config](https://www.npmjs.com/package/stylelint-config-standard). It also adds new rule options, a feature to display how long rules take, lax autofix and support for `messageArgs` in more rules. It fixes 7 bugs. Lastly, we've made a deprecation that may affect some plugins. We've updated our docs for [plugin authors](docs/developer-guide/plugins.md#quiet-deprecation-warnings) and [end users](docs/user-guide/options.md#quietdeprecationwarnings) on how to silence deprecation warnings. - Deprecated: ambiguous position arguments passed to `utils.report()` ([#8244](stylelint/stylelint#8244)) ([@romainmenke](https://github.com/romainmenke)). - Added: `lax`/`strict` values to the `fix` Node.js API option and CLI flag ([#8106](stylelint/stylelint#8106)) ([@ryo-manba](https://github.com/ryo-manba)). - Added: support for profiling rule performance via the `TIMING` environment variable ([#8108](stylelint/stylelint#8108)) ([@ryo-manba](https://github.com/ryo-manba)). - Added: `at-rule-descriptor-no-unknown` rule ([#8197](stylelint/stylelint#8197)) ([@ryo-manba](https://github.com/ryo-manba)). - Added: `at-rule-descriptor-value-no-unknown` rule ([#8211](stylelint/stylelint#8211)) ([@ryo-manba](https://github.com/ryo-manba)). - Added: `at-rule-no-deprecated` rule ([#8251](stylelint/stylelint#8251)) ([@jeddy3](https://github.com/jeddy3)). - Added: `at-rule-prelude-no-invalid` rule ([#8268](stylelint/stylelint#8268)) ([@ryo-manba](https://github.com/ryo-manba)). - Added: `declaration-property-value-keyword-no-deprecated` rule ([#8223](stylelint/stylelint#8223)) ([@Mouvedia](https://github.com/Mouvedia)). - Added: `"ignore": ["at-rule-preludes", "declaration-values"]` to `string-no-newline` ([#8214](stylelint/stylelint#8214)) ([@ryo-manba](https://github.com/ryo-manba)). - Added: `messageArgs` to `declaration-property-value-no-unknown`, `font-family-name-quotes`, `font-family-no-duplicate-names`, `function-calc-no-unspaced-operator`, `import-notation`, `media-feature-name-unit-allowed-list`, `selector-attribute-quotes` and `selector-pseudo-element-colon-notation` ([#8285](stylelint/stylelint#8285) & [#8252](stylelint/stylelint#8252)) ([@Mouvedia](https://github.com/Mouvedia)). - Fixed: deprecation warnings to only display once per (custom) rule ([#8265](stylelint/stylelint#8265)) ([@romainmenke](https://github.com/romainmenke)). - Fixed: `*-no-vendor-prefix` message ambiguity ([#8239](stylelint/stylelint#8239)) ([@Mouvedia](https://github.com/Mouvedia)). - Fixed: `at-rule-(dis)allowed-list`, `at-rule-no-vendor-prefix`, `at-rule-property-required-list` message argument ([#8277](stylelint/stylelint#8277)) ([@Mouvedia](https://github.com/Mouvedia)). - Fixed: `at-rule-property-required-list` message for inclusion of properties and descriptors ([#8207](stylelint/stylelint#8207)) ([@jeddy3](https://github.com/jeddy3)). - Fixed: `at-rule-*` false positives and negatives for `@charset` rule ([#8215](stylelint/stylelint#8215)) ([@jeddy3](https://github.com/jeddy3)). - Fixed: `declaration-property-value-no-unknown` false positives for descriptors ([#8240](stylelint/stylelint#8240)) ([@jeddy3](https://github.com/jeddy3)). - Fixed: `property-(dis)allowed-list` false negatives for custom properties, use `/^--/` to (dis)allow them ([#8209](stylelint/stylelint#8209)) ([@fbasmaison-lucca](https://github.com/fbasmaison-lucca)). - Fixed: `property-no-unknown` false positives for descriptors ([#8203](stylelint/stylelint#8203)) ([@jeddy3](https://github.com/jeddy3)). - Fixed: `selector-pseudo-class-no-unknown` false positives for deprecated pseudo-classes ([#8264](stylelint/stylelint#8264)) ([@Mouvedia](https://github.com/Mouvedia)). - Fixed: `selector-type-case` false positives for `hatchPath` ([#8264](stylelint/stylelint#8264)) ([@Mouvedia](https://github.com/Mouvedia)). - Fixed: `selector-type-no-unknown` false positives for `shadow`, `hatch` and `hatchpath` ([#8264](stylelint/stylelint#8264)) ([@Mouvedia](https://github.com/Mouvedia)).
Closes #7350
Added support for
TIMING
environment variable to track rule performance, similar to ESLint's profiling feature.Set
TIMING=1
to display the top 10 longest-running rules orTIMING=all
to display all rules.demo
TIMING=1
EDIT: The specification was modified, and only a single line is displayed when
TIMING=1
.TIMING=all