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
Fix block-no-empty
false positives for reportNeedlessDisables
#6381
Fix block-no-empty
false positives for reportNeedlessDisables
#6381
Conversation
🦋 Changeset detectedLatest commit: a30c5c3 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 |
return ( | ||
statement.nodes !== undefined && statement.nodes.length === 0 // has block | ||
); // and is empty | ||
return hasBlock(statement) && statement.nodes.length === 0; |
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.
[note] It should be more readable and less duplicated thanks to hasBlock
:
stylelint/lib/utils/hasBlock.js
Lines 9 to 11 in 736bc29
module.exports = function hasBlock(statement) { | |
return statement.nodes !== undefined; | |
}; |
function extractStylelintCommand(comment) { | ||
const [command] = comment.text.split(/\s/, 1); | ||
|
||
assertString(command); |
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.
[note] Since ''.split(/\s/, 1)
returns ['']
, this assertString
should prevent command
is undefined
.
9e44543
to
a30c5c3
Compare
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.
This is fantastic, thanks!
LGTM.
Looks like this change broke correct ignore rules that worked before:
results in
|
Ok, this works
I could not find any information in this PR or the release about changes to the disable rule for blocks: from |
@AlexSkrypnyk &__middle__content-middle1 {
// This block is not empty because of including this comment
} You can see it on the demo. It seems you need to fix the code as below: &__top__content-top1 { // stylelint-disable-line block-no-empty
} Or // stylelint-disable-next-line block-no-empty
&__top__content-top1 {
} Or &__top__content-top1 {
// You can put any command except for the Stylelint commands here.
} |
Add missing word-break to stylelint-disable, which makes it working as expected, but newer stylelint requires a word break. Side-effect from stylelint/stylelint#6381 Mistake in ea6a419 Change-Id: If5a8317d82e3e4bdcd6ebf48f9d2af2aea040eb7
* Update TimedMediaHandler from branch 'master' to d64a165c4ee526e6858fd9f97f3d93b39ec3d367 - build: Fix stylelint-disable Add missing word-break to stylelint-disable, which makes it working as expected, but newer stylelint requires a word break. Side-effect from stylelint/stylelint#6381 Mistake in ea6a419 Change-Id: If5a8317d82e3e4bdcd6ebf48f9d2af2aea040eb7
Closes #5388
As suggested in #5388 (comment), this PR extracts a new utility from
assignDisabledRanges.js
, and then it ignores a Stylelint command comment in theblock-no-empty
rule by using the new utility.