Skip to content

Commit

Permalink
Shim stylelint's .report() util
Browse files Browse the repository at this point in the history
  • Loading branch information
@jesstelford
jesstelford committed Oct 28, 2022
1 parent fa708cb commit 69e4e0b
Show file tree
Hide file tree
Showing 7 changed files with 283 additions and 96 deletions.
5 changes: 5 additions & 0 deletions .changeset/seven-zoos-sin.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/polaris-migrator': minor
---

Expose the .report() method to SASS migrations for easier aggregation of discovered issues during a migration run.
62 changes: 37 additions & 25 deletions polaris-migrator/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -278,38 +278,50 @@ migrations

#### The SASS migration function

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.
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.

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

```ts
// polaris-migrator/src/migrations/replace-sass-function/replace-sass-function.ts

import type {FileInfo} from 'jscodeshift';
import postcss, {Plugin} from 'postcss';
import valueParser from 'postcss-value-parser';

const plugin = (): Plugin => ({
postcssPlugin: 'replace-sass-function',
Declaration(decl) {
// const prop = decl.prop;
const parsedValue = valueParser(decl.value);

parsedValue.walk((node) => {
if (!(node.type === 'function' && node.value === 'hello')) return;

node.value = 'world';
import {
isSassFunction,
StopWalkingFunctionNodes,
createSassMigrator,
} from '../../utilities/sass';
import type {PolarisMigrator} from '../../utilities/sass';

const replaceHelloWorld: PolarisMigrator = (_, {methods}, context) => {
return (root) => {
root.walkDecls((decl) => {
const parsedValue = valueParser(decl.value);
parsedValue.walk((node) => {
if (isSassFunction('hello', node)) {
if (context.fix) {
node.value = 'world';
} else {
methods.report({
node: decl,
severity: 'error',
message:
'Method hello() is no longer supported. Please migrate to world().',
});
}

return StopWalkingFunctionNodes;
}
});

if (context.fix) {
decl.value = parsedValue.toString();
}

methods.flushReports();
});
};
};

decl.value = parsedValue.toString();
},
});

export default function replaceSassFunction(fileInfo: FileInfo) {
return postcss(plugin()).process(fileInfo.source, {
syntax: require('postcss-scss'),
}).css;
}
export default createSassMigrator('replace-hello-world', replaceHelloWorld);
```

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).
Expand Down
117 changes: 72 additions & 45 deletions polaris-migrator/src/migrations/replace-spacing-lengths/replace-spacing-lengths.ts
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import valueParser from 'postcss-value-parser';

import {POLARIS_MIGRATOR_COMMENT} from '../../constants';
import {
createInlineComment,
getFunctionArgs,
isNumericOperator,
isSassFunction,
Expand All @@ -16,44 +14,42 @@ import {isKeyOf} from '../../utilities/type-guards';

export default createSassMigrator(
'replace-sass-space',
(_, options, context) => {
(_, {methods, options}, context) => {
const namespacedRem = namespace('rem', options);

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

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

handleSpaceProps();

if (targets.some(({replaced}) => !replaced || hasNumericOperator)) {
decl.before(
createInlineComment(POLARIS_MIGRATOR_COMMENT, {prose: true}),
);
decl.before(
createInlineComment(`${decl.prop}: ${parsedValue.toString()};`),
);
} else if (context.fix) {
decl.value = parsedValue.toString();
const newValue = parsedValue.toString();

if (context.fix && newValue !== decl.value) {
if (methods.getReportsForNode(decl)) {
// The "partial fix" case: When there's a new value AND a report.
methods.report({
node: decl,
severity: 'suggestion',
message: `${decl.prop}: ${parsedValue.toString()}`,
});
} else {
decl.value = parsedValue.toString();
}
}

//
// Handlers
//
methods.flushReports();

function handleSpaceProps() {
parsedValue.walk((node) => {
if (isNumericOperator(node)) {
hasNumericOperator = true;
methods.report({
node: decl,
severity: 'warning',
message: 'Numeric operator detected.',
});
return;
}

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

if (!isTransformableLength(dimension)) return;

targets.push({replaced: false});

const valueInPx = toTransformablePx(node.value);

if (!isKeyOf(spaceMap, valueInPx)) return;
if (!isKeyOf(spaceMap, valueInPx)) {
methods.report({
node: decl,
severity: 'error',
message: `Non-tokenizable value '${node.value}'`,
});
return;
}

targets[targets.length - 1]!.replaced = true;
if (context.fix) {
node.value = `var(${spaceMap[valueInPx]})`;
return;
}

node.value = `var(${spaceMap[valueInPx]})`;
methods.report({
node: decl,
severity: 'error',
message: `Prefer var(${spaceMap[valueInPx]}) Polaris token.`,
});
return;
}

if (node.type === 'function') {
if (isSassFunction(namespacedRem, node)) {
targets.push({replaced: false});

const args = getFunctionArgs(node);

if (args.length !== 1) return;
if (args.length !== 1) {
methods.report({
node: decl,
severity: 'error',
message: `Expected 1 argument, got ${args.length}`,
});
return;
}

const valueInPx = toTransformablePx(args[0]);

if (!isKeyOf(spaceMap, valueInPx)) return;

targets[targets.length - 1]!.replaced = true;

node.value = 'var';
node.nodes = [
{
type: 'word',
value: spaceMap[valueInPx],
sourceIndex: node.nodes[0]?.sourceIndex ?? 0,
sourceEndIndex: spaceMap[valueInPx].length,
},
];
if (!isKeyOf(spaceMap, valueInPx)) {
methods.report({
node: decl,
severity: 'error',
message: `Non-tokenizable value '${args[0].trim()}'`,
});
return;
}

if (context.fix) {
node.value = 'var';
node.nodes = [
{
type: 'word',
value: spaceMap[valueInPx],
sourceIndex: node.nodes[0]?.sourceIndex ?? 0,
sourceEndIndex: spaceMap[valueInPx].length,
},
];
return;
}

methods.report({
node: decl,
severity: 'error',
message: `Prefer var(${spaceMap[valueInPx]}) Polaris token.`,
});
}

return StopWalkingFunctionNodes;
Expand Down
35 changes: 26 additions & 9 deletions ...migrator/src/migrations/replace-spacing-lengths/tests/replace-spacing-lengths.output.scss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
padding: 0;
padding: 1;
padding: 2;
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
// warning: Numeric operator detected.
padding: #{16px + 16px};
padding: layout-width(nav);
padding: 10em;
Expand All @@ -23,55 +25,65 @@
padding: var(--p-space-4, 16px);
// Comment
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
// padding: 10px;
// error: Non-tokenizable value '10px'
padding: 10px;
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
// padding: 10rem;
// error: Non-tokenizable value '10rem'
padding: 10rem;
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
// padding: 10px 10px;
// error: Non-tokenizable value '10px'
padding: 10px 10px;
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
// error: Non-tokenizable value '10px'
// padding: var(--p-space-4) 10px;
padding: 16px 10px;
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
// padding: 10rem 10rem;
// error: Non-tokenizable value '10rem'
padding: 10rem 10rem;
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
// error: Non-tokenizable value '10rem'
// padding: var(--p-space-4) 10rem;
padding: 1rem 10rem;
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
// padding: 10px 10rem;
// error: Non-tokenizable value '10px'
// error: Non-tokenizable value '10rem'
padding: 10px 10rem;
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
// error: Non-tokenizable value '10rem'
// padding: var(--p-space-4) 10rem;
padding: 16px 10rem;
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
// error: Non-tokenizable value '10px'
// padding: 10px var(--p-space-4);
padding: 10px 1rem;
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
// error: Non-tokenizable value '-16px'
// padding: var(--p-space-4) -16px;
padding: 16px -16px;
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
// warning: Numeric operator detected.
// padding: var(--p-space-4) + var(--p-space-4);
padding: 16px + 16px;
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
// warning: Numeric operator detected.
// padding: var(--p-space-4) + var(--p-space-4) var(--p-space-4);
padding: 16px + 16px 16px;
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
// warning: Numeric operator detected.
// padding: $var + var(--p-space-4);
padding: $var + 16px;
padding: calc(16px + 16px);
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
// warning: Numeric operator detected.
// padding: var(--p-space-4) + #{16px + 16px};
padding: 16px + #{16px + 16px};
// Skip negative lengths. Need to discuss replacement strategy e.g.
// calc(-1 * var(--p-space-*)) vs var(--p-space--*)
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
// padding: -16px;
// error: Non-tokenizable value '-16px'
padding: -16px;
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
// padding: -10px;
// error: Non-tokenizable value '-10px'
padding: -10px;

// REM FUNCTION
Expand All @@ -86,21 +98,26 @@
padding: calc(10rem + var(--p-choice-size, #{rem(10px)}));
// Comment
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
// padding: rem(10px);
// error: Non-tokenizable value '10px'
padding: rem(10px);
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
// padding: rem(10px) rem(10px);
// error: Non-tokenizable value '10px'
padding: rem(10px) rem(10px);
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
// error: Non-tokenizable value '10px'
// padding: var(--p-space-4) rem(10px);
padding: rem(16px) rem(10px);
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
// error: Non-tokenizable value '-16px'
// padding: var(--p-space-4) -16px;
padding: rem(16px) -16px;
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
// warning: Numeric operator detected.
// padding: var(--p-space-4) + var(--p-space-4);
padding: rem(16px) + 16px;
// polaris-migrator: Unable to migrate the following expression. Please upgrade manually.
// error: Non-tokenizable value '$var \* 16px'
// warning: Numeric operator detected.
// padding: rem($var * var(--p-space-4));
padding: rem($var * 16px);
}
Loading

0 comments on commit 69e4e0b

Please sign in to comment.