From 07af2a1f33e20fea9cb759d817f503ef46e38a8f Mon Sep 17 00:00:00 2001 From: Laura Griffee Date: Wed, 21 Sep 2022 10:52:41 -0600 Subject: [PATCH 01/16] Create initial script scaffolding --- .../replace-sass-padding.ts | 63 +++++++++++++++++++ .../tests/replace-sass-padding.input.scss | 3 + .../tests/replace-sass-padding.output.scss | 3 + .../tests/replace-sass-padding.test.ts | 12 ++++ 4 files changed, 81 insertions(+) create mode 100644 polaris-migrator/src/migrations/replace-sass-padding/replace-sass-padding.ts create mode 100644 polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.input.scss create mode 100644 polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.output.scss create mode 100644 polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.test.ts diff --git a/polaris-migrator/src/migrations/replace-sass-padding/replace-sass-padding.ts b/polaris-migrator/src/migrations/replace-sass-padding/replace-sass-padding.ts new file mode 100644 index 00000000000..630d96f9a1f --- /dev/null +++ b/polaris-migrator/src/migrations/replace-sass-padding/replace-sass-padding.ts @@ -0,0 +1,63 @@ +import type {FileInfo} from 'jscodeshift'; +import postcss, {Plugin} from 'postcss'; +import valueParser from 'postcss-value-parser'; + +// Mapping of spacing tokens and their corresponding px values +// const spacingTokensMap = { +// '0': '--p-space-0', +// '1px': '--p-space-025', +// '2px': '--p-space-05', +// '4px': '--p-space-1', +// '8px': '--p-space-2', +// '12px': '--p-space-3', +// '16px': '--p-space-4', +// '20px': '--p-space-5', +// '24px': '--p-space-6', +// '32px': '--p-space-8', +// '40px': '--p-space-10', +// '48px': '--p-space-12', +// '64px': '--p-space-16', +// '80px': '--p-space-20', +// '96px': '--p-space-24', +// '112px': '--p-space-28', +// '128px': '--p-space-32', +// }; + +// List of the props we want to run this migration on +const targetProps = [ + 'padding', + 'padding-top', + 'padding-right', + 'padding-bottom', + 'padding-left', +]; + +const isTargetProp = (propName: string): boolean => + targetProps.includes(propName); + +const plugin = (): Plugin => ({ + postcssPlugin: 'replace-sass-padding', + Declaration(decl) { + const prop = decl.prop; + const parsedValue = valueParser(decl.value); + + // Return if not a padding declaration + if (!isTargetProp(prop)) return; + console.log(`${prop}: ${parsedValue}`); + + // parsedValue.walk((node) => { + + // // If so then check if the value matches the new spacing token values + + // // Repalce with new spacing token value if there is a match + // }); + + decl.value = parsedValue.toString(); + }, +}); + +export default function replaceSassPadding(fileInfo: FileInfo) { + return postcss(plugin()).process(fileInfo.source, { + syntax: require('postcss-scss'), + }).css; +} diff --git a/polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.input.scss b/polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.input.scss new file mode 100644 index 00000000000..160f204f870 --- /dev/null +++ b/polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.input.scss @@ -0,0 +1,3 @@ +.my-class { + color: hello(); +} diff --git a/polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.output.scss b/polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.output.scss new file mode 100644 index 00000000000..b58db76a7b3 --- /dev/null +++ b/polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.output.scss @@ -0,0 +1,3 @@ +.my-class { + color: world(); +} diff --git a/polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.test.ts b/polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.test.ts new file mode 100644 index 00000000000..2bf9f40a713 --- /dev/null +++ b/polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.test.ts @@ -0,0 +1,12 @@ +import {check} from '../../../utilities/testUtils'; + +const migration = 'replace-sass-padding'; +const fixtures = ['replace-sass-padding']; + +for (const fixture of fixtures) { + check(__dirname, { + fixture, + migration, + extension: 'scss', + }); +} From 6c9d7755890d4fb6c878782e15fabce95dbb5a39 Mon Sep 17 00:00:00 2001 From: Laura Griffee Date: Wed, 21 Sep 2022 16:22:39 -0600 Subject: [PATCH 02/16] Add migration logic for dimension values --- .../replace-sass-padding.ts | 79 +++++++++++++------ .../tests/replace-sass-padding.input.scss | 11 ++- .../tests/replace-sass-padding.output.scss | 11 ++- 3 files changed, 73 insertions(+), 28 deletions(-) diff --git a/polaris-migrator/src/migrations/replace-sass-padding/replace-sass-padding.ts b/polaris-migrator/src/migrations/replace-sass-padding/replace-sass-padding.ts index 630d96f9a1f..ef88a63c772 100644 --- a/polaris-migrator/src/migrations/replace-sass-padding/replace-sass-padding.ts +++ b/polaris-migrator/src/migrations/replace-sass-padding/replace-sass-padding.ts @@ -3,25 +3,25 @@ import postcss, {Plugin} from 'postcss'; import valueParser from 'postcss-value-parser'; // Mapping of spacing tokens and their corresponding px values -// const spacingTokensMap = { -// '0': '--p-space-0', -// '1px': '--p-space-025', -// '2px': '--p-space-05', -// '4px': '--p-space-1', -// '8px': '--p-space-2', -// '12px': '--p-space-3', -// '16px': '--p-space-4', -// '20px': '--p-space-5', -// '24px': '--p-space-6', -// '32px': '--p-space-8', -// '40px': '--p-space-10', -// '48px': '--p-space-12', -// '64px': '--p-space-16', -// '80px': '--p-space-20', -// '96px': '--p-space-24', -// '112px': '--p-space-28', -// '128px': '--p-space-32', -// }; +const spacingTokensMap = { + '0': '--p-space-0', + '1px': '--p-space-025', + '2px': '--p-space-05', + '4px': '--p-space-1', + '8px': '--p-space-2', + '12px': '--p-space-3', + '16px': '--p-space-4', + '20px': '--p-space-5', + '24px': '--p-space-6', + '32px': '--p-space-8', + '40px': '--p-space-10', + '48px': '--p-space-12', + '64px': '--p-space-16', + '80px': '--p-space-20', + '96px': '--p-space-24', + '112px': '--p-space-28', + '128px': '--p-space-32', +}; // List of the props we want to run this migration on const targetProps = [ @@ -35,22 +35,49 @@ const targetProps = [ const isTargetProp = (propName: string): boolean => targetProps.includes(propName); +const isSpacingTokenValue = ( + value: string, +): value is keyof typeof spacingTokensMap => + Object.keys(spacingTokensMap).includes(value); + const plugin = (): Plugin => ({ postcssPlugin: 'replace-sass-padding', Declaration(decl) { const prop = decl.prop; const parsedValue = valueParser(decl.value); - // Return if not a padding declaration if (!isTargetProp(prop)) return; - console.log(`${prop}: ${parsedValue}`); - - // parsedValue.walk((node) => { - // // If so then check if the value matches the new spacing token values + parsedValue.walk((node) => { + const dimension = valueParser.unit(node.value); - // // Repalce with new spacing token value if there is a match - // }); + if (node.type === 'word' && dimension) { + switch (dimension.unit) { + case '': { + if (!isSpacingTokenValue(node.value)) return; + // Does this correctly reassign the value or do we need to use decl.assign? + node.value = `var(${spacingTokensMap[node.value]})`; + break; + } + case 'px': { + if (!isSpacingTokenValue(node.value)) return; + node.value = `var(${spacingTokensMap[node.value]})`; + break; + } + case 'rem': + case 'em': { + const pxNumber = parseFloat(dimension.number) * 16; + const pxDimension = `${pxNumber}px`; + if (!isSpacingTokenValue(pxDimension)) return; + node.value = `var(${spacingTokensMap[pxDimension]})`; + break; + } + default: { + console.log(`Unit ${dimension.unit} is not supported`); + } + } + } + }); decl.value = parsedValue.toString(); }, diff --git a/polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.input.scss b/polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.input.scss index 160f204f870..eea55a76fc6 100644 --- a/polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.input.scss +++ b/polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.input.scss @@ -1,3 +1,12 @@ .my-class { - color: hello(); + color: red; + padding-top: 13px; + padding: 0 1rem 2em 3px 4in; + // padding-bottom: var(--p-space-0); + // padding-right: rem(2px); +} + +.another-class { + padding-bottom: 0.5rem; + padding-top: 1.25rem; } diff --git a/polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.output.scss b/polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.output.scss index b58db76a7b3..2063247edfa 100644 --- a/polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.output.scss +++ b/polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.output.scss @@ -1,3 +1,12 @@ .my-class { - color: world(); + color: red; + padding-top: 13px; + padding: var(--p-space-0) var(--p-space-4) var(--p-space-8) 3px 4in; + // padding-bottom: var(--p-space-0); + // padding-right: rem(2px); +} + +.another-class { + padding-bottom: var(--p-space-2); + padding-top: var(--p-space-5); } From fe30bb22f9966a1cce2b504b8f207920430cb558 Mon Sep 17 00:00:00 2001 From: Laura Griffee Date: Wed, 21 Sep 2022 17:18:28 -0600 Subject: [PATCH 03/16] Start building logic for handling functions and variables --- .../replace-sass-padding.ts | 83 +++++++++++++++---- .../tests/replace-sass-padding.input.scss | 5 +- .../tests/replace-sass-padding.output.scss | 7 +- 3 files changed, 76 insertions(+), 19 deletions(-) diff --git a/polaris-migrator/src/migrations/replace-sass-padding/replace-sass-padding.ts b/polaris-migrator/src/migrations/replace-sass-padding/replace-sass-padding.ts index ef88a63c772..afe7377d1cc 100644 --- a/polaris-migrator/src/migrations/replace-sass-padding/replace-sass-padding.ts +++ b/polaris-migrator/src/migrations/replace-sass-padding/replace-sass-padding.ts @@ -1,6 +1,20 @@ import type {FileInfo} from 'jscodeshift'; import postcss, {Plugin} from 'postcss'; -import valueParser from 'postcss-value-parser'; +import valueParser, {Node} from 'postcss-value-parser'; + +import {POLARIS_MIGRATOR_COMMENT} from '../../constants'; + +// List of the props we want to run this migration on +const targetProps = [ + 'padding', + 'padding-top', + 'padding-right', + 'padding-bottom', + 'padding-left', +]; + +const isTargetProp = (propName: string): boolean => + targetProps.includes(propName); // Mapping of spacing tokens and their corresponding px values const spacingTokensMap = { @@ -23,35 +37,42 @@ const spacingTokensMap = { '128px': '--p-space-32', }; -// List of the props we want to run this migration on -const targetProps = [ - 'padding', - 'padding-top', - 'padding-right', - 'padding-bottom', - 'padding-left', -]; - -const isTargetProp = (propName: string): boolean => - targetProps.includes(propName); - const isSpacingTokenValue = ( value: string, ): value is keyof typeof spacingTokensMap => Object.keys(spacingTokensMap).includes(value); +const processed = Symbol('processed'); + +function isNumericOperator(node: Node): boolean { + return ( + node.value === '+' || + node.value === '-' || + node.value === '*' || + node.value === '/' || + node.value === '%' + ); +} + const plugin = (): Plugin => ({ postcssPlugin: 'replace-sass-padding', 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); if (!isTargetProp(prop)) return; + let containsCalculation = false; + parsedValue.walk((node) => { const dimension = valueParser.unit(node.value); - if (node.type === 'word' && dimension) { + if (isNumericOperator(node)) containsCalculation = true; + + if (node.type === 'word' && dimension && !containsCalculation) { switch (dimension.unit) { case '': { if (!isSpacingTokenValue(node.value)) return; @@ -77,9 +98,41 @@ const plugin = (): Plugin => ({ } } } + if (node.type === 'function') { + const funcType = node.value; + const funcValue = node.nodes[0]?.value ?? ''; + + switch (funcType) { + case 'calc': { + containsCalculation = true; + break; + } + case 'var': { + break; + } + case 'rem': { + // TODO: create logic to prevent innards from being converted + break; + } + default: { + console.log(`Function ${funcType} is not supported`); + } + } + } }); - decl.value = parsedValue.toString(); + if (containsCalculation) { + // Insert comment if the declaration value contains calculations + decl.before(postcss.comment({text: POLARIS_MIGRATOR_COMMENT})); + decl.before( + postcss.comment({text: `${decl.prop}: ${parsedValue.toString()};`}), + ); + } else { + decl.value = parsedValue.toString(); + } + + // @ts-expect-error - Mark the declaration as processed + decl[processed] = true; }, }); diff --git a/polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.input.scss b/polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.input.scss index eea55a76fc6..51ce4682f3b 100644 --- a/polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.input.scss +++ b/polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.input.scss @@ -2,11 +2,12 @@ color: red; padding-top: 13px; padding: 0 1rem 2em 3px 4in; - // padding-bottom: var(--p-space-0); - // padding-right: rem(2px); + padding-bottom: var(--p-space-0); + padding-right: rem(2px); } .another-class { padding-bottom: 0.5rem; padding-top: 1.25rem; + padding-right: calc(2px + 2px); } diff --git a/polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.output.scss b/polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.output.scss index 2063247edfa..390c7061119 100644 --- a/polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.output.scss +++ b/polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.output.scss @@ -2,11 +2,14 @@ color: red; padding-top: 13px; padding: var(--p-space-0) var(--p-space-4) var(--p-space-8) 3px 4in; - // padding-bottom: var(--p-space-0); - // padding-right: rem(2px); + padding-bottom: var(--p-space-0); + padding-right: rem(2px); } .another-class { padding-bottom: var(--p-space-2); padding-top: var(--p-space-5); + /* polaris-migrator: This is a complex expression that we can't automatically convert. Please check this manually. */ + /* padding-right: calc(2px + 2px); */ + padding-right: calc(2px + 2px); } From df844f7a24e7c49644656a9606539ae8c4a3a989 Mon Sep 17 00:00:00 2001 From: Laura Griffee Date: Thu, 22 Sep 2022 10:50:14 -0600 Subject: [PATCH 04/16] Start building out logic for handling rem functions --- .../replace-sass-padding.ts | 21 ++++++++++++++++++- .../tests/replace-sass-padding.output.scss | 2 +- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/polaris-migrator/src/migrations/replace-sass-padding/replace-sass-padding.ts b/polaris-migrator/src/migrations/replace-sass-padding/replace-sass-padding.ts index afe7377d1cc..c175fba59f4 100644 --- a/polaris-migrator/src/migrations/replace-sass-padding/replace-sass-padding.ts +++ b/polaris-migrator/src/migrations/replace-sass-padding/replace-sass-padding.ts @@ -101,6 +101,7 @@ const plugin = (): Plugin => ({ if (node.type === 'function') { const funcType = node.value; const funcValue = node.nodes[0]?.value ?? ''; + const funcDimension = valueParser.unit(funcValue); switch (funcType) { case 'calc': { @@ -111,7 +112,25 @@ const plugin = (): Plugin => ({ break; } case 'rem': { - // TODO: create logic to prevent innards from being converted + console.log(`funcValue: ${funcValue}`); + console.log(`funcDimension: ${Object.entries(funcDimension)}`); + + if (funcDimension && !containsCalculation) { + if (!isSpacingTokenValue(funcDimension.number)) return; + + const spacingToken = spacingTokensMap[funcDimension.number]; + + node.value = 'var'; + node.nodes = [ + { + type: 'word', + value: spacingToken, + sourceIndex: node.nodes[0]?.sourceIndex ?? 0, + sourceEndIndex: spacingToken.length, + }, + ...node.nodes.slice(1), + ]; + } break; } default: { diff --git a/polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.output.scss b/polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.output.scss index 390c7061119..65e5a2df43b 100644 --- a/polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.output.scss +++ b/polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.output.scss @@ -3,7 +3,7 @@ padding-top: 13px; padding: var(--p-space-0) var(--p-space-4) var(--p-space-8) 3px 4in; padding-bottom: var(--p-space-0); - padding-right: rem(2px); + padding-right: var(--p-space-05); } .another-class { From 447b2b42eed3a2b2b3af75e31625fe2edef74d64 Mon Sep 17 00:00:00 2001 From: Laura Griffee Date: Thu, 22 Sep 2022 17:00:33 -0600 Subject: [PATCH 05/16] Rearchitect migration Co-Authored-By: Aaron Casanova <32409546+aaronccasanova@users.noreply.github.com> --- polaris-migrator/package.json | 1 + .../replace-sass-padding.ts | 166 +++++++++--------- .../tests/replace-sass-padding.input.scss | 2 +- .../tests/replace-sass-padding.output.scss | 6 +- 4 files changed, 84 insertions(+), 91 deletions(-) diff --git a/polaris-migrator/package.json b/polaris-migrator/package.json index 6c793962f72..4a9a24fd71c 100644 --- a/polaris-migrator/package.json +++ b/polaris-migrator/package.json @@ -33,6 +33,7 @@ "generate": "plop" }, "dependencies": { + "@shopify/polaris-tokens": "^6.0.0", "chalk": "^4.1.0", "globby": "11.0.1", "is-git-clean": "^1.1.0", diff --git a/polaris-migrator/src/migrations/replace-sass-padding/replace-sass-padding.ts b/polaris-migrator/src/migrations/replace-sass-padding/replace-sass-padding.ts index c175fba59f4..709bb0d7bfb 100644 --- a/polaris-migrator/src/migrations/replace-sass-padding/replace-sass-padding.ts +++ b/polaris-migrator/src/migrations/replace-sass-padding/replace-sass-padding.ts @@ -1,6 +1,7 @@ import type {FileInfo} from 'jscodeshift'; import postcss, {Plugin} from 'postcss'; import valueParser, {Node} from 'postcss-value-parser'; +import {toPx} from '@shopify/polaris-tokens'; import {POLARIS_MIGRATOR_COMMENT} from '../../constants'; @@ -11,14 +12,17 @@ const targetProps = [ 'padding-right', 'padding-bottom', 'padding-left', -]; +] as const; -const isTargetProp = (propName: string): boolean => - targetProps.includes(propName); +type TargetProp = typeof targetProps[number]; + +const isTargetProp = (propName: unknown): propName is TargetProp => + targetProps.includes(propName as TargetProp); // Mapping of spacing tokens and their corresponding px values const spacingTokensMap = { '0': '--p-space-0', + '0px': '--p-space-0', '1px': '--p-space-025', '2px': '--p-space-05', '4px': '--p-space-1', @@ -35,24 +39,27 @@ const spacingTokensMap = { '96px': '--p-space-24', '112px': '--p-space-28', '128px': '--p-space-32', -}; +} as const; + +type SpacingToken = keyof typeof spacingTokensMap; -const isSpacingTokenValue = ( - value: string, -): value is keyof typeof spacingTokensMap => - Object.keys(spacingTokensMap).includes(value); +const isSpacingTokenValue = (value: unknown): value is SpacingToken => + Object.keys(spacingTokensMap).includes(value as SpacingToken); + +/** + * All supported dimension units. These values are used to determine + * if a decl.value can be converted to pixels and mapped to a Polaris custom property. + * Note: The empty string is used to match the `0` value + */ +const supportedDimensionUnits = ['px', 'rem', '']; const processed = Symbol('processed'); -function isNumericOperator(node: Node): boolean { - return ( - node.value === '+' || - node.value === '-' || - node.value === '*' || - node.value === '/' || - node.value === '%' - ); -} +/** + * Exit early and stop traversing descendant nodes: + * https://www.npmjs.com/package/postcss-value-parser:~:text=Returning%20false%20in%20the%20callback%20will%20prevent%20traversal%20of%20descendent%20nodes + */ +const ExitAndStopTraversing = false; const plugin = (): Plugin => ({ postcssPlugin: 'replace-sass-padding', @@ -65,82 +72,47 @@ const plugin = (): Plugin => ({ if (!isTargetProp(prop)) return; - let containsCalculation = false; - parsedValue.walk((node) => { - const dimension = valueParser.unit(node.value); - - if (isNumericOperator(node)) containsCalculation = true; - - if (node.type === 'word' && dimension && !containsCalculation) { - switch (dimension.unit) { - case '': { - if (!isSpacingTokenValue(node.value)) return; - // Does this correctly reassign the value or do we need to use decl.assign? - node.value = `var(${spacingTokensMap[node.value]})`; - break; - } - case 'px': { - if (!isSpacingTokenValue(node.value)) return; - node.value = `var(${spacingTokensMap[node.value]})`; - break; - } - case 'rem': - case 'em': { - const pxNumber = parseFloat(dimension.number) * 16; - const pxDimension = `${pxNumber}px`; - if (!isSpacingTokenValue(pxDimension)) return; - node.value = `var(${spacingTokensMap[pxDimension]})`; - break; - } - default: { - console.log(`Unit ${dimension.unit} is not supported`); - } + if (node.type === 'function' && node.value === 'rem') { + const argDimension = valueParser.unit(node.nodes[0]?.value ?? ''); + + if ( + argDimension && + supportedDimensionUnits.includes(argDimension.unit) + ) { + const argInPx = toPx(`${argDimension.number}${argDimension.unit}`); + + if (!isSpacingTokenValue(argInPx)) return ExitAndStopTraversing; + + const spacingToken = spacingTokensMap[argInPx]; + + node.value = 'var'; + node.nodes = [ + { + type: 'word', + value: spacingToken, + sourceIndex: node.nodes[0]?.sourceIndex ?? 0, + sourceEndIndex: spacingToken.length, + }, + ...node.nodes.slice(1), + ]; } - } - if (node.type === 'function') { - const funcType = node.value; - const funcValue = node.nodes[0]?.value ?? ''; - const funcDimension = valueParser.unit(funcValue); - - switch (funcType) { - case 'calc': { - containsCalculation = true; - break; - } - case 'var': { - break; - } - case 'rem': { - console.log(`funcValue: ${funcValue}`); - console.log(`funcDimension: ${Object.entries(funcDimension)}`); - - if (funcDimension && !containsCalculation) { - if (!isSpacingTokenValue(funcDimension.number)) return; - - const spacingToken = spacingTokensMap[funcDimension.number]; - - node.value = 'var'; - node.nodes = [ - { - type: 'word', - value: spacingToken, - sourceIndex: node.nodes[0]?.sourceIndex ?? 0, - sourceEndIndex: spacingToken.length, - }, - ...node.nodes.slice(1), - ]; - } - break; - } - default: { - console.log(`Function ${funcType} is not supported`); - } + + return ExitAndStopTraversing; + } else if (node.type === 'word') { + const dimension = valueParser.unit(node.value); + + if (dimension && supportedDimensionUnits.includes(dimension.unit)) { + const dimensionInPx = toPx(`${dimension.number}${dimension.unit}`); + + if (!isSpacingTokenValue(dimensionInPx)) return; + + node.value = `var(${spacingTokensMap[dimensionInPx]})`; } } }); - if (containsCalculation) { + if (hasCalculation(parsedValue)) { // Insert comment if the declaration value contains calculations decl.before(postcss.comment({text: POLARIS_MIGRATOR_COMMENT})); decl.before( @@ -160,3 +132,23 @@ export default function replaceSassPadding(fileInfo: FileInfo) { syntax: require('postcss-scss'), }).css; } + +function hasCalculation(parsedValue: valueParser.ParsedValue): boolean { + let hasCalc = false; + + parsedValue.walk((node) => { + if (isNumericOperator(node)) hasCalc = true; + }); + + return hasCalc; +} + +function isNumericOperator(node: Node): boolean { + return ( + node.value === '+' || + node.value === '-' || + node.value === '*' || + node.value === '/' || + node.value === '%' + ); +} diff --git a/polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.input.scss b/polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.input.scss index 51ce4682f3b..6435b0ce660 100644 --- a/polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.input.scss +++ b/polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.input.scss @@ -3,7 +3,7 @@ padding-top: 13px; padding: 0 1rem 2em 3px 4in; padding-bottom: var(--p-space-0); - padding-right: rem(2px); + padding: rem(2px) 2px; } .another-class { diff --git a/polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.output.scss b/polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.output.scss index 65e5a2df43b..0a0a122b131 100644 --- a/polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.output.scss +++ b/polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.output.scss @@ -1,15 +1,15 @@ .my-class { color: red; padding-top: 13px; - padding: var(--p-space-0) var(--p-space-4) var(--p-space-8) 3px 4in; + padding: var(--p-space-0) var(--p-space-4) 2em 3px 4in; padding-bottom: var(--p-space-0); - padding-right: var(--p-space-05); + padding: var(--p-space-05) var(--p-space-05); } .another-class { padding-bottom: var(--p-space-2); padding-top: var(--p-space-5); /* polaris-migrator: This is a complex expression that we can't automatically convert. Please check this manually. */ - /* padding-right: calc(2px + 2px); */ + /* padding-right: calc(var(--p-space-05) + var(--p-space-05)); */ padding-right: calc(2px + 2px); } From 6d9ad5fbdcd36d50eec21d01328a9549b7986a3f Mon Sep 17 00:00:00 2001 From: Laura Griffee Date: Thu, 22 Sep 2022 17:02:47 -0600 Subject: [PATCH 06/16] Add changeset Co-Authored-By: Aaron Casanova <32409546+aaronccasanova@users.noreply.github.com> --- .changeset/dirty-mice-camp.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/dirty-mice-camp.md diff --git a/.changeset/dirty-mice-camp.md b/.changeset/dirty-mice-camp.md new file mode 100644 index 00000000000..bcb5fe96e19 --- /dev/null +++ b/.changeset/dirty-mice-camp.md @@ -0,0 +1,5 @@ +--- +'@shopify/polaris-migrator': minor +--- + +Add sass padding migration From e89eb2b6e3467df7ae0a22e2c0fbf120a255714b Mon Sep 17 00:00:00 2001 From: Laura Griffee Date: Mon, 26 Sep 2022 10:14:19 -0600 Subject: [PATCH 07/16] Rename migration Co-Authored-By: Aaron Casanova <32409546+aaronccasanova@users.noreply.github.com> --- .../replace-sass-lengths.ts} | 4 ++-- .../tests/replace-sass-lengths.input.scss} | 0 .../tests/replace-sass-lengths.output.scss} | 0 .../tests/replace-sass-lengths.test.ts} | 4 ++-- 4 files changed, 4 insertions(+), 4 deletions(-) rename polaris-migrator/src/migrations/{replace-sass-padding/replace-sass-padding.ts => replace-sass-lengths/replace-sass-lengths.ts} (97%) rename polaris-migrator/src/migrations/{replace-sass-padding/tests/replace-sass-padding.input.scss => replace-sass-lengths/tests/replace-sass-lengths.input.scss} (100%) rename polaris-migrator/src/migrations/{replace-sass-padding/tests/replace-sass-padding.output.scss => replace-sass-lengths/tests/replace-sass-lengths.output.scss} (100%) rename polaris-migrator/src/migrations/{replace-sass-padding/tests/replace-sass-padding.test.ts => replace-sass-lengths/tests/replace-sass-lengths.test.ts} (66%) diff --git a/polaris-migrator/src/migrations/replace-sass-padding/replace-sass-padding.ts b/polaris-migrator/src/migrations/replace-sass-lengths/replace-sass-lengths.ts similarity index 97% rename from polaris-migrator/src/migrations/replace-sass-padding/replace-sass-padding.ts rename to polaris-migrator/src/migrations/replace-sass-lengths/replace-sass-lengths.ts index 709bb0d7bfb..fbac7de8012 100644 --- a/polaris-migrator/src/migrations/replace-sass-padding/replace-sass-padding.ts +++ b/polaris-migrator/src/migrations/replace-sass-lengths/replace-sass-lengths.ts @@ -62,7 +62,7 @@ const processed = Symbol('processed'); const ExitAndStopTraversing = false; const plugin = (): Plugin => ({ - postcssPlugin: 'replace-sass-padding', + postcssPlugin: 'replace-sass-lengths', Declaration(decl) { // @ts-expect-error - Skip if processed so we don't process it again if (decl[processed]) return; @@ -127,7 +127,7 @@ const plugin = (): Plugin => ({ }, }); -export default function replaceSassPadding(fileInfo: FileInfo) { +export default function replaceSassLengths(fileInfo: FileInfo) { return postcss(plugin()).process(fileInfo.source, { syntax: require('postcss-scss'), }).css; diff --git a/polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.input.scss b/polaris-migrator/src/migrations/replace-sass-lengths/tests/replace-sass-lengths.input.scss similarity index 100% rename from polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.input.scss rename to polaris-migrator/src/migrations/replace-sass-lengths/tests/replace-sass-lengths.input.scss diff --git a/polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.output.scss b/polaris-migrator/src/migrations/replace-sass-lengths/tests/replace-sass-lengths.output.scss similarity index 100% rename from polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.output.scss rename to polaris-migrator/src/migrations/replace-sass-lengths/tests/replace-sass-lengths.output.scss diff --git a/polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.test.ts b/polaris-migrator/src/migrations/replace-sass-lengths/tests/replace-sass-lengths.test.ts similarity index 66% rename from polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.test.ts rename to polaris-migrator/src/migrations/replace-sass-lengths/tests/replace-sass-lengths.test.ts index 2bf9f40a713..cc8a033983e 100644 --- a/polaris-migrator/src/migrations/replace-sass-padding/tests/replace-sass-padding.test.ts +++ b/polaris-migrator/src/migrations/replace-sass-lengths/tests/replace-sass-lengths.test.ts @@ -1,7 +1,7 @@ import {check} from '../../../utilities/testUtils'; -const migration = 'replace-sass-padding'; -const fixtures = ['replace-sass-padding']; +const migration = 'replace-sass-lengths'; +const fixtures = ['replace-sass-lengths']; for (const fixture of fixtures) { check(__dirname, { From 5b6066e96869899c34f79c2e55d1b88784647d73 Mon Sep 17 00:00:00 2001 From: Laura Griffee Date: Mon, 26 Sep 2022 10:24:08 -0600 Subject: [PATCH 08/16] Expand target props Co-Authored-By: Aaron Casanova <32409546+aaronccasanova@users.noreply.github.com> --- .../replace-sass-lengths.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/polaris-migrator/src/migrations/replace-sass-lengths/replace-sass-lengths.ts b/polaris-migrator/src/migrations/replace-sass-lengths/replace-sass-lengths.ts index fbac7de8012..a3ebe28dc89 100644 --- a/polaris-migrator/src/migrations/replace-sass-lengths/replace-sass-lengths.ts +++ b/polaris-migrator/src/migrations/replace-sass-lengths/replace-sass-lengths.ts @@ -12,6 +12,23 @@ const targetProps = [ 'padding-right', 'padding-bottom', 'padding-left', + 'padding-inline', + 'padding-inline-start', + 'padding-inline-end', + 'padding-block', + 'padding-block-start', + 'padding-block-end', + 'margin', + 'margin-top', + 'margin-right', + 'margin-bottom', + 'margin-left', + 'margin-inline', + 'margin-inline-start', + 'margin-inline-end', + 'margin-block', + 'margin-block-start', + 'margin-block-end', ] as const; type TargetProp = typeof targetProps[number]; From e1bbaec4289186d21be6506a8dde5052dfd96f48 Mon Sep 17 00:00:00 2001 From: Laura Griffee Date: Mon, 26 Sep 2022 12:06:48 -0600 Subject: [PATCH 09/16] Add test for negative values Co-Authored-By: Aaron Casanova <32409546+aaronccasanova@users.noreply.github.com> --- .../replace-sass-lengths/tests/replace-sass-lengths.input.scss | 2 ++ .../replace-sass-lengths/tests/replace-sass-lengths.output.scss | 2 ++ 2 files changed, 4 insertions(+) diff --git a/polaris-migrator/src/migrations/replace-sass-lengths/tests/replace-sass-lengths.input.scss b/polaris-migrator/src/migrations/replace-sass-lengths/tests/replace-sass-lengths.input.scss index 6435b0ce660..6bd7936e38e 100644 --- a/polaris-migrator/src/migrations/replace-sass-lengths/tests/replace-sass-lengths.input.scss +++ b/polaris-migrator/src/migrations/replace-sass-lengths/tests/replace-sass-lengths.input.scss @@ -4,6 +4,8 @@ padding: 0 1rem 2em 3px 4in; padding-bottom: var(--p-space-0); padding: rem(2px) 2px; + /* Expect to skip negative lengths */ + padding: rem(-2px) -2px; } .another-class { diff --git a/polaris-migrator/src/migrations/replace-sass-lengths/tests/replace-sass-lengths.output.scss b/polaris-migrator/src/migrations/replace-sass-lengths/tests/replace-sass-lengths.output.scss index 0a0a122b131..cee7d12ff60 100644 --- a/polaris-migrator/src/migrations/replace-sass-lengths/tests/replace-sass-lengths.output.scss +++ b/polaris-migrator/src/migrations/replace-sass-lengths/tests/replace-sass-lengths.output.scss @@ -4,6 +4,8 @@ padding: var(--p-space-0) var(--p-space-4) 2em 3px 4in; padding-bottom: var(--p-space-0); padding: var(--p-space-05) var(--p-space-05); + /* Expect to skip negative lengths */ + padding: rem(-2px) -2px; } .another-class { From c38c8904c74a444da67bc86f9f057981a23ec52f Mon Sep 17 00:00:00 2001 From: Laura Griffee Date: Mon, 26 Sep 2022 16:41:58 -0600 Subject: [PATCH 10/16] Add namespace utilities and update migrations Co-Authored-By: Aaron Casanova <32409546+aaronccasanova@users.noreply.github.com> --- .../replace-sass-lengths.ts | 142 ++++++++++-------- .../tests/replace-sass-lengths.test.ts | 7 +- .../tests/with-namespace.input.scss | 17 +++ .../tests/with-namespace.output.scss | 19 +++ .../replace-sass-spacing.ts | 20 +-- polaris-migrator/src/utilities/sass.ts | 31 ++++ 6 files changed, 162 insertions(+), 74 deletions(-) create mode 100644 polaris-migrator/src/migrations/replace-sass-lengths/tests/with-namespace.input.scss create mode 100644 polaris-migrator/src/migrations/replace-sass-lengths/tests/with-namespace.output.scss create mode 100644 polaris-migrator/src/utilities/sass.ts diff --git a/polaris-migrator/src/migrations/replace-sass-lengths/replace-sass-lengths.ts b/polaris-migrator/src/migrations/replace-sass-lengths/replace-sass-lengths.ts index a3ebe28dc89..2d87abd6428 100644 --- a/polaris-migrator/src/migrations/replace-sass-lengths/replace-sass-lengths.ts +++ b/polaris-migrator/src/migrations/replace-sass-lengths/replace-sass-lengths.ts @@ -1,9 +1,14 @@ -import type {FileInfo} from 'jscodeshift'; +import type {FileInfo, API, Options} from 'jscodeshift'; import postcss, {Plugin} from 'postcss'; import valueParser, {Node} from 'postcss-value-parser'; import {toPx} from '@shopify/polaris-tokens'; import {POLARIS_MIGRATOR_COMMENT} from '../../constants'; +import { + NamespaceOptions, + namespace, + createIsSassFunction, +} from '../../utilities/sass'; // List of the props we want to run this migration on const targetProps = [ @@ -78,74 +83,85 @@ const processed = Symbol('processed'); */ const ExitAndStopTraversing = false; -const plugin = (): Plugin => ({ - postcssPlugin: 'replace-sass-lengths', - 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); - - if (!isTargetProp(prop)) return; - - parsedValue.walk((node) => { - if (node.type === 'function' && node.value === 'rem') { - const argDimension = valueParser.unit(node.nodes[0]?.value ?? ''); - - if ( - argDimension && - supportedDimensionUnits.includes(argDimension.unit) - ) { - const argInPx = toPx(`${argDimension.number}${argDimension.unit}`); - - if (!isSpacingTokenValue(argInPx)) return ExitAndStopTraversing; - - const spacingToken = spacingTokensMap[argInPx]; - - node.value = 'var'; - node.nodes = [ - { - type: 'word', - value: spacingToken, - sourceIndex: node.nodes[0]?.sourceIndex ?? 0, - sourceEndIndex: spacingToken.length, - }, - ...node.nodes.slice(1), - ]; - } +interface PluginOptions extends Options, NamespaceOptions {} + +const plugin = (options: PluginOptions = {}): Plugin => { + const remFunction = namespace('rem', options); + const isRemFunction = createIsSassFunction(remFunction); + + return { + postcssPlugin: 'replace-sass-lengths', + 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); + + if (!isTargetProp(prop)) return; + + parsedValue.walk((node) => { + if (isRemFunction(node)) { + const argDimension = valueParser.unit(node.nodes[0]?.value ?? ''); - return ExitAndStopTraversing; - } else if (node.type === 'word') { - const dimension = valueParser.unit(node.value); + if ( + argDimension && + supportedDimensionUnits.includes(argDimension.unit) + ) { + const argInPx = toPx(`${argDimension.number}${argDimension.unit}`); - if (dimension && supportedDimensionUnits.includes(dimension.unit)) { - const dimensionInPx = toPx(`${dimension.number}${dimension.unit}`); + if (!isSpacingTokenValue(argInPx)) return ExitAndStopTraversing; - if (!isSpacingTokenValue(dimensionInPx)) return; + const spacingToken = spacingTokensMap[argInPx]; - node.value = `var(${spacingTokensMap[dimensionInPx]})`; + node.value = 'var'; + node.nodes = [ + { + type: 'word', + value: spacingToken, + sourceIndex: node.nodes[0]?.sourceIndex ?? 0, + sourceEndIndex: spacingToken.length, + }, + ...node.nodes.slice(1), + ]; + } + + return ExitAndStopTraversing; + } else if (node.type === 'word') { + const dimension = valueParser.unit(node.value); + + if (dimension && supportedDimensionUnits.includes(dimension.unit)) { + const dimensionInPx = toPx(`${dimension.number}${dimension.unit}`); + + if (!isSpacingTokenValue(dimensionInPx)) return; + + node.value = `var(${spacingTokensMap[dimensionInPx]})`; + } } + }); + + if (hasCalculation(parsedValue)) { + // Insert comment if the declaration value contains calculations + decl.before(postcss.comment({text: POLARIS_MIGRATOR_COMMENT})); + decl.before( + postcss.comment({text: `${decl.prop}: ${parsedValue.toString()};`}), + ); + } else { + decl.value = parsedValue.toString(); } - }); - - if (hasCalculation(parsedValue)) { - // Insert comment if the declaration value contains calculations - decl.before(postcss.comment({text: POLARIS_MIGRATOR_COMMENT})); - decl.before( - postcss.comment({text: `${decl.prop}: ${parsedValue.toString()};`}), - ); - } else { - decl.value = parsedValue.toString(); - } - - // @ts-expect-error - Mark the declaration as processed - decl[processed] = true; - }, -}); - -export default function replaceSassLengths(fileInfo: FileInfo) { - return postcss(plugin()).process(fileInfo.source, { + + // @ts-expect-error - Mark the declaration as processed + decl[processed] = true; + }, + }; +}; + +export default function replaceSassLengths( + fileInfo: FileInfo, + _: API, + options: Options, +) { + return postcss(plugin(options)).process(fileInfo.source, { syntax: require('postcss-scss'), }).css; } diff --git a/polaris-migrator/src/migrations/replace-sass-lengths/tests/replace-sass-lengths.test.ts b/polaris-migrator/src/migrations/replace-sass-lengths/tests/replace-sass-lengths.test.ts index cc8a033983e..8774c0f021d 100644 --- a/polaris-migrator/src/migrations/replace-sass-lengths/tests/replace-sass-lengths.test.ts +++ b/polaris-migrator/src/migrations/replace-sass-lengths/tests/replace-sass-lengths.test.ts @@ -1,12 +1,17 @@ import {check} from '../../../utilities/testUtils'; const migration = 'replace-sass-lengths'; -const fixtures = ['replace-sass-lengths']; +const fixtures = ['replace-sass-lengths', 'with-namespace']; for (const fixture of fixtures) { check(__dirname, { fixture, migration, extension: 'scss', + options: { + namespace: fixture.includes('with-namespace') + ? 'legacy-polaris-v8' + : undefined, + }, }); } diff --git a/polaris-migrator/src/migrations/replace-sass-lengths/tests/with-namespace.input.scss b/polaris-migrator/src/migrations/replace-sass-lengths/tests/with-namespace.input.scss new file mode 100644 index 00000000000..fc456fb5d26 --- /dev/null +++ b/polaris-migrator/src/migrations/replace-sass-lengths/tests/with-namespace.input.scss @@ -0,0 +1,17 @@ +@use 'global-styles/legacy-polaris-v8'; + +.my-class { + color: red; + padding-top: 13px; + padding: 0 1rem 2em 3px 4in; + padding-bottom: var(--p-space-0); + padding: legacy-polaris-v8.rem(2px) 2px; + /* Expect to skip negative lengths */ + padding: legacy-polaris-v8.rem(-2px) -2px; +} + +.another-class { + padding-bottom: 0.5rem; + padding-top: 1.25rem; + padding-right: calc(2px + 2px); +} diff --git a/polaris-migrator/src/migrations/replace-sass-lengths/tests/with-namespace.output.scss b/polaris-migrator/src/migrations/replace-sass-lengths/tests/with-namespace.output.scss new file mode 100644 index 00000000000..4b188b0828d --- /dev/null +++ b/polaris-migrator/src/migrations/replace-sass-lengths/tests/with-namespace.output.scss @@ -0,0 +1,19 @@ +@use 'global-styles/legacy-polaris-v8'; + +.my-class { + color: red; + padding-top: 13px; + padding: var(--p-space-0) var(--p-space-4) 2em 3px 4in; + padding-bottom: var(--p-space-0); + padding: var(--p-space-05) var(--p-space-05); + /* Expect to skip negative lengths */ + padding: legacy-polaris-v8.rem(-2px) -2px; +} + +.another-class { + padding-bottom: var(--p-space-2); + padding-top: var(--p-space-5); + /* polaris-migrator: This is a complex expression that we can't automatically convert. Please check this manually. */ + /* padding-right: calc(var(--p-space-05) + var(--p-space-05)); */ + padding-right: calc(2px + 2px); +} diff --git a/polaris-migrator/src/migrations/replace-sass-spacing/replace-sass-spacing.ts b/polaris-migrator/src/migrations/replace-sass-spacing/replace-sass-spacing.ts index 5a7d5e3caf3..9ef03dc94f4 100644 --- a/polaris-migrator/src/migrations/replace-sass-spacing/replace-sass-spacing.ts +++ b/polaris-migrator/src/migrations/replace-sass-spacing/replace-sass-spacing.ts @@ -3,6 +3,11 @@ import postcss, {Plugin} from 'postcss'; import valueParser, {Node, FunctionNode} from 'postcss-value-parser'; import {POLARIS_MIGRATOR_COMMENT} from '../../constants'; +import { + NamespaceOptions, + namespace, + createIsSassFunction, +} from '../../utilities/sass'; const spacingMap = { none: '--p-space-0', @@ -30,16 +35,11 @@ function isNumericOperator(node: Node): boolean { const processed = Symbol('processed'); -interface PluginOptions extends Options { - namespace?: string; -} +interface PluginOptions extends Options, NamespaceOptions {} const plugin = (options: PluginOptions = {}): Plugin => { - const namespace = options?.namespace || ''; - const functionName = namespace ? `${namespace}.spacing` : 'spacing'; - const isSpacingFn = (node: Node): node is FunctionNode => { - return node.type === 'function' && node.value === functionName; - }; + const spacingFunction = namespace('spacing', options); + const isSpacingFunction = createIsSassFunction(spacingFunction); return { postcssPlugin: 'ReplaceSassSpacing', @@ -53,10 +53,10 @@ const plugin = (options: PluginOptions = {}): Plugin => { let containsCalculation = false; parsed.walk((node) => { - if (isSpacingFn(node)) containsSpacingFn = true; + if (isSpacingFunction(node)) containsSpacingFn = true; if (isNumericOperator(node)) containsCalculation = true; - if (!isSpacingFn(node)) return; + if (!isSpacingFunction(node)) return; const spacing = node.nodes[0]?.value ?? ''; diff --git a/polaris-migrator/src/utilities/sass.ts b/polaris-migrator/src/utilities/sass.ts new file mode 100644 index 00000000000..a355e42f5f4 --- /dev/null +++ b/polaris-migrator/src/utilities/sass.ts @@ -0,0 +1,31 @@ +import {Node, FunctionNode} from 'postcss-value-parser'; + +function getNamespace(options?: NamespaceOptions) { + return options?.namespace || ''; +} + +export interface NamespaceOptions { + namespace?: string; +} + +export function namespace(name: string, options?: NamespaceOptions) { + const namespace = getNamespace(options); + return namespace ? `${namespace}.${name}` : name; +} + +/** + * @example + * const spacingFunction = namespace('spacing', options); + * const remFunction = namespace('rem', options); + * + * const isSpacingFunction = createIsSassFunction(spacingFunction); + * const isRemFunction = createIsSassFunction(remFunction); + * const isCalcFunction = createIsSassFunction('calc'); + * + * if (isSpacingFunction(node)) node // FunctionNode + */ +export function createIsSassFunction(name: string) { + return (node: Node): node is FunctionNode => { + return node.type === 'function' && node.value === name; + }; +} From f2c0f18f1c0cd1a9dfb998c21392dbfe949b16b6 Mon Sep 17 00:00:00 2001 From: Laura Griffee Date: Mon, 26 Sep 2022 16:46:44 -0600 Subject: [PATCH 11/16] Update types Co-Authored-By: Aaron Casanova <32409546+aaronccasanova@users.noreply.github.com> --- .../src/migrations/replace-sass-spacing/replace-sass-spacing.ts | 2 +- polaris-migrator/src/utilities/sass.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/polaris-migrator/src/migrations/replace-sass-spacing/replace-sass-spacing.ts b/polaris-migrator/src/migrations/replace-sass-spacing/replace-sass-spacing.ts index 9ef03dc94f4..46e44bb9591 100644 --- a/polaris-migrator/src/migrations/replace-sass-spacing/replace-sass-spacing.ts +++ b/polaris-migrator/src/migrations/replace-sass-spacing/replace-sass-spacing.ts @@ -1,6 +1,6 @@ import type {FileInfo, API, Options} from 'jscodeshift'; import postcss, {Plugin} from 'postcss'; -import valueParser, {Node, FunctionNode} from 'postcss-value-parser'; +import valueParser, {Node} from 'postcss-value-parser'; import {POLARIS_MIGRATOR_COMMENT} from '../../constants'; import { diff --git a/polaris-migrator/src/utilities/sass.ts b/polaris-migrator/src/utilities/sass.ts index a355e42f5f4..ef6104dfccf 100644 --- a/polaris-migrator/src/utilities/sass.ts +++ b/polaris-migrator/src/utilities/sass.ts @@ -1,4 +1,4 @@ -import {Node, FunctionNode} from 'postcss-value-parser'; +import type {Node, FunctionNode} from 'postcss-value-parser'; function getNamespace(options?: NamespaceOptions) { return options?.namespace || ''; From d29cf538a6e08d6ee055c127366725f488f1937e Mon Sep 17 00:00:00 2001 From: Laura Griffee Date: Tue, 27 Sep 2022 10:31:38 -0600 Subject: [PATCH 12/16] Update .changeset/dirty-mice-camp.md --- .changeset/dirty-mice-camp.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/dirty-mice-camp.md b/.changeset/dirty-mice-camp.md index bcb5fe96e19..796cc35d1c8 100644 --- a/.changeset/dirty-mice-camp.md +++ b/.changeset/dirty-mice-camp.md @@ -2,4 +2,4 @@ '@shopify/polaris-migrator': minor --- -Add sass padding migration +Add sass padding and margin migration From f6e47fa81dfe256b7fc4ec787a86e26381735a92 Mon Sep 17 00:00:00 2001 From: Laura Griffee Date: Wed, 28 Sep 2022 12:25:50 -0600 Subject: [PATCH 13/16] Make isNumericOperator a sass utility --- .../replace-sass-lengths/replace-sass-lengths.ts | 13 ++----------- .../replace-sass-spacing/replace-sass-spacing.ts | 13 ++----------- polaris-migrator/src/utilities/sass.ts | 10 ++++++++++ 3 files changed, 14 insertions(+), 22 deletions(-) diff --git a/polaris-migrator/src/migrations/replace-sass-lengths/replace-sass-lengths.ts b/polaris-migrator/src/migrations/replace-sass-lengths/replace-sass-lengths.ts index 2d87abd6428..6fe3f9d5f12 100644 --- a/polaris-migrator/src/migrations/replace-sass-lengths/replace-sass-lengths.ts +++ b/polaris-migrator/src/migrations/replace-sass-lengths/replace-sass-lengths.ts @@ -1,6 +1,6 @@ import type {FileInfo, API, Options} from 'jscodeshift'; import postcss, {Plugin} from 'postcss'; -import valueParser, {Node} from 'postcss-value-parser'; +import valueParser from 'postcss-value-parser'; import {toPx} from '@shopify/polaris-tokens'; import {POLARIS_MIGRATOR_COMMENT} from '../../constants'; @@ -8,6 +8,7 @@ import { NamespaceOptions, namespace, createIsSassFunction, + isNumericOperator, } from '../../utilities/sass'; // List of the props we want to run this migration on @@ -175,13 +176,3 @@ function hasCalculation(parsedValue: valueParser.ParsedValue): boolean { return hasCalc; } - -function isNumericOperator(node: Node): boolean { - return ( - node.value === '+' || - node.value === '-' || - node.value === '*' || - node.value === '/' || - node.value === '%' - ); -} diff --git a/polaris-migrator/src/migrations/replace-sass-spacing/replace-sass-spacing.ts b/polaris-migrator/src/migrations/replace-sass-spacing/replace-sass-spacing.ts index 46e44bb9591..b3b1aa693ea 100644 --- a/polaris-migrator/src/migrations/replace-sass-spacing/replace-sass-spacing.ts +++ b/polaris-migrator/src/migrations/replace-sass-spacing/replace-sass-spacing.ts @@ -1,12 +1,13 @@ import type {FileInfo, API, Options} from 'jscodeshift'; import postcss, {Plugin} from 'postcss'; -import valueParser, {Node} from 'postcss-value-parser'; +import valueParser from 'postcss-value-parser'; import {POLARIS_MIGRATOR_COMMENT} from '../../constants'; import { NamespaceOptions, namespace, createIsSassFunction, + isNumericOperator, } from '../../utilities/sass'; const spacingMap = { @@ -23,16 +24,6 @@ const spacingMap = { const isSpacing = (spacing: unknown): spacing is keyof typeof spacingMap => Object.keys(spacingMap).includes(spacing as string); -function isNumericOperator(node: Node): boolean { - return ( - node.value === '+' || - node.value === '-' || - node.value === '*' || - node.value === '/' || - node.value === '%' - ); -} - const processed = Symbol('processed'); interface PluginOptions extends Options, NamespaceOptions {} diff --git a/polaris-migrator/src/utilities/sass.ts b/polaris-migrator/src/utilities/sass.ts index ef6104dfccf..809252516ad 100644 --- a/polaris-migrator/src/utilities/sass.ts +++ b/polaris-migrator/src/utilities/sass.ts @@ -29,3 +29,13 @@ export function createIsSassFunction(name: string) { return node.type === 'function' && node.value === name; }; } + +export function isNumericOperator(node: Node): boolean { + return ( + node.value === '+' || + node.value === '-' || + node.value === '*' || + node.value === '/' || + node.value === '%' + ); +} From eece17e4fc156595c445c1039ab87aa9beeb9228 Mon Sep 17 00:00:00 2001 From: Laura Griffee Date: Wed, 28 Sep 2022 14:59:23 -0600 Subject: [PATCH 14/16] Add JSDoc comment Co-Authored-By: Aaron Casanova <32409546+aaronccasanova@users.noreply.github.com> --- polaris-migrator/src/utilities/sass.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/polaris-migrator/src/utilities/sass.ts b/polaris-migrator/src/utilities/sass.ts index 809252516ad..f4cdc1bf13b 100644 --- a/polaris-migrator/src/utilities/sass.ts +++ b/polaris-migrator/src/utilities/sass.ts @@ -30,6 +30,9 @@ export function createIsSassFunction(name: string) { }; } +/** + * Checks if a `valueParser` node is a [Sass numeric operator](https://sass-lang.com/documentation/operators/numeric) + */ export function isNumericOperator(node: Node): boolean { return ( node.value === '+' || From 4d3307e6168b232df6643e5a00f8281cf9521430 Mon Sep 17 00:00:00 2001 From: Laura Griffee Date: Thu, 29 Sep 2022 13:02:46 -0600 Subject: [PATCH 15/16] Rescope migration and update utils Co-Authored-By: Aaron Casanova <32409546+aaronccasanova@users.noreply.github.com> --- .../replace-sass-lengths.ts | 25 ++++----- .../tests/replace-sass-lengths.output.scss | 2 - .../tests/with-namespace.output.scss | 2 - .../replace-sass-spacing.ts | 19 +++---- polaris-migrator/src/utilities/sass.ts | 55 +++++++++++++++---- 5 files changed, 64 insertions(+), 39 deletions(-) diff --git a/polaris-migrator/src/migrations/replace-sass-lengths/replace-sass-lengths.ts b/polaris-migrator/src/migrations/replace-sass-lengths/replace-sass-lengths.ts index 6fe3f9d5f12..6ce53365362 100644 --- a/polaris-migrator/src/migrations/replace-sass-lengths/replace-sass-lengths.ts +++ b/polaris-migrator/src/migrations/replace-sass-lengths/replace-sass-lengths.ts @@ -8,7 +8,8 @@ import { NamespaceOptions, namespace, createIsSassFunction, - isNumericOperator, + createHasSassFunction, + hasCalculation, } from '../../utilities/sass'; // List of the props we want to run this migration on @@ -89,6 +90,7 @@ interface PluginOptions extends Options, NamespaceOptions {} const plugin = (options: PluginOptions = {}): Plugin => { const remFunction = namespace('rem', options); const isRemFunction = createIsSassFunction(remFunction); + const hasRemFunction = createHasSassFunction(remFunction); return { postcssPlugin: 'replace-sass-lengths', @@ -97,12 +99,17 @@ const plugin = (options: PluginOptions = {}): Plugin => { if (decl[processed]) return; const prop = decl.prop; - const parsedValue = valueParser(decl.value); if (!isTargetProp(prop)) return; + const parsedValue = valueParser(decl.value); + const containsRemFunction = hasRemFunction(parsedValue); + const containsCalculation = hasCalculation(parsedValue); + parsedValue.walk((node) => { - if (isRemFunction(node)) { + if (node.type === 'function') { + if (!isRemFunction(node)) return ExitAndStopTraversing; + const argDimension = valueParser.unit(node.nodes[0]?.value ?? ''); if ( @@ -141,7 +148,7 @@ const plugin = (options: PluginOptions = {}): Plugin => { } }); - if (hasCalculation(parsedValue)) { + if (containsRemFunction && containsCalculation) { // Insert comment if the declaration value contains calculations decl.before(postcss.comment({text: POLARIS_MIGRATOR_COMMENT})); decl.before( @@ -166,13 +173,3 @@ export default function replaceSassLengths( syntax: require('postcss-scss'), }).css; } - -function hasCalculation(parsedValue: valueParser.ParsedValue): boolean { - let hasCalc = false; - - parsedValue.walk((node) => { - if (isNumericOperator(node)) hasCalc = true; - }); - - return hasCalc; -} diff --git a/polaris-migrator/src/migrations/replace-sass-lengths/tests/replace-sass-lengths.output.scss b/polaris-migrator/src/migrations/replace-sass-lengths/tests/replace-sass-lengths.output.scss index cee7d12ff60..93ad34f0074 100644 --- a/polaris-migrator/src/migrations/replace-sass-lengths/tests/replace-sass-lengths.output.scss +++ b/polaris-migrator/src/migrations/replace-sass-lengths/tests/replace-sass-lengths.output.scss @@ -11,7 +11,5 @@ .another-class { padding-bottom: var(--p-space-2); padding-top: var(--p-space-5); - /* polaris-migrator: This is a complex expression that we can't automatically convert. Please check this manually. */ - /* padding-right: calc(var(--p-space-05) + var(--p-space-05)); */ padding-right: calc(2px + 2px); } diff --git a/polaris-migrator/src/migrations/replace-sass-lengths/tests/with-namespace.output.scss b/polaris-migrator/src/migrations/replace-sass-lengths/tests/with-namespace.output.scss index 4b188b0828d..e8842d81daa 100644 --- a/polaris-migrator/src/migrations/replace-sass-lengths/tests/with-namespace.output.scss +++ b/polaris-migrator/src/migrations/replace-sass-lengths/tests/with-namespace.output.scss @@ -13,7 +13,5 @@ .another-class { padding-bottom: var(--p-space-2); padding-top: var(--p-space-5); - /* polaris-migrator: This is a complex expression that we can't automatically convert. Please check this manually. */ - /* padding-right: calc(var(--p-space-05) + var(--p-space-05)); */ padding-right: calc(2px + 2px); } diff --git a/polaris-migrator/src/migrations/replace-sass-spacing/replace-sass-spacing.ts b/polaris-migrator/src/migrations/replace-sass-spacing/replace-sass-spacing.ts index b3b1aa693ea..af81638a990 100644 --- a/polaris-migrator/src/migrations/replace-sass-spacing/replace-sass-spacing.ts +++ b/polaris-migrator/src/migrations/replace-sass-spacing/replace-sass-spacing.ts @@ -6,8 +6,9 @@ import {POLARIS_MIGRATOR_COMMENT} from '../../constants'; import { NamespaceOptions, namespace, + hasCalculation, createIsSassFunction, - isNumericOperator, + createHasSassFunction, } from '../../utilities/sass'; const spacingMap = { @@ -31,6 +32,7 @@ interface PluginOptions extends Options, NamespaceOptions {} const plugin = (options: PluginOptions = {}): Plugin => { const spacingFunction = namespace('spacing', options); const isSpacingFunction = createIsSassFunction(spacingFunction); + const hasSpacingFunction = createHasSassFunction(spacingFunction); return { postcssPlugin: 'ReplaceSassSpacing', @@ -38,15 +40,12 @@ const plugin = (options: PluginOptions = {}): Plugin => { // @ts-expect-error - Skip if processed so we don't process it again if (decl[processed]) return; - const parsed = valueParser(decl.value); + const parsedValue = valueParser(decl.value); - let containsSpacingFn = false; - let containsCalculation = false; - - parsed.walk((node) => { - if (isSpacingFunction(node)) containsSpacingFn = true; - if (isNumericOperator(node)) containsCalculation = true; + const containsSpacingFn = hasSpacingFunction(parsedValue); + const containsCalculation = hasCalculation(parsedValue); + parsedValue.walk((node) => { if (!isSpacingFunction(node)) return; const spacing = node.nodes[0]?.value ?? ''; @@ -70,10 +69,10 @@ const plugin = (options: PluginOptions = {}): Plugin => { // Insert comment if the declaration value contains calculations decl.before(postcss.comment({text: POLARIS_MIGRATOR_COMMENT})); decl.before( - postcss.comment({text: `${decl.prop}: ${parsed.toString()};`}), + postcss.comment({text: `${decl.prop}: ${parsedValue.toString()};`}), ); } else { - decl.value = parsed.toString(); + decl.value = parsedValue.toString(); } // @ts-expect-error - Mark the declaration as processed diff --git a/polaris-migrator/src/utilities/sass.ts b/polaris-migrator/src/utilities/sass.ts index f4cdc1bf13b..46af2193d9b 100644 --- a/polaris-migrator/src/utilities/sass.ts +++ b/polaris-migrator/src/utilities/sass.ts @@ -1,4 +1,4 @@ -import type {Node, FunctionNode} from 'postcss-value-parser'; +import type {Node, ParsedValue, FunctionNode} from 'postcss-value-parser'; function getNamespace(options?: NamespaceOptions) { return options?.namespace || ''; @@ -14,6 +14,34 @@ export function namespace(name: string, options?: NamespaceOptions) { } /** + * Checks if a `valueParser` node is a [Sass numeric operator](https://sass-lang.com/documentation/operators/numeric) + */ +export function isNumericOperator(node: Node): boolean { + return ( + node.value === '+' || + node.value === '-' || + node.value === '*' || + node.value === '/' || + node.value === '%' + ); +} + +/** + * Checks if any descendant `valueParser` node is a numeric operator + */ +export function hasCalculation(parsedValue: ParsedValue): boolean { + let hasCalc = false; + + parsedValue.walk((node) => { + if (isNumericOperator(node)) hasCalc = true; + }); + + return hasCalc; +} + +/** + * Creates a function to check if a `valueParser` node is a given Sass function + * * @example * const spacingFunction = namespace('spacing', options); * const remFunction = namespace('rem', options); @@ -25,20 +53,25 @@ export function namespace(name: string, options?: NamespaceOptions) { * if (isSpacingFunction(node)) node // FunctionNode */ export function createIsSassFunction(name: string) { - return (node: Node): node is FunctionNode => { + return function isSassFunction(node: Node): node is FunctionNode { return node.type === 'function' && node.value === name; }; } /** - * Checks if a `valueParser` node is a [Sass numeric operator](https://sass-lang.com/documentation/operators/numeric) + * Creates a function to check if any descendant `valueParser` node is a given Sass function + * Important: Use before mutating `parsedValue` */ -export function isNumericOperator(node: Node): boolean { - return ( - node.value === '+' || - node.value === '-' || - node.value === '*' || - node.value === '/' || - node.value === '%' - ); +export function createHasSassFunction(name: string) { + const isSassFunction = createIsSassFunction(name); + + return function hasSassFunction(parsedValue: ParsedValue) { + let hasFn = false; + + parsedValue.walk((node) => { + if (isSassFunction(node)) hasFn = true; + }); + + return hasFn; + }; } From b43a2b0a01cb1a0b6c0e1711158fe2015431bb5c Mon Sep 17 00:00:00 2001 From: Aaron Casanova <32409546+aaronccasanova@users.noreply.github.com> Date: Thu, 29 Sep 2022 14:57:01 -0700 Subject: [PATCH 16/16] Replace sass lengths experiments (#7308) --- .../replace-sass-lengths.ts | 93 ++++++++----------- .../tests/replace-sass-lengths.input.scss | 73 ++++++++++++--- .../tests/replace-sass-lengths.output.scss | 89 +++++++++++++++--- .../tests/with-namespace.input.scss | 72 +++++++++++--- .../tests/with-namespace.output.scss | 88 +++++++++++++++--- .../replace-sass-spacing.ts | 17 ++-- polaris-migrator/src/utilities/sass.ts | 52 +++++------ 7 files changed, 346 insertions(+), 138 deletions(-) diff --git a/polaris-migrator/src/migrations/replace-sass-lengths/replace-sass-lengths.ts b/polaris-migrator/src/migrations/replace-sass-lengths/replace-sass-lengths.ts index 6ce53365362..1331881018a 100644 --- a/polaris-migrator/src/migrations/replace-sass-lengths/replace-sass-lengths.ts +++ b/polaris-migrator/src/migrations/replace-sass-lengths/replace-sass-lengths.ts @@ -4,13 +4,7 @@ import valueParser from 'postcss-value-parser'; import {toPx} from '@shopify/polaris-tokens'; import {POLARIS_MIGRATOR_COMMENT} from '../../constants'; -import { - NamespaceOptions, - namespace, - createIsSassFunction, - createHasSassFunction, - hasCalculation, -} from '../../utilities/sass'; +import {hasNumericOperator} from '../../utilities/sass'; // List of the props we want to run this migration on const targetProps = [ @@ -70,28 +64,17 @@ type SpacingToken = keyof typeof spacingTokensMap; const isSpacingTokenValue = (value: unknown): value is SpacingToken => Object.keys(spacingTokensMap).includes(value as SpacingToken); -/** - * All supported dimension units. These values are used to determine - * if a decl.value can be converted to pixels and mapped to a Polaris custom property. - * Note: The empty string is used to match the `0` value - */ -const supportedDimensionUnits = ['px', 'rem', '']; - const processed = Symbol('processed'); /** * Exit early and stop traversing descendant nodes: * https://www.npmjs.com/package/postcss-value-parser:~:text=Returning%20false%20in%20the%20callback%20will%20prevent%20traversal%20of%20descendent%20nodes */ -const ExitAndStopTraversing = false; - -interface PluginOptions extends Options, NamespaceOptions {} +const StopWalkingFunctionNodes = false; -const plugin = (options: PluginOptions = {}): Plugin => { - const remFunction = namespace('rem', options); - const isRemFunction = createIsSassFunction(remFunction); - const hasRemFunction = createHasSassFunction(remFunction); +interface PluginOptions extends Options {} +const plugin = (_options: PluginOptions = {}): Plugin => { return { postcssPlugin: 'replace-sass-lengths', Declaration(decl) { @@ -103,42 +86,16 @@ const plugin = (options: PluginOptions = {}): Plugin => { if (!isTargetProp(prop)) return; const parsedValue = valueParser(decl.value); - const containsRemFunction = hasRemFunction(parsedValue); - const containsCalculation = hasCalculation(parsedValue); + + if (!hasTransformableLength(parsedValue)) return; parsedValue.walk((node) => { if (node.type === 'function') { - if (!isRemFunction(node)) return ExitAndStopTraversing; - - const argDimension = valueParser.unit(node.nodes[0]?.value ?? ''); - - if ( - argDimension && - supportedDimensionUnits.includes(argDimension.unit) - ) { - const argInPx = toPx(`${argDimension.number}${argDimension.unit}`); - - if (!isSpacingTokenValue(argInPx)) return ExitAndStopTraversing; - - const spacingToken = spacingTokensMap[argInPx]; - - node.value = 'var'; - node.nodes = [ - { - type: 'word', - value: spacingToken, - sourceIndex: node.nodes[0]?.sourceIndex ?? 0, - sourceEndIndex: spacingToken.length, - }, - ...node.nodes.slice(1), - ]; - } - - return ExitAndStopTraversing; + return StopWalkingFunctionNodes; } else if (node.type === 'word') { const dimension = valueParser.unit(node.value); - if (dimension && supportedDimensionUnits.includes(dimension.unit)) { + if (isTransformableLength(dimension)) { const dimensionInPx = toPx(`${dimension.number}${dimension.unit}`); if (!isSpacingTokenValue(dimensionInPx)) return; @@ -148,7 +105,7 @@ const plugin = (options: PluginOptions = {}): Plugin => { } }); - if (containsRemFunction && containsCalculation) { + if (hasNumericOperator(parsedValue)) { // Insert comment if the declaration value contains calculations decl.before(postcss.comment({text: POLARIS_MIGRATOR_COMMENT})); decl.before( @@ -173,3 +130,35 @@ export default function replaceSassLengths( syntax: require('postcss-scss'), }).css; } + +/** + * All transformable dimension units. These values are used to determine + * if a decl.value can be converted to pixels and mapped to a Polaris custom property. + */ +const transformableLengthUnits = ['px', 'rem']; + +function isTransformableLength( + dimension: false | valueParser.Dimension, +): dimension is valueParser.Dimension { + if (!dimension) return false; + + // Zero is the only unitless length we can transform + if (dimension.unit === '' && dimension.number === '0') return true; + + return transformableLengthUnits.includes(dimension.unit); +} + +function hasTransformableLength(parsedValue: valueParser.ParsedValue): boolean { + let transformableLength = false; + + parsedValue.walk((node) => { + if ( + node.type === 'word' && + isTransformableLength(valueParser.unit(node.value)) + ) { + transformableLength = true; + } + }); + + return transformableLength; +} diff --git a/polaris-migrator/src/migrations/replace-sass-lengths/tests/replace-sass-lengths.input.scss b/polaris-migrator/src/migrations/replace-sass-lengths/tests/replace-sass-lengths.input.scss index 6bd7936e38e..a72d9ab5c10 100644 --- a/polaris-migrator/src/migrations/replace-sass-lengths/tests/replace-sass-lengths.input.scss +++ b/polaris-migrator/src/migrations/replace-sass-lengths/tests/replace-sass-lengths.input.scss @@ -1,15 +1,64 @@ +/* stylelint-disable shorthand-property-no-redundant-values */ + .my-class { - color: red; - padding-top: 13px; - padding: 0 1rem 2em 3px 4in; - padding-bottom: var(--p-space-0); - padding: rem(2px) 2px; - /* Expect to skip negative lengths */ - padding: rem(-2px) -2px; -} + color: lightsteelblue; + + // Unitless + padding: 0; + padding: 1; + padding: 2; + + // Single shorthand pixels + padding: 16px; + padding: 10px; + + // Single shorthand rems + padding: 1rem; + padding: 10rem; + + // Double shorthand pixels + padding: 16px 16px; + padding: 10px 10px; + padding: 16px 10px; + + // Double shorthand rems + padding: 1rem 1rem; + padding: 10rem 10rem; + padding: 1rem 10rem; + + // Double shorthand mixed (px, rem, unitless) + padding: 16px 1rem; + padding: 10px 10rem; + padding: 16px 10rem; + padding: 10px 1rem; + padding: 0 16px; + padding: 1rem 0; + + // Double shorthand mixed edge cases + padding: 16px 4in; + padding: 16px 1em; + padding: 16px -16px; + + // Unsupported transforms with comments + padding: 16px + 16px; + padding: 16px + 16px 16px; + padding: (16px + 16px) 16px; + padding: $var + 16px; + padding: rem(16px) + 16px; + padding: rem($var * 16px); + padding: calc(16px + 16px); + padding: 16px + #{16px + 16px}; -.another-class { - padding-bottom: 0.5rem; - padding-top: 1.25rem; - padding-right: calc(2px + 2px); + // Unsupported transforms without comments + padding: rem(16px); + padding: #{16px + 16px}; + padding: layout-width(nav); + padding: 10em; + padding: var(--var); + padding: var(--p-space-4); + padding: var(--p-space-4, 16px); + // Skip negative lengths. Need to discuss replacement strategy e.g. + // calc(-1 * var(--p-space-*)) vs var(--p-space--*) + padding: -16px; + padding: -10px; } diff --git a/polaris-migrator/src/migrations/replace-sass-lengths/tests/replace-sass-lengths.output.scss b/polaris-migrator/src/migrations/replace-sass-lengths/tests/replace-sass-lengths.output.scss index 93ad34f0074..5930572714e 100644 --- a/polaris-migrator/src/migrations/replace-sass-lengths/tests/replace-sass-lengths.output.scss +++ b/polaris-migrator/src/migrations/replace-sass-lengths/tests/replace-sass-lengths.output.scss @@ -1,15 +1,80 @@ +/* stylelint-disable shorthand-property-no-redundant-values */ + .my-class { - color: red; - padding-top: 13px; - padding: var(--p-space-0) var(--p-space-4) 2em 3px 4in; - padding-bottom: var(--p-space-0); - padding: var(--p-space-05) var(--p-space-05); - /* Expect to skip negative lengths */ - padding: rem(-2px) -2px; -} + color: lightsteelblue; + + // Unitless + padding: var(--p-space-0); + padding: 1; + padding: 2; + + // Single shorthand pixels + padding: var(--p-space-4); + padding: 10px; + + // Single shorthand rems + padding: var(--p-space-4); + padding: 10rem; + + // Double shorthand pixels + padding: var(--p-space-4) var(--p-space-4); + padding: 10px 10px; + padding: var(--p-space-4) 10px; + + // Double shorthand rems + padding: var(--p-space-4) var(--p-space-4); + padding: 10rem 10rem; + padding: var(--p-space-4) 10rem; + + // Double shorthand mixed (px, rem, unitless) + padding: var(--p-space-4) var(--p-space-4); + padding: 10px 10rem; + padding: var(--p-space-4) 10rem; + padding: 10px var(--p-space-4); + padding: var(--p-space-0) var(--p-space-4); + padding: var(--p-space-4) var(--p-space-0); + + // Double shorthand mixed edge cases + padding: var(--p-space-4) 4in; + padding: var(--p-space-4) 1em; + padding: var(--p-space-4) -16px; + + // Unsupported transforms with comments + /* polaris-migrator: This is a complex expression that we can't automatically convert. Please check this manually. */ + /* padding: var(--p-space-4) + var(--p-space-4); */ + padding: 16px + 16px; + /* polaris-migrator: This is a complex expression that we can't automatically convert. Please check this manually. */ + /* padding: var(--p-space-4) + var(--p-space-4) var(--p-space-4); */ + padding: 16px + 16px 16px; + /* polaris-migrator: This is a complex expression that we can't automatically convert. Please check this manually. */ + /* padding: (16px + 16px) var(--p-space-4); */ + padding: (16px + 16px) 16px; + /* polaris-migrator: This is a complex expression that we can't automatically convert. Please check this manually. */ + /* padding: $var + var(--p-space-4); */ + padding: $var + 16px; + /* polaris-migrator: This is a complex expression that we can't automatically convert. Please check this manually. */ + /* padding: rem(16px) + var(--p-space-4); */ + padding: rem(16px) + 16px; + /* polaris-migrator: This is a complex expression that we can't automatically convert. Please check this manually. */ + /* padding: rem($var * 16px); */ + padding: rem($var * 16px); + /* polaris-migrator: This is a complex expression that we can't automatically convert. Please check this manually. */ + /* padding: calc(16px + 16px); */ + padding: calc(16px + 16px); + /* polaris-migrator: This is a complex expression that we can't automatically convert. Please check this manually. */ + /* padding: var(--p-space-4) + #{16px + 16px}; */ + padding: 16px + #{16px + 16px}; -.another-class { - padding-bottom: var(--p-space-2); - padding-top: var(--p-space-5); - padding-right: calc(2px + 2px); + // Unsupported transforms without comments + padding: rem(16px); + padding: #{16px + 16px}; + padding: layout-width(nav); + padding: 10em; + padding: var(--var); + padding: var(--p-space-4); + padding: var(--p-space-4, 16px); + // Skip negative lengths. Need to discuss replacement strategy e.g. + // calc(-1 * var(--p-space-*)) vs var(--p-space--*) + padding: -16px; + padding: -10px; } diff --git a/polaris-migrator/src/migrations/replace-sass-lengths/tests/with-namespace.input.scss b/polaris-migrator/src/migrations/replace-sass-lengths/tests/with-namespace.input.scss index fc456fb5d26..f89e5b82d7b 100644 --- a/polaris-migrator/src/migrations/replace-sass-lengths/tests/with-namespace.input.scss +++ b/polaris-migrator/src/migrations/replace-sass-lengths/tests/with-namespace.input.scss @@ -1,17 +1,65 @@ +/* stylelint-disable shorthand-property-no-redundant-values */ @use 'global-styles/legacy-polaris-v8'; .my-class { - color: red; - padding-top: 13px; - padding: 0 1rem 2em 3px 4in; - padding-bottom: var(--p-space-0); - padding: legacy-polaris-v8.rem(2px) 2px; - /* Expect to skip negative lengths */ - padding: legacy-polaris-v8.rem(-2px) -2px; -} + color: lightsteelblue; + + // Unitless + padding: 0; + padding: 1; + padding: 2; + + // Single shorthand pixels + padding: 16px; + padding: 10px; + + // Single shorthand rems + padding: 1rem; + padding: 10rem; + + // Double shorthand pixels + padding: 16px 16px; + padding: 10px 10px; + padding: 16px 10px; + + // Double shorthand rems + padding: 1rem 1rem; + padding: 10rem 10rem; + padding: 1rem 10rem; + + // Double shorthand mixed (px, rem, unitless) + padding: 16px 1rem; + padding: 10px 10rem; + padding: 16px 10rem; + padding: 10px 1rem; + padding: 0 16px; + padding: 1rem 0; + + // Double shorthand mixed edge cases + padding: 16px 4in; + padding: 16px 1em; + padding: 16px -16px; + + // Unsupported transforms with comments + padding: 16px + 16px; + padding: 16px + 16px 16px; + padding: (16px + 16px) 16px; + padding: $var + 16px; + padding: legacy-polaris-v8.rem(16px) + 16px; + padding: legacy-polaris-v8.rem($var * 16px); + padding: calc(16px + 16px); + padding: 16px + #{16px + 16px}; -.another-class { - padding-bottom: 0.5rem; - padding-top: 1.25rem; - padding-right: calc(2px + 2px); + // Unsupported transforms without comments + padding: legacy-polaris-v8.rem(16px); + padding: #{16px + 16px}; + padding: layout-width(nav); + padding: 10em; + padding: var(--var); + padding: var(--p-space-4); + padding: var(--p-space-4, 16px); + // Skip negative lengths. Need to discuss replacement strategy e.g. + // calc(-1 * var(--p-space-*)) vs var(--p-space--*) + padding: -16px; + padding: -10px; } diff --git a/polaris-migrator/src/migrations/replace-sass-lengths/tests/with-namespace.output.scss b/polaris-migrator/src/migrations/replace-sass-lengths/tests/with-namespace.output.scss index e8842d81daa..1959968837d 100644 --- a/polaris-migrator/src/migrations/replace-sass-lengths/tests/with-namespace.output.scss +++ b/polaris-migrator/src/migrations/replace-sass-lengths/tests/with-namespace.output.scss @@ -1,17 +1,81 @@ +/* stylelint-disable shorthand-property-no-redundant-values */ @use 'global-styles/legacy-polaris-v8'; .my-class { - color: red; - padding-top: 13px; - padding: var(--p-space-0) var(--p-space-4) 2em 3px 4in; - padding-bottom: var(--p-space-0); - padding: var(--p-space-05) var(--p-space-05); - /* Expect to skip negative lengths */ - padding: legacy-polaris-v8.rem(-2px) -2px; -} + color: lightsteelblue; + + // Unitless + padding: var(--p-space-0); + padding: 1; + padding: 2; + + // Single shorthand pixels + padding: var(--p-space-4); + padding: 10px; + + // Single shorthand rems + padding: var(--p-space-4); + padding: 10rem; + + // Double shorthand pixels + padding: var(--p-space-4) var(--p-space-4); + padding: 10px 10px; + padding: var(--p-space-4) 10px; + + // Double shorthand rems + padding: var(--p-space-4) var(--p-space-4); + padding: 10rem 10rem; + padding: var(--p-space-4) 10rem; + + // Double shorthand mixed (px, rem, unitless) + padding: var(--p-space-4) var(--p-space-4); + padding: 10px 10rem; + padding: var(--p-space-4) 10rem; + padding: 10px var(--p-space-4); + padding: var(--p-space-0) var(--p-space-4); + padding: var(--p-space-4) var(--p-space-0); + + // Double shorthand mixed edge cases + padding: var(--p-space-4) 4in; + padding: var(--p-space-4) 1em; + padding: var(--p-space-4) -16px; + + // Unsupported transforms with comments + /* polaris-migrator: This is a complex expression that we can't automatically convert. Please check this manually. */ + /* padding: var(--p-space-4) + var(--p-space-4); */ + padding: 16px + 16px; + /* polaris-migrator: This is a complex expression that we can't automatically convert. Please check this manually. */ + /* padding: var(--p-space-4) + var(--p-space-4) var(--p-space-4); */ + padding: 16px + 16px 16px; + /* polaris-migrator: This is a complex expression that we can't automatically convert. Please check this manually. */ + /* padding: (16px + 16px) var(--p-space-4); */ + padding: (16px + 16px) 16px; + /* polaris-migrator: This is a complex expression that we can't automatically convert. Please check this manually. */ + /* padding: $var + var(--p-space-4); */ + padding: $var + 16px; + /* polaris-migrator: This is a complex expression that we can't automatically convert. Please check this manually. */ + /* padding: legacy-polaris-v8.rem(16px) + var(--p-space-4); */ + padding: legacy-polaris-v8.rem(16px) + 16px; + /* polaris-migrator: This is a complex expression that we can't automatically convert. Please check this manually. */ + /* padding: legacy-polaris-v8.rem($var * 16px); */ + padding: legacy-polaris-v8.rem($var * 16px); + /* polaris-migrator: This is a complex expression that we can't automatically convert. Please check this manually. */ + /* padding: calc(16px + 16px); */ + padding: calc(16px + 16px); + /* polaris-migrator: This is a complex expression that we can't automatically convert. Please check this manually. */ + /* padding: var(--p-space-4) + #{16px + 16px}; */ + padding: 16px + #{16px + 16px}; -.another-class { - padding-bottom: var(--p-space-2); - padding-top: var(--p-space-5); - padding-right: calc(2px + 2px); + // Unsupported transforms without comments + padding: legacy-polaris-v8.rem(16px); + padding: #{16px + 16px}; + padding: layout-width(nav); + padding: 10em; + padding: var(--var); + padding: var(--p-space-4); + padding: var(--p-space-4, 16px); + // Skip negative lengths. Need to discuss replacement strategy e.g. + // calc(-1 * var(--p-space-*)) vs var(--p-space--*) + padding: -16px; + padding: -10px; } diff --git a/polaris-migrator/src/migrations/replace-sass-spacing/replace-sass-spacing.ts b/polaris-migrator/src/migrations/replace-sass-spacing/replace-sass-spacing.ts index af81638a990..4007434e008 100644 --- a/polaris-migrator/src/migrations/replace-sass-spacing/replace-sass-spacing.ts +++ b/polaris-migrator/src/migrations/replace-sass-spacing/replace-sass-spacing.ts @@ -6,9 +6,9 @@ import {POLARIS_MIGRATOR_COMMENT} from '../../constants'; import { NamespaceOptions, namespace, - hasCalculation, - createIsSassFunction, - createHasSassFunction, + isSassFunction, + hasSassFunction, + hasNumericOperator, } from '../../utilities/sass'; const spacingMap = { @@ -30,9 +30,7 @@ const processed = Symbol('processed'); interface PluginOptions extends Options, NamespaceOptions {} const plugin = (options: PluginOptions = {}): Plugin => { - const spacingFunction = namespace('spacing', options); - const isSpacingFunction = createIsSassFunction(spacingFunction); - const hasSpacingFunction = createHasSassFunction(spacingFunction); + const namespacedSpacing = namespace('spacing', options); return { postcssPlugin: 'ReplaceSassSpacing', @@ -42,11 +40,10 @@ const plugin = (options: PluginOptions = {}): Plugin => { const parsedValue = valueParser(decl.value); - const containsSpacingFn = hasSpacingFunction(parsedValue); - const containsCalculation = hasCalculation(parsedValue); + if (!hasSassFunction(namespacedSpacing, parsedValue)) return; parsedValue.walk((node) => { - if (!isSpacingFunction(node)) return; + if (!isSassFunction(namespacedSpacing, node)) return; const spacing = node.nodes[0]?.value ?? ''; @@ -65,7 +62,7 @@ const plugin = (options: PluginOptions = {}): Plugin => { ]; }); - if (containsSpacingFn && containsCalculation) { + if (hasNumericOperator(parsedValue)) { // Insert comment if the declaration value contains calculations decl.before(postcss.comment({text: POLARIS_MIGRATOR_COMMENT})); decl.before( diff --git a/polaris-migrator/src/utilities/sass.ts b/polaris-migrator/src/utilities/sass.ts index 46af2193d9b..45210328ff9 100644 --- a/polaris-migrator/src/utilities/sass.ts +++ b/polaris-migrator/src/utilities/sass.ts @@ -29,49 +29,45 @@ export function isNumericOperator(node: Node): boolean { /** * Checks if any descendant `valueParser` node is a numeric operator */ -export function hasCalculation(parsedValue: ParsedValue): boolean { - let hasCalc = false; +export function hasNumericOperator(parsedValue: ParsedValue): boolean { + let containsNumericOperator = false; parsedValue.walk((node) => { - if (isNumericOperator(node)) hasCalc = true; + if (isNumericOperator(node)) containsNumericOperator = true; }); - return hasCalc; + return containsNumericOperator; } /** - * Creates a function to check if a `valueParser` node is a given Sass function + * Checks if a `valueParser` node is a given Sass function * * @example - * const spacingFunction = namespace('spacing', options); - * const remFunction = namespace('rem', options); + * const namespacedRem = namespace('rem', options); * - * const isSpacingFunction = createIsSassFunction(spacingFunction); - * const isRemFunction = createIsSassFunction(remFunction); - * const isCalcFunction = createIsSassFunction('calc'); - * - * if (isSpacingFunction(node)) node // FunctionNode + * if (isSassFunction(namespacedRem, node)) node // FunctionNode */ -export function createIsSassFunction(name: string) { - return function isSassFunction(node: Node): node is FunctionNode { - return node.type === 'function' && node.value === name; - }; +export function isSassFunction(name: string, node: Node): node is FunctionNode { + return node.type === 'function' && node.value === name; } /** - * Creates a function to check if any descendant `valueParser` node is a given Sass function - * Important: Use before mutating `parsedValue` + * Checks if any descendant `valueParser` node is a given Sass function + * + * @example + * const namespacedRem = namespace('rem', options); + * + * if (!hasSassFunction(namespacedRem, parsedValue)) return; */ -export function createHasSassFunction(name: string) { - const isSassFunction = createIsSassFunction(name); +export function hasSassFunction( + name: string, + parsedValue: ParsedValue, +): boolean { + let containsSassFunction = false; - return function hasSassFunction(parsedValue: ParsedValue) { - let hasFn = false; - - parsedValue.walk((node) => { - if (isSassFunction(node)) hasFn = true; - }); + parsedValue.walk((node) => { + if (isSassFunction(name, node)) containsSassFunction = true; + }); - return hasFn; - }; + return containsSassFunction; }