Skip to content

Commit

Permalink
build: add stylelint rule to ban concrete CSS rules inside theme files (
Browse files Browse the repository at this point in the history
#10003)

Adds a stylelint rule that will ensure that all of the CSS rules inside theme files are placed inside a mixin. This avoids style duplication whenever the file is imported.

Relates to #9999.
  • Loading branch information
crisbeto authored and jelbourn committed Feb 27, 2018
1 parent 6ec0af1 commit 1cf52a9
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 17 deletions.
10 changes: 4 additions & 6 deletions src/lib/button/_button-base.scss
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ $mat-mini-fab-size: 40px !default;
$mat-mini-fab-padding: 8px !default;

// Applies base styles to all button types.
%mat-button-base {
@mixin mat-button-base {
box-sizing: border-box;
position: relative;

Expand Down Expand Up @@ -69,9 +69,8 @@ $mat-mini-fab-padding: 8px !default;
}

// Applies styles to buttons with backgrounds: raised, fab, and mini-fab
%mat-raised-button {
@extend %mat-button-base;

@mixin mat-raised-button {
@include mat-button-base;
@include mat-overridable-elevation(2);

// Force hardware acceleration.
Expand All @@ -92,8 +91,7 @@ $mat-mini-fab-padding: 8px !default;

// Applies styles to fab and mini-fab button types only
@mixin mat-fab($size, $padding) {
@extend %mat-raised-button;

@include mat-raised-button;
@include mat-overridable-elevation(6);

// Reset the min-width from the button base.
Expand Down
12 changes: 5 additions & 7 deletions src/lib/button/button.scss
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@
@import '../core/style/layout-common';
@import '../../cdk/a11y/a11y';

.mat-button, .mat-icon-button {
@extend %mat-button-base;
.mat-button, .mat-icon-button, .mat-stroked-button, .mat-flat-button {
@include mat-button-base;
}

.mat-button, .mat-icon-button {
.mat-button-focus-overlay {
transition: none;
opacity: 0;
Expand All @@ -21,12 +23,10 @@
}

.mat-raised-button {
@extend %mat-raised-button;
@include mat-raised-button;
}

.mat-stroked-button {
@extend %mat-button-base;

@include mat-overridable-elevation(0);

border: 1px solid currentColor;
Expand All @@ -35,8 +35,6 @@
}

.mat-flat-button {
@extend %mat-button-base;

@include mat-overridable-elevation(0);
}

Expand Down
12 changes: 8 additions & 4 deletions stylelint-config.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
"./tools/stylelint/no-prefixes/no-prefixes.js",
"./tools/stylelint/selector-nested-pattern-scoped/index.js",
"./tools/stylelint/selector-no-deep/index.js",
"./tools/stylelint/no-nested-mixin/index.js"
"./tools/stylelint/no-nested-mixin/index.js",
"./tools/stylelint/no-concrete-rules/index.js"
],
"rules": {
"material/no-prefixes": [["last 2 versions", "not ie <= 10", "not ie_mob <= 10"]],
Expand All @@ -13,6 +14,9 @@
"message": "The & operator is not allowed at the end of theme selectors.",
"filePattern": "-theme\\.scss$"
}],
"material/no-concrete-rules": [true, {
"filePattern": "^_.*\\.scss$"
}],

"color-hex-case": "lower",
"color-no-invalid-hex": true,
Expand Down Expand Up @@ -43,7 +47,7 @@

"property-case": "lower",

"declaration-block-no-duplicate-properties": [ true, {
"declaration-block-no-duplicate-properties": [true, {
"ignore": ["consecutive-duplicates-with-different-values"]
}],
"declaration-block-trailing-semicolon": "always",
Expand All @@ -53,8 +57,8 @@
"declaration-block-semicolon-newline-before": "never-multi-line",
"declaration-block-semicolon-newline-after": "always-multi-line",
"declaration-property-value-blacklist": [
{ "/.*/": ["initial"] },
{ "message": "The `initial` value is not supported in IE."}
{"/.*/": ["initial"]},
{"message": "The `initial` value is not supported in IE."}
],

"block-closing-brace-newline-after": "always",
Expand Down
39 changes: 39 additions & 0 deletions tools/stylelint/no-concrete-rules/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
const stylelint = require('stylelint');
const path = require('path');
const ruleName = 'material/no-concrete-rules';
const messages = stylelint.utils.ruleMessages(ruleName, {
expected: pattern => `CSS rules must be placed inside a mixin for files matching '${pattern}'.`
});

/**
* Stylelint plugin that will log a warning for all top-level CSS rules.
* Can be used in theme files to ensure that everything is inside a mixin.
*/
const plugin = stylelint.createPlugin(ruleName, (isEnabled, options) => {
return (root, result) => {
if (!isEnabled) return;

const filePattern = new RegExp(options.filePattern);
const fileName = path.basename(root.source.input.file);

if (!filePattern.test(fileName)) return;

// Go through all the nodes and report a warning for every CSS rule or mixin inclusion.
// We use a regular `forEach`, instead of the PostCSS walker utils, because we only care
// about the top-level nodes.
root.nodes.forEach(node => {
if (node.type === 'rule' || (node.type === 'atrule' && node.name === 'include')) {
stylelint.utils.report({
result,
ruleName,
node,
message: messages.expected(filePattern)
});
}
});
};
});

plugin.ruleName = ruleName;
plugin.messages = messages;
module.exports = plugin;

0 comments on commit 1cf52a9

Please sign in to comment.