Skip to content

Commit 1a0d562

Browse files
committed
More obvious error if theme variables are named incorrectly
Also ran clang-format
1 parent 99b9339 commit 1a0d562

File tree

1 file changed

+35
-17
lines changed

1 file changed

+35
-17
lines changed

tools/stylelint/theme-mixin-api.js

Lines changed: 35 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ const plugin = stylelint.createPlugin(ruleName, (isEnabled, options, context) =>
3333
// Name of the component with prefix. e.g. `mat-mdc-button` or `mat-slide-toggle`.
3434
const componentName = matches[1] || matches[3];
3535
// Type of the theme mixin. e.g. `density`, `color`, `theme`.
36-
const type = matches[2] ||matches[4];
36+
const type = matches[2] || matches[4];
3737
// Naively assumes that mixin arguments can be easily retrieved by splitting based on
3838
// a comma. This is not always correct because Sass maps can be constructed in parameters.
3939
// These would contain commas that throw of the argument retrieval. It's acceptable that
@@ -62,35 +62,46 @@ const plugin = stylelint.createPlugin(ruleName, (isEnabled, options, context) =>
6262
const legacyColorExtractExpr = `_mat-legacy-get-theme($theme-or-color-config)`;
6363
const duplicateStylesCheckExpr =
6464
`_mat-check-duplicate-theme-styles(${themePropName}, '${componentName}')`;
65-
const isLegacyColorConfigHandled = node.nodes.find(n => n.type === 'decl' &&
66-
n.prop === themePropName && n.value === legacyColorExtractExpr);
67-
const hasDuplicateStylesCheck = node.nodes.find(n => n.type === 'atrule' &&
68-
n.name === 'include' && n.params === duplicateStylesCheckExpr);
65+
const legacyConfigDecl =
66+
node.nodes.find(n => n.type === 'decl' && n.value === legacyColorExtractExpr);
67+
const hasDuplicateStylesCheck = node.nodes.find(
68+
n =>
69+
n.type === 'atrule' && n.name === 'include' && n.params === duplicateStylesCheckExpr);
6970

70-
if (!isLegacyColorConfigHandled) {
71+
if (!legacyConfigDecl) {
7172
if (context.fix) {
7273
node.insertBefore(0, {prop: themePropName, value: legacyColorExtractExpr});
7374
} else {
74-
reportError(node, `Legacy color API is not handled. Consumers could pass in a ` +
75-
`color configuration directly to the theme mixin. For backwards compatibility, ` +
76-
`use "_mat-legacy-get-theme(...)" to retrieve the theme object.`);
75+
reportError(
76+
node,
77+
`Legacy color API is not handled. Consumers could pass in a ` +
78+
`color configuration directly to the theme mixin. For backwards compatibility, ` +
79+
`use the following declaration to retrieve the theme object: ` +
80+
`${themePropName}: ${legacyColorExtractExpr}`);
7781
}
82+
} else if (legacyConfigDecl.prop !== themePropName) {
83+
reportError(
84+
legacyConfigDecl,
85+
`For consistency, variable for theme should ` +
86+
`be called: ${themePropName}`);
7887
}
7988

8089
if (!hasDuplicateStylesCheck) {
8190
if (context.fix) {
8291
node.insertBefore(0, {name: 'include', params: duplicateStylesCheckExpr});
8392
} else {
84-
reportError(node, `Missing check for duplicative theme styles. Please include the ` +
85-
`"_mat-check-duplicate-theme-styles(...)" mixin.: `);
93+
reportError(
94+
node,
95+
`Missing check for duplicative theme styles. Please include the ` +
96+
`duplicate styles check mixin: ${duplicateStylesCheckExpr}`);
8697
}
8798
}
8899
}
89100

90101
function validateIndividualSystemMixins(node, type, arguments) {
91102
if (arguments.length !== 1) {
92103
reportError(node, 'Expected mixin to only declare a single argument.');
93-
} if (arguments[0] !== '$config-or-theme') {
104+
} else if (arguments[0] !== '$config-or-theme') {
94105
if (context.fix) {
95106
node.params = node.params.replace(arguments[0], '$config-or-theme');
96107
} else {
@@ -100,16 +111,23 @@ const plugin = stylelint.createPlugin(ruleName, (isEnabled, options, context) =>
100111

101112
const expectedProperty = type === 'density' ? '$density-scale' : '$config';
102113
const expectedValue = `mat-get-${type}-config($config-or-theme)`;
103-
const isConfigExtracted = node.nodes.find(n => n.type === 'decl' &&
104-
n.prop === expectedProperty && n.value === expectedValue);
114+
const configExtractionNode =
115+
node.nodes.find(n => n.type === 'decl' && n.value === expectedValue);
105116

106-
if (!isConfigExtracted) {
117+
if (!configExtractionNode) {
107118
if (context.fix) {
108119
node.insertBefore(0, {prop: expectedProperty, value: expectedValue});
109120
} else {
110-
reportError(node, `Config is not extracted. Consumers could pass a theme object. ` +
111-
`Extract the configuration by using: ${expectedProperty}: ${expectedValue}`);
121+
reportError(
122+
node,
123+
`Config is not extracted. Consumers could pass a theme object. ` +
124+
`Extract the configuration by using: ${expectedProperty}: ${expectedValue}`);
112125
}
126+
} else if (configExtractionNode.prop !== expectedProperty) {
127+
reportError(
128+
configExtractionNode,
129+
`For consistency, variable for configuration should ` +
130+
`be called: ${expectedProperty}`);
113131
}
114132
}
115133

0 commit comments

Comments
 (0)