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; }