-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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: less noisy CSS unused rules #1466
Conversation
* filter out duplicate stylesheets * don't report stylesheets that have no rules addresses #1456
R: @ebidel FWIW, I still think flagging the sheets as duplicate is cleaner than not returning them at all, but I totally empathize with the unnecessary noisiness :) |
@@ -101,6 +101,15 @@ | |||
font-size: 20px; | |||
} | |||
</style> | |||
|
|||
<style> | |||
/* Empty on purpose */ |
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.
/* PASS - Empty on purpose */
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.
done
</style> | ||
|
||
<style> | ||
/* Just an import shouldn't show up */ |
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.
PASS - Just an import...
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.
done
@@ -127,7 +132,7 @@ class UnusedCSSRules extends Audit { | |||
const unusedRatio = (unused / usage.length) || 0; | |||
const results = Object.keys(indexedSheets).map(sheetId => { | |||
return UnusedCSSRules.mapSheetToResult(indexedSheets[sheetId]); | |||
}); | |||
}).filter(Boolean); |
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.
Don't feel strongly about it, but .filter(n => n)
would also filter out the falsey values.
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.
Yeah I guess this is a pattern I'm used to, but is there a reason we don't use Boolean
anywhere? !!
seems to be preferred for casting throughout.
7845b2c
to
af413d5
Compare
PTAL :) |
addresses #1456