Skip to content

Commit 97c00a0

Browse files
crisbetocrisbeto
committed
build: update stylelint and minor cleanup of custom rules (#25224)
Updates us to the latest version of Stylelint and does some minor cleanup of our custom rules. Includes: * Removing some `as any` casts that aren't necessary anymore. * Moving out some functions so that they aren't recreated for each file that is being linted. (cherry picked from commit 1fec8f7)
1 parent c8ccbd7 commit 97c00a0

File tree

4 files changed

+187
-176
lines changed

4 files changed

+187
-176
lines changed

‎package.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -197,8 +197,8 @@
197197
"moment": "^2.29.1",
198198
"node-fetch": "^2.6.0",
199199
"parse5": "^6.0.1",
200-
"postcss": "^8.4.5",
201-
"postcss-scss": "^4.0.3",
200+
"postcss": "^8.4.14",
201+
"postcss-scss": "^4.0.4",
202202
"protractor": "^7.0.0",
203203
"reflect-metadata": "^0.1.13",
204204
"requirejs": "^2.3.6",
@@ -209,7 +209,7 @@
209209
"semver": "^7.3.5",
210210
"send": "^0.17.2",
211211
"shelljs": "^0.8.5",
212-
"stylelint": "^14.5.0",
212+
"stylelint": "^14.9.1",
213213
"terser": "^5.10.0",
214214
"ts-node": "^10.8.1",
215215
"tsec": "0.2.2",

‎tools/stylelint/single-line-comment-only.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,8 @@ const plugin = createPlugin(ruleName, (isEnabled: boolean, options?: {filePatter
2424
}
2525

2626
root.walkComments(comment => {
27-
// The `raws.inline` property isn't in the typing so we need to cast to any. Also allow
28-
// comments starting with `!` since they're used to tell minifiers to preserve the comment.
29-
if (!(comment.raws as any).inline && !comment.text.startsWith('!')) {
27+
// Allow comments starting with `!` since they're used to tell minifiers to preserve the comment.
28+
if (!comment.raws.inline && !comment.text.startsWith('!')) {
3029
utils.report({
3130
result,
3231
ruleName,

‎tools/stylelint/theme-mixin-api.ts

Lines changed: 159 additions & 135 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {createPlugin, utils} from 'stylelint';
1+
import {createPlugin, utils, PostcssResult} from 'stylelint';
22
import {basename} from 'path';
33
import {AtRule, Declaration, Node} from 'postcss';
44

@@ -49,159 +49,183 @@ const plugin = createPlugin(ruleName, (isEnabled: boolean, _options, context) =>
4949
const args = matches[2].split(',').map(arg => arg.trim());
5050

5151
if (type === 'theme') {
52-
validateThemeMixin(node, args);
52+
validateThemeMixin(result, componentName, node, args, !!context.fix);
5353
} else {
54-
validateIndividualSystemMixins(node, type, args);
54+
validateIndividualSystemMixins(result, node, type, args, !!context.fix);
5555
}
5656
});
57+
};
58+
});
5759

58-
function validateThemeMixin(node: AtRule, args: string[]) {
59-
if (args.length !== 1) {
60-
reportError(node, 'Expected theme mixin to only declare a single argument.');
61-
} else if (args[0] !== '$theme-or-color-config') {
62-
if (context.fix) {
63-
node.params = node.params.replace(args[0], '$theme-or-color-config');
64-
} else {
65-
reportError(node, 'Expected first mixin argument to be called `$theme-or-color-config`.');
66-
}
67-
}
68-
69-
const themePropName = `$theme`;
70-
const legacyColorExtractExpr = anyPattern(
71-
`<..>.private-legacy-get-theme($theme-or-color-config)`,
72-
);
73-
const duplicateStylesCheckExpr = anyPattern(
74-
`<..>.private-check-duplicate-theme-styles(${themePropName}, '${componentName}')`,
60+
/** Validates a `theme` mixin. */
61+
function validateThemeMixin(
62+
result: PostcssResult,
63+
componentName: string,
64+
node: AtRule,
65+
args: string[],
66+
shouldFix: boolean,
67+
) {
68+
if (args.length !== 1) {
69+
reportError(result, node, 'Expected theme mixin to only declare a single argument.');
70+
} else if (args[0] !== '$theme-or-color-config') {
71+
if (shouldFix) {
72+
node.params = node.params.replace(args[0], '$theme-or-color-config');
73+
} else {
74+
reportError(
75+
result,
76+
node,
77+
'Expected first mixin argument to be called `$theme-or-color-config`.',
7578
);
79+
}
80+
}
7681

77-
let legacyConfigDecl: Declaration | null = null;
78-
let duplicateStylesCheck: AtRule | null = null;
79-
let hasNodesOutsideDuplicationCheck = false;
80-
let isLegacyConfigRetrievalFirstStatement = false;
81-
82-
if (node.nodes) {
83-
for (let i = 0; i < node.nodes.length; i++) {
84-
const childNode = node.nodes[i];
85-
if (childNode.type === 'decl' && legacyColorExtractExpr.test(childNode.value)) {
86-
legacyConfigDecl = childNode;
87-
isLegacyConfigRetrievalFirstStatement = i === 0;
88-
} else if (
89-
childNode.type === 'atrule' &&
90-
childNode.name === 'include' &&
91-
duplicateStylesCheckExpr.test(childNode.params)
92-
) {
93-
duplicateStylesCheck = childNode;
94-
} else if (childNode.type !== 'comment') {
95-
hasNodesOutsideDuplicationCheck = true;
96-
}
97-
}
98-
}
82+
const themePropName = `$theme`;
83+
const legacyColorExtractExpr = anyPattern(
84+
`<..>.private-legacy-get-theme($theme-or-color-config)`,
85+
);
86+
const duplicateStylesCheckExpr = anyPattern(
87+
`<..>.private-check-duplicate-theme-styles(${themePropName}, '${componentName}')`,
88+
);
9989

100-
if (!legacyConfigDecl) {
101-
reportError(
102-
node,
103-
`Legacy color API is not handled. Consumers could pass in a ` +
104-
`color configuration directly to the theme mixin. For backwards compatibility, ` +
105-
`use the following declaration to retrieve the theme object: ` +
106-
`${themePropName}: ${legacyColorExtractExpr}`,
107-
);
108-
} else if (legacyConfigDecl.prop !== themePropName) {
109-
reportError(
110-
legacyConfigDecl,
111-
`For consistency, theme variable should be called: ${themePropName}`,
112-
);
113-
}
90+
let legacyConfigDecl: Declaration | null = null;
91+
let duplicateStylesCheck: AtRule | null = null;
92+
let hasNodesOutsideDuplicationCheck = false;
93+
let isLegacyConfigRetrievalFirstStatement = false;
11494

115-
if (!duplicateStylesCheck) {
116-
reportError(
117-
node,
118-
`Missing check for duplicative theme styles. Please include the ` +
119-
`duplicate styles check mixin: ${duplicateStylesCheckExpr}`,
120-
);
95+
if (node.nodes) {
96+
for (let i = 0; i < node.nodes.length; i++) {
97+
const childNode = node.nodes[i];
98+
if (childNode.type === 'decl' && legacyColorExtractExpr.test(childNode.value)) {
99+
legacyConfigDecl = childNode;
100+
isLegacyConfigRetrievalFirstStatement = i === 0;
101+
} else if (
102+
childNode.type === 'atrule' &&
103+
childNode.name === 'include' &&
104+
duplicateStylesCheckExpr.test(childNode.params)
105+
) {
106+
duplicateStylesCheck = childNode;
107+
} else if (childNode.type !== 'comment') {
108+
hasNodesOutsideDuplicationCheck = true;
121109
}
110+
}
111+
}
122112

123-
if (hasNodesOutsideDuplicationCheck) {
124-
reportError(
125-
node,
126-
`Expected nodes other than the "${legacyColorExtractExpr}" ` +
127-
`declaration to be nested inside the duplicate styles check.`,
128-
);
129-
}
113+
if (!legacyConfigDecl) {
114+
reportError(
115+
result,
116+
node,
117+
`Legacy color API is not handled. Consumers could pass in a ` +
118+
`color configuration directly to the theme mixin. For backwards compatibility, ` +
119+
`use the following declaration to retrieve the theme object: ` +
120+
`${themePropName}: ${legacyColorExtractExpr}`,
121+
);
122+
} else if (legacyConfigDecl.prop !== themePropName) {
123+
reportError(
124+
result,
125+
legacyConfigDecl,
126+
`For consistency, theme variable should be called: ${themePropName}`,
127+
);
128+
}
130129

131-
if (legacyConfigDecl !== null && !isLegacyConfigRetrievalFirstStatement) {
132-
reportError(
133-
legacyConfigDecl,
134-
'Legacy configuration should be retrieved first in theme mixin.',
135-
);
136-
}
130+
if (!duplicateStylesCheck) {
131+
reportError(
132+
result,
133+
node,
134+
`Missing check for duplicative theme styles. Please include the ` +
135+
`duplicate styles check mixin: ${duplicateStylesCheckExpr}`,
136+
);
137+
}
138+
139+
if (hasNodesOutsideDuplicationCheck) {
140+
reportError(
141+
result,
142+
node,
143+
`Expected nodes other than the "${legacyColorExtractExpr}" ` +
144+
`declaration to be nested inside the duplicate styles check.`,
145+
);
146+
}
147+
148+
if (legacyConfigDecl !== null && !isLegacyConfigRetrievalFirstStatement) {
149+
reportError(
150+
result,
151+
legacyConfigDecl,
152+
'Legacy configuration should be retrieved first in theme mixin.',
153+
);
154+
}
155+
}
156+
157+
/** Validates one of the individual theming mixins (`color`, `typography` etc.) */
158+
function validateIndividualSystemMixins(
159+
result: PostcssResult,
160+
node: AtRule,
161+
type: string,
162+
args: string[],
163+
shouldFix: boolean,
164+
) {
165+
if (args.length !== 1) {
166+
reportError(result, node, 'Expected mixin to only declare a single argument.');
167+
} else if (args[0] !== '$config-or-theme') {
168+
if (shouldFix) {
169+
node.params = node.params.replace(args[0], '$config-or-theme');
170+
} else {
171+
reportError(result, node, 'Expected first mixin argument to be called `$config-or-theme`.');
137172
}
173+
}
138174

139-
function validateIndividualSystemMixins(node: AtRule, type: string, args: string[]) {
140-
if (args.length !== 1) {
141-
reportError(node, 'Expected mixin to only declare a single argument.');
142-
} else if (args[0] !== '$config-or-theme') {
143-
if (context.fix) {
144-
node.params = node.params.replace(args[0], '$config-or-theme');
145-
} else {
146-
reportError(node, 'Expected first mixin argument to be called `$config-or-theme`.');
147-
}
148-
}
175+
const expectedProperty = type === 'density' ? '$density-scale' : '$config';
176+
const expectedValues =
177+
type === 'typography'
178+
? [
179+
anyPattern(
180+
'<..>.private-typography-to-2014-config(' +
181+
'<..>.get-typography-config($config-or-theme))',
182+
),
183+
anyPattern(
184+
'<..>.private-typography-to-2018-config(' +
185+
'<..>.get-typography-config($config-or-theme))',
186+
),
187+
]
188+
: [anyPattern(`<..>.get-${type}-config($config-or-theme)`)];
189+
let configExtractionNode: Declaration | null = null;
190+
let nonCommentNodeCount = 0;
149191

150-
const expectedProperty = type === 'density' ? '$density-scale' : '$config';
151-
const expectedValues =
152-
type === 'typography'
153-
? [
154-
anyPattern(
155-
'<..>.private-typography-to-2014-config(' +
156-
'<..>.get-typography-config($config-or-theme))',
157-
),
158-
anyPattern(
159-
'<..>.private-typography-to-2018-config(' +
160-
'<..>.get-typography-config($config-or-theme))',
161-
),
162-
]
163-
: [anyPattern(`<..>.get-${type}-config($config-or-theme)`)];
164-
let configExtractionNode: Declaration | null = null;
165-
let nonCommentNodeCount = 0;
166-
167-
if (node.nodes) {
168-
for (const currentNode of node.nodes) {
169-
if (currentNode.type !== 'comment') {
170-
nonCommentNodeCount++;
171-
}
172-
173-
if (
174-
currentNode.type === 'decl' &&
175-
expectedValues.some(v => v.test(stripNewlinesAndIndentation(currentNode.value)))
176-
) {
177-
configExtractionNode = currentNode;
178-
break;
179-
}
180-
}
192+
if (node.nodes) {
193+
for (const currentNode of node.nodes) {
194+
if (currentNode.type !== 'comment') {
195+
nonCommentNodeCount++;
181196
}
182197

183-
if (!configExtractionNode && nonCommentNodeCount > 0) {
184-
reportError(
185-
node,
186-
`Config is not extracted. Consumers could pass a theme object. ` +
187-
`Extract the configuration by using one of the following:` +
188-
expectedValues.map(expectedValue => `${expectedProperty}: ${expectedValue}`).join('\n'),
189-
);
190-
} else if (configExtractionNode && configExtractionNode.prop !== expectedProperty) {
191-
reportError(
192-
configExtractionNode,
193-
`For consistency, variable for configuration should ` + `be called: ${expectedProperty}`,
194-
);
198+
if (
199+
currentNode.type === 'decl' &&
200+
expectedValues.some(v => v.test(stripNewlinesAndIndentation(currentNode.value)))
201+
) {
202+
configExtractionNode = currentNode;
203+
break;
195204
}
196205
}
206+
}
197207

198-
function reportError(node: Node, message: string) {
199-
// We need these `as any` casts, because Stylelint uses an older version
200-
// of the postcss typings that don't match up with our anymore.
201-
utils.report({result: result as any, ruleName, node: node, message});
202-
}
203-
};
204-
});
208+
if (!configExtractionNode && nonCommentNodeCount > 0) {
209+
reportError(
210+
result,
211+
node,
212+
`Config is not extracted. Consumers could pass a theme object. ` +
213+
`Extract the configuration by using one of the following:` +
214+
expectedValues.map(expectedValue => `${expectedProperty}: ${expectedValue}`).join('\n'),
215+
);
216+
} else if (configExtractionNode && configExtractionNode.prop !== expectedProperty) {
217+
reportError(
218+
result,
219+
configExtractionNode,
220+
`For consistency, variable for configuration should ` + `be called: ${expectedProperty}`,
221+
);
222+
}
223+
}
224+
225+
/** Reports a lint error. */
226+
function reportError(result: PostcssResult, node: Node, message: string) {
227+
utils.report({result, ruleName, node, message});
228+
}
205229

206230
/** Figures out the name of the component from a file path. */
207231
function getComponentNameFromPath(filePath: string): string | null {

0 commit comments

Comments
 (0)