-
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 all 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 and margin 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
164 changes: 164 additions & 0 deletions
164
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,164 @@ | ||
| import type {FileInfo, API, Options} from 'jscodeshift'; | ||
| import postcss, {Plugin} from 'postcss'; | ||
| import valueParser from 'postcss-value-parser'; | ||
| import {toPx} from '@shopify/polaris-tokens'; | ||
|
|
||
| import {POLARIS_MIGRATOR_COMMENT} from '../../constants'; | ||
| import {hasNumericOperator} 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', | ||
| '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); | ||
|
|
||
| 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 StopWalkingFunctionNodes = false; | ||
|
|
||
| interface PluginOptions extends Options {} | ||
|
|
||
| const plugin = (_options: PluginOptions = {}): Plugin => { | ||
| 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; | ||
|
|
||
| if (!isTargetProp(prop)) return; | ||
|
|
||
| const parsedValue = valueParser(decl.value); | ||
|
|
||
| if (!hasTransformableLength(parsedValue)) return; | ||
|
|
||
| parsedValue.walk((node) => { | ||
| if (node.type === 'function') { | ||
| return StopWalkingFunctionNodes; | ||
| } else if (node.type === 'word') { | ||
| const dimension = valueParser.unit(node.value); | ||
|
|
||
| if (isTransformableLength(dimension)) { | ||
| const dimensionInPx = toPx(`${dimension.number}${dimension.unit}`); | ||
|
|
||
| if (!isSpacingTokenValue(dimensionInPx)) return; | ||
|
|
||
| node.value = `var(${spacingTokensMap[dimensionInPx]})`; | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| if (hasNumericOperator(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; | ||
| } | ||
|
|
||
| /** | ||
| * 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; | ||
| } | ||
64 changes: 64 additions & 0 deletions
64
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,64 @@ | ||
| /* stylelint-disable shorthand-property-no-redundant-values */ | ||
|
|
||
| .my-class { | ||
| 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}; | ||
|
|
||
| // 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; | ||
| } |
80 changes: 80 additions & 0 deletions
80
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,80 @@ | ||
| /* stylelint-disable shorthand-property-no-redundant-values */ | ||
|
|
||
| .my-class { | ||
| 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}; | ||
|
|
||
| // 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; | ||
| } |
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, | ||
| }, | ||
| }); | ||
| } |
Oops, something went wrong.
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.