Skip to content

Commit 36cdd8c

Browse files
jesstelfordjesstelford
committed
Shim stylelint's .report() util
1 parent 03453e2 commit 36cdd8c

File tree

7 files changed

+264
-78
lines changed

7 files changed

+264
-78
lines changed

‎.changeset/seven-zoos-sin.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@shopify/polaris-migrator': minor
3+
---
4+
5+
Expose the .report() method to SASS migrations for easier aggregation of discovered issues during a migration run.

‎polaris-migrator/README.md‎

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,7 @@ migrations
278278

279279
#### The SASS migration function
280280

281-
Each migrator has a default export adhering to the [PostCSS Plugin API](https://github.com/postcss/postcss/blob/main/docs/writing-a-plugin.md) with one main difference: events are only executed once.
281+
Each migrator has a default export adhering to the [Stylelint Rule API](https://github.com/postcss/postcss/blob/main/docs/writing-a-plugin.md). A PostCSS AST is passed as the `root` and can be mutated inline, or emit warning/error reports.
282282

283283
Continuing the example, here is what the migration may look like if our goal is to replace the Sass function `hello()` with `world()`.
284284

@@ -291,26 +291,37 @@ import {
291291
StopWalkingFunctionNodes,
292292
createSassMigrator,
293293
} from '../../utilities/sass';
294+
import type {PolarisMigrator} from '../../utilities/sass';
294295

295-
// options can be passed in from cli / config.
296-
export default createSassMigrator('replace-sass-function', (_, options, context) => {
296+
const replaceHelloWorld: PolarisMigrator = (_, {methods}, context) => {
297297
return (root) => {
298298
root.walkDecls((decl) => {
299299
const parsedValue = valueParser(decl.value);
300-
301300
parsedValue.walk((node) => {
302301
if (isSassFunction('hello', node)) {
303-
node.value = 'world';
302+
if (context.fix) {
303+
node.value = 'world';
304+
} else {
305+
methods.report({
306+
node: decl,
307+
severity: 'error',
308+
message:
309+
'Method hello() is no longer supported. Please migrate to world().',
310+
});
311+
}
312+
304313
return StopWalkingFunctionNodes;
305314
}
306315
});
307-
308316
if (context.fix) {
309317
decl.value = parsedValue.toString();
310318
}
319+
methods.flushReports();
311320
});
312321
};
313-
});
322+
};
323+
324+
export default createSassMigrator('replace-hello-world', replaceHelloWorld);
314325
```
315326

316327
A more complete example can be seen in [`replace-spacing-lengths.ts`](https://github.com/Shopify/polaris/blob/main/polaris-migrator/src/migrations/replace-spacing-lengths/replace-spacing-lengths.ts).

‎polaris-migrator/src/migrations/replace-spacing-lengths/replace-spacing-lengths.ts‎

Lines changed: 72 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
import valueParser from 'postcss-value-parser';
22

3-
import {POLARIS_MIGRATOR_COMMENT} from '../../constants';
43
import {
5-
createInlineComment,
64
getFunctionArgs,
75
isNumericOperator,
86
isSassFunction,
@@ -16,44 +14,42 @@ import {isKeyOf} from '../../utilities/type-guards';
1614

1715
export default createSassMigrator(
1816
'replace-sass-space',
19-
(_, options, context) => {
17+
(_, {methods, options}, context) => {
2018
const namespacedRem = namespace('rem', options);
2119

2220
return (root) => {
2321
root.walkDecls((decl) => {
2422
if (!spaceProps.has(decl.prop)) return;
2523

26-
/**
27-
* A collection of transformable values to migrate (e.g. decl lengths, functions, etc.)
28-
*
29-
* Note: This is evaluated at the end of each visitor execution to determine whether
30-
* or not to replace the declaration or insert a comment.
31-
*/
32-
const targets: {replaced: boolean}[] = [];
33-
let hasNumericOperator = false;
3424
const parsedValue = valueParser(decl.value);
3525

3626
handleSpaceProps();
3727

38-
if (targets.some(({replaced}) => !replaced || hasNumericOperator)) {
39-
decl.before(
40-
createInlineComment(POLARIS_MIGRATOR_COMMENT, {prose: true}),
41-
);
42-
decl.before(
43-
createInlineComment(`${decl.prop}: ${parsedValue.toString()};`),
44-
);
45-
} else if (context.fix) {
46-
decl.value = parsedValue.toString();
28+
const newValue = parsedValue.toString();
29+
30+
if (context.fix && newValue !== decl.value) {
31+
if (methods.getReportsForNode(decl)) {
32+
// The "partial fix" case: When there's a new value AND a report.
33+
methods.report({
34+
node: decl,
35+
severity: 'suggestion',
36+
message: `${decl.prop}: ${parsedValue.toString()}`,
37+
});
38+
} else {
39+
decl.value = parsedValue.toString();
40+
}
4741
}
4842

49-
//
50-
// Handlers
51-
//
43+
methods.flushReports();
5244

5345
function handleSpaceProps() {
5446
parsedValue.walk((node) => {
5547
if (isNumericOperator(node)) {
56-
hasNumericOperator = true;
48+
methods.report({
49+
node: decl,
50+
severity: 'warning',
51+
message: 'Numeric operator detected.',
52+
});
5753
return;
5854
}
5955

@@ -64,41 +60,72 @@ export default createSassMigrator(
6460

6561
if (!isTransformableLength(dimension)) return;
6662

67-
targets.push({replaced: false});
68-
6963
const valueInPx = toTransformablePx(node.value);
7064

71-
if (!isKeyOf(spaceMap, valueInPx)) return;
65+
if (!isKeyOf(spaceMap, valueInPx)) {
66+
methods.report({
67+
node: decl,
68+
severity: 'error',
69+
message: `Non-tokenizable value '${node.value}'`,
70+
});
71+
return;
72+
}
7273

73-
targets[targets.length - 1]!.replaced = true;
74+
if (context.fix) {
75+
node.value = `var(${spaceMap[valueInPx]})`;
76+
return;
77+
}
7478

75-
node.value = `var(${spaceMap[valueInPx]})`;
79+
methods.report({
80+
node: decl,
81+
severity: 'error',
82+
message: `Prefer var(${spaceMap[valueInPx]}) Polaris token.`,
83+
});
7684
return;
7785
}
7886

7987
if (node.type === 'function') {
8088
if (isSassFunction(namespacedRem, node)) {
81-
targets.push({replaced: false});
82-
8389
const args = getFunctionArgs(node);
8490

85-
if (args.length !== 1) return;
91+
if (args.length !== 1) {
92+
methods.report({
93+
node: decl,
94+
severity: 'error',
95+
message: `Expected 1 argument, got ${args.length}`,
96+
});
97+
return;
98+
}
8699

87100
const valueInPx = toTransformablePx(args[0]);
88101

89-
if (!isKeyOf(spaceMap, valueInPx)) return;
90-
91-
targets[targets.length - 1]!.replaced = true;
92-
93-
node.value = 'var';
94-
node.nodes = [
95-
{
96-
type: 'word',
97-
value: spaceMap[valueInPx],
98-
sourceIndex: node.nodes[0]?.sourceIndex ?? 0,
99-
sourceEndIndex: spaceMap[valueInPx].length,
100-
},
101-
];
102+
if (!isKeyOf(spaceMap, valueInPx)) {
103+
methods.report({
104+
node: decl,
105+
severity: 'error',
106+
message: `Non-tokenizable value '${args[0].trim()}'`,
107+
});
108+
return;
109+
}
110+
111+
if (context.fix) {
112+
node.value = 'var';
113+
node.nodes = [
114+
{
115+
type: 'word',
116+
value: spaceMap[valueInPx],
117+
sourceIndex: node.nodes[0]?.sourceIndex ?? 0,
118+
sourceEndIndex: spaceMap[valueInPx].length,
119+
},
120+
];
121+
return;
122+
}
123+
124+
methods.report({
125+
node: decl,
126+
severity: 'error',
127+
message: `Prefer var(${spaceMap[valueInPx]}) Polaris token.`,
128+
});
102129
}
103130

104131
return StopWalkingFunctionNodes;

‎polaris-migrator/src/migrations/replace-spacing-lengths/tests/replace-spacing-lengths.output.scss‎

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
padding: 0;
1616
padding: 1;
1717
padding: 2;
18+
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
19+
// warning: Numeric operator detected.
1820
padding: #{16px + 16px};
1921
padding: layout-width(nav);
2022
padding: 10em;
@@ -23,55 +25,65 @@
2325
padding: var(--p-space-4, 16px);
2426
// Comment
2527
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
26-
// padding: 10px;
28+
// error: Non-tokenizable value '10px'
2729
padding: 10px;
2830
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
29-
// padding: 10rem;
31+
// error: Non-tokenizable value '10rem'
3032
padding: 10rem;
3133
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
32-
// padding: 10px 10px;
34+
// error: Non-tokenizable value '10px'
3335
padding: 10px 10px;
3436
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
37+
// error: Non-tokenizable value '10px'
3538
// padding: var(--p-space-4) 10px;
3639
padding: 16px 10px;
3740
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
38-
// padding: 10rem 10rem;
41+
// error: Non-tokenizable value '10rem'
3942
padding: 10rem 10rem;
4043
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
44+
// error: Non-tokenizable value '10rem'
4145
// padding: var(--p-space-4) 10rem;
4246
padding: 1rem 10rem;
4347
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
44-
// padding: 10px 10rem;
48+
// error: Non-tokenizable value '10px'
49+
// error: Non-tokenizable value '10rem'
4550
padding: 10px 10rem;
4651
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
52+
// error: Non-tokenizable value '10rem'
4753
// padding: var(--p-space-4) 10rem;
4854
padding: 16px 10rem;
4955
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
56+
// error: Non-tokenizable value '10px'
5057
// padding: 10px var(--p-space-4);
5158
padding: 10px 1rem;
5259
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
60+
// error: Non-tokenizable value '-16px'
5361
// padding: var(--p-space-4) -16px;
5462
padding: 16px -16px;
5563
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
64+
// warning: Numeric operator detected.
5665
// padding: var(--p-space-4) + var(--p-space-4);
5766
padding: 16px + 16px;
5867
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
68+
// warning: Numeric operator detected.
5969
// padding: var(--p-space-4) + var(--p-space-4) var(--p-space-4);
6070
padding: 16px + 16px 16px;
6171
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
72+
// warning: Numeric operator detected.
6273
// padding: $var + var(--p-space-4);
6374
padding: $var + 16px;
6475
padding: calc(16px + 16px);
6576
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
77+
// warning: Numeric operator detected.
6678
// padding: var(--p-space-4) + #{16px + 16px};
6779
padding: 16px + #{16px + 16px};
6880
// Skip negative lengths. Need to discuss replacement strategy e.g.
6981
// calc(-1 * var(--p-space-*)) vs var(--p-space--*)
7082
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
71-
// padding: -16px;
83+
// error: Non-tokenizable value '-16px'
7284
padding: -16px;
7385
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
74-
// padding: -10px;
86+
// error: Non-tokenizable value '-10px'
7587
padding: -10px;
7688

7789
// REM FUNCTION
@@ -86,21 +98,26 @@
8698
padding: calc(10rem + var(--p-choice-size, #{rem(10px)}));
8799
// Comment
88100
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
89-
// padding: rem(10px);
101+
// error: Non-tokenizable value '10px'
90102
padding: rem(10px);
91103
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
92-
// padding: rem(10px) rem(10px);
104+
// error: Non-tokenizable value '10px'
93105
padding: rem(10px) rem(10px);
94106
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
107+
// error: Non-tokenizable value '10px'
95108
// padding: var(--p-space-4) rem(10px);
96109
padding: rem(16px) rem(10px);
97110
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
111+
// error: Non-tokenizable value '-16px'
98112
// padding: var(--p-space-4) -16px;
99113
padding: rem(16px) -16px;
100114
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
115+
// warning: Numeric operator detected.
101116
// padding: var(--p-space-4) + var(--p-space-4);
102117
padding: rem(16px) + 16px;
103118
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
119+
// error: Non-tokenizable value '$var \* 16px'
120+
// warning: Numeric operator detected.
104121
// padding: rem($var * var(--p-space-4));
105122
padding: rem($var * 16px);
106123
}

0 commit comments

Comments
 (0)