diff --git a/.changeset/silent-spiders-poke.md b/.changeset/silent-spiders-poke.md new file mode 100644 index 00000000000..cc303c94935 --- /dev/null +++ b/.changeset/silent-spiders-poke.md @@ -0,0 +1,5 @@ +--- +'@shopify/polaris-migrator': minor +--- + +Add stylelint API shim for forward migration compatibility diff --git a/polaris-migrator/README.md b/polaris-migrator/README.md index 9d115066cea..b244546a04e 100644 --- a/polaris-migrator/README.md +++ b/polaris-migrator/README.md @@ -235,11 +235,18 @@ Be aware that this may also create additional code changes in your codebase, we npx @shopify/polaris-migrator replace-sass-spacing ``` -## Creating a migration +## Creating Migrations -### Setup +Sometimes referred to as "codemods", migrations are JavaScript functions which modify some code from one form to another (eg; to move between breaking versions of `@shopify/polaris`). ASTs (Abstract Syntax Trees) are used to "walk" through the code in discreet, strongly typed steps, called "nodes". All changes made to nodes (and thus the AST) are then written out as the new/"migrated" version of the code. -Run `yarn new-migration` to generate a new migration from a template. +`polaris-migrator` supports two types of migrations: + +- SASS Migrations +- Typescript Migrations + +### Creating a SASS migration + +Run `yarn new-migration` to generate a new migration from the `sass-migration` template: ```sh ❯ yarn new-migration @@ -250,7 +257,7 @@ $ plop typescript-migration ``` -We will use the `sass-migration` and call our migration `replace-sass-function` for this example. Provide the name of your migration: +Next, provide the name of your migration. For example; `replace-sass-function`: ```sh ? [PLOP] Please choose a generator. sass-migration @@ -269,45 +276,50 @@ migrations └── replace-sass-function.test.ts ``` -### Writing migration function +#### The SASS migration function -A migration is simply a javascript function which serves as the entry-point for your codemod. The `replace-sass-function.ts` file defines a "migration" function. This function is named the same as the provided migration name, `replace-sass-function`, and is the default export of the file. +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. -Some example code has been provided for each template. You can make any migration code adjustments in the migration function. For Sass migrations, a [PostCSS plugin](https://github.com/postcss/postcss/blob/main/docs/writing-a-plugin.md) is used to parse and transform the source code provided by the [jscodeshift](https://github.com/facebook/jscodeshift). +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, + createStylelintRule, + StopWalkingFunctionNodes, +} from '../../utilities/sass'; +import type {PolarisMigrator} from '../../utilities/sass'; + +const replaceHelloWorld: PolarisMigrator = (_, {methods}, context) => { + return (root) => { + methods.walkDecls(root, (decl, parsedValue) => { + 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; + } + }); }); + }; +}; - decl.value = parsedValue.toString(); - }, -}); - -export default function replaceSassFunction(fileInfo: FileInfo) { - return postcss(plugin()).process(fileInfo.source, { - syntax: require('postcss-scss'), - }).css; -} +export default createStylelintRule('replace-hello-world', replaceHelloWorld); ``` -This example migration will replace the Sass function `hello()` with `world()`. +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). -### Testing +#### Testing The template will also generate starting test files you can use to test your migration. In your migrations `tests` folder, you can see 3 files: @@ -317,6 +329,8 @@ The template will also generate starting test files you can use to test your mig The main test file will load the input/output fixtures to test your migration against. You can configure additional fixtures and test migration options (see the `replace-sass-spacing.test.ts` as an example). +## Running Migrations + Run tests locally from workspace root by filtering to the migrations package: ```sh diff --git a/polaris-migrator/src/migrations/replace-spacing-lengths/replace-spacing-lengths.ts b/polaris-migrator/src/migrations/replace-spacing-lengths/replace-spacing-lengths.ts index 1f5ac5ee39e..9252151e4b7 100644 --- a/polaris-migrator/src/migrations/replace-spacing-lengths/replace-spacing-lengths.ts +++ b/polaris-migrator/src/migrations/replace-spacing-lengths/replace-spacing-lengths.ts @@ -1,131 +1,127 @@ -import type {FileInfo, API, Options} from 'jscodeshift'; -import postcss, {Plugin} from 'postcss'; import valueParser from 'postcss-value-parser'; -import {POLARIS_MIGRATOR_COMMENT} from '../../constants'; import { - createInlineComment, getFunctionArgs, isNumericOperator, isSassFunction, isTransformableLength, namespace, - NamespaceOptions, toTransformablePx, StopWalkingFunctionNodes, + createStylelintRule, } from '../../utilities/sass'; import {isKeyOf} from '../../utilities/type-guards'; -export default function replaceSpacingLengths( - fileInfo: FileInfo, - _: API, - options: Options, -) { - return postcss(plugin(options)).process(fileInfo.source, { - syntax: require('postcss-scss'), - }).css; -} +export default createStylelintRule( + 'replace-sass-space', + (_, {methods, options}, context) => { + const namespacedRem = namespace('rem', options); -const processed = Symbol('processed'); + return (root) => { + methods.walkDecls(root, (decl, parsedValue) => { + if (!spaceProps.has(decl.prop)) return; -interface PluginOptions extends Options, NamespaceOptions {} + let hasNumericOperator = false; -const plugin = (options: PluginOptions = {}): Plugin => { - const namespacedRem = namespace('rem', options); + handleSpaceProps(); - return { - postcssPlugin: 'replace-sass-space', - Declaration(decl) { - // @ts-expect-error - Skip if processed so we don't process it again - if (decl[processed]) return; + if (hasNumericOperator) { + methods.report({ + node: decl, + severity: 'warning', + message: 'Numeric operator detected.', + }); + } - 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 { - decl.value = parsedValue.toString(); - } - - // - // Handlers - // - - function handleSpaceProps() { - parsedValue.walk((node) => { - if (isNumericOperator(node)) { - hasNumericOperator = true; - return; - } - - if (node.type === 'word') { - if (globalValues.has(node.value)) return; - - const dimension = valueParser.unit(node.value); - - if (!isTransformableLength(dimension)) return; - - targets.push({replaced: false}); - - const valueInPx = toTransformablePx(node.value); - - if (!isKeyOf(spaceMap, valueInPx)) return; - - targets[targets.length - 1]!.replaced = true; - - node.value = `var(${spaceMap[valueInPx]})`; - return; - } + function handleSpaceProps() { + parsedValue.walk((node) => { + if (isNumericOperator(node)) { + hasNumericOperator = true; + return; + } - if (node.type === 'function') { - if (isSassFunction(namespacedRem, node)) { - targets.push({replaced: false}); + if (node.type === 'word') { + if (globalValues.has(node.value)) return; - const args = getFunctionArgs(node); + const dimension = valueParser.unit(node.value); - if (args.length !== 1) return; + if (!isTransformableLength(dimension)) return; - const valueInPx = toTransformablePx(args[0]); + 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'; - node.nodes = [ - { - type: 'word', - value: spaceMap[valueInPx], - sourceIndex: node.nodes[0]?.sourceIndex ?? 0, - sourceEndIndex: spaceMap[valueInPx].length, - }, - ]; + methods.report({ + node: decl, + severity: 'error', + message: `Prefer var(${spaceMap[valueInPx]}) Polaris token.`, + }); + return; } - return StopWalkingFunctionNodes; - } - }); - } - }, - }; -}; + if (node.type === 'function') { + if (isSassFunction(namespacedRem, node)) { + const args = getFunctionArgs(node); + + 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)) { + 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; + } + }); + } + }); + }; + }, +); const globalValues = new Set(['inherit', 'initial', 'unset']); diff --git a/polaris-migrator/src/migrations/replace-spacing-lengths/tests/replace-spacing-lengths.output.scss b/polaris-migrator/src/migrations/replace-spacing-lengths/tests/replace-spacing-lengths.output.scss index d783142336b..becc4078125 100644 --- a/polaris-migrator/src/migrations/replace-spacing-lengths/tests/replace-spacing-lengths.output.scss +++ b/polaris-migrator/src/migrations/replace-spacing-lengths/tests/replace-spacing-lengths.output.scss @@ -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; @@ -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 @@ -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); } diff --git a/polaris-migrator/src/migrations/replace-spacing-lengths/tests/with-namespace.output.scss b/polaris-migrator/src/migrations/replace-spacing-lengths/tests/with-namespace.output.scss index 6da935f98aa..2b9be603f01 100644 --- a/polaris-migrator/src/migrations/replace-spacing-lengths/tests/with-namespace.output.scss +++ b/polaris-migrator/src/migrations/replace-spacing-lengths/tests/with-namespace.output.scss @@ -17,6 +17,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; @@ -25,55 +27,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 @@ -88,21 +100,26 @@ padding: calc(10rem + var(--p-choice-size, #{legacy-polaris-v8.rem(10px)})); // Comment // polaris-migrator: Unable to migrate the following expression. Please upgrade manually. - // padding: legacy-polaris-v8.rem(10px); + // error: Non-tokenizable value '10px' padding: legacy-polaris-v8.rem(10px); // polaris-migrator: Unable to migrate the following expression. Please upgrade manually. - // padding: legacy-polaris-v8.rem(10px) legacy-polaris-v8.rem(10px); + // error: Non-tokenizable value '10px' padding: legacy-polaris-v8.rem(10px) legacy-polaris-v8.rem(10px); // polaris-migrator: Unable to migrate the following expression. Please upgrade manually. + // error: Non-tokenizable value '10px' // padding: var(--p-space-4) legacy-polaris-v8.rem(10px); padding: legacy-polaris-v8.rem(16px) legacy-polaris-v8.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: legacy-polaris-v8.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: legacy-polaris-v8.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: legacy-polaris-v8.rem($var * var(--p-space-4)); padding: legacy-polaris-v8.rem($var * 16px); } diff --git a/polaris-migrator/src/utilities/sass.ts b/polaris-migrator/src/utilities/sass.ts index 1e40eca0901..e6394c323b2 100644 --- a/polaris-migrator/src/utilities/sass.ts +++ b/polaris-migrator/src/utilities/sass.ts @@ -1,4 +1,14 @@ -import postcss from 'postcss'; +import type {FileInfo, API, Options} from 'jscodeshift'; +import postcss, { + Root, + Plugin, + Container, + Declaration, + Node as PostCSSNode, + Rule as PostCSSRule, + Comment as PostCSSComment, + AtRule, +} from 'postcss'; import valueParser, { Node, ParsedValue, @@ -8,6 +18,8 @@ import valueParser, { import {toPx} from '@shopify/polaris-tokens'; import prettier from 'prettier'; +import {POLARIS_MIGRATOR_COMMENT} from '../constants'; + const defaultNamespace = ''; function getNamespace(options?: NamespaceOptions) { @@ -251,3 +263,394 @@ export function createInlineComment(text: string, options?: {prose?: boolean}) { return comment; } + +interface PluginOptions extends Options, NamespaceOptions { + partialFixStrategy: 'fix' | 'report' | undefined | false; + // Only applies when fixing + __injectReportsAsComments: boolean; +} + +interface Report { + node: PostCSSNode; + severity: 'warning' | 'error' | 'suggestion'; + message: string; +} + +interface PluginContext { + fix: boolean; +} + +// Extracted from stylelint +type StylelintRuleBase

= ( + primaryOption: P, + secondaryOptions: {[key: string]: S}, + context: PluginContext, +) => (root: Root) => void; + +interface StylelintRuleMeta { + url: string; + deprecated?: boolean; + fixable?: boolean; +} + +type StylelintRule

= StylelintRuleBase & { + ruleName: string; + meta?: StylelintRuleMeta; +}; + +export type PolarisMigrator

= ( + primaryOption: P, + secondaryOptions: { + options: {[key: string]: S}; + methods: { + report: (report: Report) => void; + each: ( + root: T, + walker: (decl: PostCSSNode) => false | void, + ) => void; + walk: ( + root: T, + walker: (decl: PostCSSNode) => false | void, + ) => void; + walkComments: ( + root: T, + walker: (comment: PostCSSComment) => false | void, + ) => void; + walkAtRules: ( + root: T, + atRuleWalker: ( + atRule: AtRule, + parsedValue: ParsedValue, + ) => false | void, + ) => void; + walkDecls: ( + root: T, + declWalker: ( + decl: Declaration, + parsedValue: ParsedValue, + ) => false | void, + ) => void; + walkRules: ( + root: T, + ruleWalker: (decl: PostCSSRule) => false | void, + ) => void; + }; + }, + context: PluginContext, +) => (root: Root) => void; + +// Expose a stylelint-like API for creating sass migrators so we can easily +// migrate to that tool in the future. +function convertStylelintRuleToPostcssProcessor(ruleFn: StylelintRule) { + return (fileInfo: FileInfo, _: API, options: Options) => { + const plugin: Plugin = { + postcssPlugin: ruleFn.ruleName, + // PostCSS will rewalk the AST every time a declaration/rule/etc is + // mutated by a plugin. This can be useful in some cases, but in ours we + // only want single-pass behaviour. + // + // This can be avoided in 2 ways: + // + // 1) Flagging each declaration as we pass it, then skipping it on + // subsequent passes. + // 2) Using postcss's Once() plugin callback. + // + // stylelint also uses `Once()`, so we're able to remove this once we've + // migrated: + // https://github.com/stylelint/stylelint/blob/cb425cb/lib/postcssPlugin.js#L22 + Once(root: Root) { + // NOTE: For fullest compatibility with stylelint, we initialise the + // rule here _inside_ the postcss Once function just like stylelint + // does. I _think_ this means multiple passes can be performed without + // rules accidentally retaining scoped variables, etc. + const ruleProcessor = ruleFn( + // Normally, this comes from stylelint config, but for this shim we + // just hard-code it, and instead rely on the "seconary" options + // object for passing through the jscodeshift options. + true, + { + ...options, + partialFixStrategy: 'report', + __injectReportsAsComments: true, + }, + {fix: true}, + ); + + ruleProcessor(root); + }, + }; + + return postcss(plugin).process(fileInfo.source, { + syntax: require('postcss-scss'), + }).css; + }; +} + +export function createStylelintRule(ruleName: string, ruleFn: PolarisMigrator) { + const wrappedRule: StylelintRule = (( + primary, + { + partialFixStrategy = 'fix', + __injectReportsAsComments = false, + ...secondaryOptions + }: PluginOptions, + context, + ) => { + const reports = new Map(); + const suggestions = new Map(); + + const addDedupedReport = (newReport: Report) => { + if (!reports.has(newReport.node)) { + reports.set(newReport.node, []); + } + + const reportsForNode = reports.get(newReport.node)!; + + if ( + reportsForNode.findIndex( + (existingReport) => + existingReport.severity === newReport.severity && + existingReport.message === newReport.message, + ) === -1 + ) { + reportsForNode.push(newReport); + } + }; + + const flushReportsAsComments = () => { + // @ts-expect-error No idea why TS is complaining here + for (const [node, reportsForNode] of reports) { + node.before( + createInlineComment(POLARIS_MIGRATOR_COMMENT, {prose: true}), + ); + + for (const report of reportsForNode) { + node.before( + createInlineComment(`${report.severity}: ${report.message}`, { + prose: true, + }), + ); + } + } + reports.clear(); + + // @ts-expect-error No idea why TS is complaining here + for (const [node, suggestion] of suggestions) { + node.before(createInlineComment(suggestion)); + } + suggestions.clear(); + }; + + // TODO: When moving to style-lint, migrate this to call style-lint's + // stylelint.utils.report() + const flushReportsToStylelint = flushReportsAsComments; + + // PostCSS is missing functionality: It doesn't parse `value`s (declaration + // values / @-rule params), so we have to do that ourselves. This form of + // `createWalker` handles that with a `valueParserKey`. + function createWalker(args: { + valueParserKey: keyof T; + walker: (node: T, parsedValue: ParsedValue) => false | void; + mutableKeys?: (keyof T)[]; + serialiseSuggestion: (node: T) => string; + }): (node: T) => false | void; + + // When there's no `value` key, we don't parse or supply that value to the + // walker. + function createWalker(args: { + walker: (node: T) => false | void; + mutableKeys?: (keyof T)[]; + serialiseSuggestion: (node: T) => string; + }): (node: T) => false | void; + + function createWalker(args: { + valueParserKey?: keyof T; + walker: + | ((node: T) => false | void) + | ((node: T, parsedValue: ParsedValue) => false | void); + mutableKeys?: (keyof T)[]; + serialiseSuggestion: (node: T) => string; + }): (node: T) => false | void { + const { + valueParserKey, + walker, + mutableKeys: incomingMutableKeys = [], + serialiseSuggestion, + } = args; + + const mutableKeys = [...incomingMutableKeys]; + + if ( + typeof valueParserKey !== 'undefined' && + !mutableKeys.includes(valueParserKey) + ) { + mutableKeys.push(valueParserKey); + } + + return (node: T) => { + const oldValues = mutableKeys.reduce>((memo, key) => { + memo[key] = node[key]; + return memo; + }, {}); + + let result: false | void; + if (typeof valueParserKey !== 'undefined') { + const parsedValue = valueParser( + node[valueParserKey] as unknown as string, + ); + result = walker(node, parsedValue); + + if (context.fix && parsedValue) { + (node[valueParserKey] as unknown as string) = + parsedValue.toString(); + } + } else { + result = (walker as (node: T) => false | void)(node); + } + + if (context.fix) { + const newValues = mutableKeys.reduce>((memo, key) => { + memo[key] = node[key]; + return memo; + }, {}); + + const isDirty = mutableKeys.some( + (key) => oldValues[key] !== newValues[key], + ); + + const isPartialFix = isDirty && reports.has(node); + + if (isPartialFix) { + if (partialFixStrategy === 'report') { + // Insert the changes as a suggestion + suggestions.set(node, serialiseSuggestion(node)); + } + + if (partialFixStrategy !== 'fix') { + // Undo changes + mutableKeys.forEach((key) => (node[key] = oldValues[key]!)); + } + } else { + mutableKeys.forEach((key) => (node[key] = newValues[key]!)); + } + } + + if (__injectReportsAsComments) { + flushReportsAsComments(); + } else { + flushReportsToStylelint(); + } + + return result; + }; + } + + function each( + root: T, + walker: (node: PostCSSNode) => false | void, + ) { + root.each( + createWalker({ + walker, + serialiseSuggestion: (node) => node.toString(), + }), + ); + } + + function walk( + root: T, + walker: (node: PostCSSNode) => false | void, + ) { + root.walk( + createWalker({ + walker, + serialiseSuggestion: (node) => node.toString(), + }), + ); + } + + function walkAtRules( + root: T, + walker: (node: AtRule, parsedValue: ParsedValue) => false | void, + ) { + root.walkAtRules( + createWalker({ + walker, + valueParserKey: 'params', + mutableKeys: ['name', 'params'], + serialiseSuggestion: (node) => `@${node.name} ${node.params}`, + }), + ); + } + + function walkComments( + root: T, + walker: (node: PostCSSComment) => false | void, + ) { + root.walkComments( + createWalker({ + walker, + mutableKeys: ['text'], + serialiseSuggestion: (node) => node.text, + }), + ); + } + + function walkDecls( + root: T, + walker: (node: Declaration, parsedValue: ParsedValue) => false | void, + ) { + root.walkDecls( + createWalker({ + walker, + valueParserKey: 'value', + mutableKeys: ['value', 'prop'], + serialiseSuggestion: (node) => `${node.prop}: ${node.value}`, + }), + ); + } + + function walkRules( + root: T, + walker: (node: PostCSSRule) => false | void, + ) { + root.walkRules( + createWalker({ + walker, + mutableKeys: ['selector', 'selectors'], + serialiseSuggestion: (node) => node.selector, + }), + ); + } + + return ruleFn( + primary, + // We're kind of abusing stylelint's types here since the + // SecondaryOptions param can take an arbitrary object. But we need a + // way to pass the methods into the rule function somehow, and this way + // means less Typescript hackery. + { + options: secondaryOptions, + methods: { + report: addDedupedReport, + each, + walk, + walkAtRules, + walkComments, + walkDecls, + walkRules, + }, + }, + context, + ); + }) as StylelintRule; + + wrappedRule.ruleName = ruleName; + wrappedRule.meta = { + // TODO: link directly to the specific rule + url: 'https://www.npmjs.com/package/@shopify/stylelint-polaris', + fixable: true, + }; + + return convertStylelintRuleToPostcssProcessor(wrappedRule); +} diff --git a/polaris-migrator/templates/sass-migration/{{kebabCase migrationName}}/{{kebabCase migrationName}}.ts.hbs b/polaris-migrator/templates/sass-migration/{{kebabCase migrationName}}/{{kebabCase migrationName}}.ts.hbs index 2a718cf5db3..17bb59d17fd 100644 --- a/polaris-migrator/templates/sass-migration/{{kebabCase migrationName}}/{{kebabCase migrationName}}.ts.hbs +++ b/polaris-migrator/templates/sass-migration/{{kebabCase migrationName}}/{{kebabCase migrationName}}.ts.hbs @@ -1,33 +1,48 @@ -import type {FileInfo} from 'jscodeshift'; -import postcss, {Plugin} from 'postcss'; -import valueParser from 'postcss-value-parser'; - -const processed = Symbol('processed'); - -const plugin = (): Plugin => ({ - postcssPlugin: '{{kebabCase migrationName}}', - Declaration(decl) { - // @ts-expect-error - Skip if processed so we don't process it again - if (decl[processed]) return; - - // 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, + createStylelintRule, + StopWalkingFunctionNodes, +} from '../../utilities/sass'; +import type {PolarisMigrator} from '../../utilities/sass'; + +const {{camelCase migrationName}}: PolarisMigrator = ( + _, + // options will be passed in from cli / config. + {methods /* , options */}, + // Use context.fix to change behaviour based on if the user has passed the + // `--fix` flag (always true for `polaris-migrator` CLI). + context, +) => { + return (root) => { + methods.walkDecls(root, (decl, parsedValue) => { + // Using the parsedValue allows easy detection of individual functions and + // properties. Particularly useful when dealing with shorthand + // declarations such as `border`, `padding`, etc. + parsedValue.walk((node) => { + if (isSassFunction('hello', node)) { + if (context.fix) { + // When fixing, we mutate the node directly. + node.value = 'world'; + } else { + // When not fixing, emit a report which will be aggregated and shown + // to the user once all migrations are run. + methods.report({ + node: decl, + severity: 'error', + message: + 'Method hello() is no longer supported. Please migrate to world().', + }); + } + + // We do not want to recursively walk the function's arguments. + return StopWalkingFunctionNodes; + } + }); }); + }; +}; - decl.value = parsedValue.toString(); - - // @ts-expect-error - Mark the declaration as processed - decl[processed] = true; - }, -}); - -export default function {{camelCase migrationName}}(fileInfo: FileInfo) { - return postcss(plugin()).process(fileInfo.source, { - syntax: require('postcss-scss'), - }).css; -} +export default createStylelintRule( + '{{kebabCase migrationName}}', + {{camelCase migrationName}} +); diff --git a/yarn.lock b/yarn.lock index bced73f0988..714c562eefa 100644 --- a/yarn.lock +++ b/yarn.lock @@ -16568,6 +16568,11 @@ obuf@^1.0.0, obuf@^1.1.2: resolved "https://registry.yarnpkg.com/obuf/-/obuf-1.1.2.tgz#09bea3343d41859ebd446292d11c9d4db619084e" integrity sha512-PX1wu0AmAdPqOL1mWhqmlOd8kOIZQwGZw6rh7uby9fTc5lhaOWFLX3I6R1hrF9k3zUY40e6igsLGkDXK92LJNg== +on-change@^4.0.1: + version "4.0.1" + resolved "https://registry.yarnpkg.com/on-change/-/on-change-4.0.1.tgz#a3fea9971748efab3560351c8278ea1a692b7c16" + integrity sha512-Gxmr/8NsLTigoUUEPvnAIUGl7uxwfC3Mm1G+qTmODEZcT4ZGfeaFyQgfGCdbKVS2pICIFYB82RH6MUHToPcS4Q== + on-exit-leak-free@^2.1.0: version "2.1.0" resolved "https://registry.yarnpkg.com/on-exit-leak-free/-/on-exit-leak-free-2.1.0.tgz#5c703c968f7e7f851885f6459bf8a8a57edc9cc4"