-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Add migration for sass padding and margin length values #7264
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 12 commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
07af2a1
Create initial script scaffolding
lgriffee 6c9d775
Add migration logic for dimension values
lgriffee fe30bb2
Start building logic for handling functions and variables
lgriffee df844f7
Start building out logic for handling rem functions
lgriffee 447b2b4
Rearchitect migration
lgriffee 6d9ad5f
Add changeset
lgriffee d885e33
Merge branch 'main' of https://github.com/Shopify/polaris into replac…
lgriffee e89eb2b
Rename migration
lgriffee 5b6066e
Expand target props
lgriffee e1bbaec
Add test for negative values
lgriffee c38c890
Add namespace utilities and update migrations
lgriffee f2c0f18
Update types
lgriffee d29cf53
Update .changeset/dirty-mice-camp.md
lgriffee f6e47fa
Make isNumericOperator a sass utility
lgriffee eece17e
Add JSDoc comment
lgriffee 4d3307e
Rescope migration and update utils
lgriffee b43a2b0
Replace sass lengths experiments (#7308)
aaronccasanova File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@shopify/polaris-migrator': minor | ||
| --- | ||
|
|
||
| Add sass padding migration | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
187 changes: 187 additions & 0 deletions
187
polaris-migrator/src/migrations/replace-sass-lengths/replace-sass-lengths.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,187 @@ | ||
| 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 = [ | ||
| 'padding', | ||
| 'padding-top', | ||
| '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]; | ||
|
|
||
| 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', | ||
sam-b-rose marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| '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', | ||
| } as const; | ||
|
|
||
| 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 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 ?? ''); | ||
|
|
||
| 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; | ||
lgriffee marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } 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(); | ||
| } | ||
|
|
||
| // @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; | ||
| } | ||
|
|
||
| function hasCalculation(parsedValue: valueParser.ParsedValue): boolean { | ||
| let hasCalc = false; | ||
|
|
||
| parsedValue.walk((node) => { | ||
| if (isNumericOperator(node)) hasCalc = true; | ||
| }); | ||
|
|
||
| return hasCalc; | ||
| } | ||
|
|
||
| function isNumericOperator(node: Node): boolean { | ||
lgriffee marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| return ( | ||
| node.value === '+' || | ||
| node.value === '-' || | ||
| node.value === '*' || | ||
| node.value === '/' || | ||
| node.value === '%' | ||
| ); | ||
| } | ||
15 changes: 15 additions & 0 deletions
15
polaris-migrator/src/migrations/replace-sass-lengths/tests/replace-sass-lengths.input.scss
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| .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; | ||
| } | ||
|
|
||
| .another-class { | ||
| padding-bottom: 0.5rem; | ||
| padding-top: 1.25rem; | ||
| padding-right: calc(2px + 2px); | ||
| } |
17 changes: 17 additions & 0 deletions
17
polaris-migrator/src/migrations/replace-sass-lengths/tests/replace-sass-lengths.output.scss
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| .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 */ | ||
sam-b-rose marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| padding: 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); | ||
| } | ||
17 changes: 17 additions & 0 deletions
17
polaris-migrator/src/migrations/replace-sass-lengths/tests/replace-sass-lengths.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| import {check} from '../../../utilities/testUtils'; | ||
|
|
||
| const migration = '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, | ||
| }, | ||
| }); | ||
| } |
17 changes: 17 additions & 0 deletions
17
polaris-migrator/src/migrations/replace-sass-lengths/tests/with-namespace.input.scss
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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); | ||
| } |
19 changes: 19 additions & 0 deletions
19
polaris-migrator/src/migrations/replace-sass-lengths/tests/with-namespace.output.scss
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| import type {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) { | ||
sam-b-rose marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| return (node: Node): node is FunctionNode => { | ||
| return node.type === 'function' && node.value === name; | ||
| }; | ||
| } | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.