Skip to content

Conversation

jesstelford
Copy link
Contributor

@jesstelford jesstelford commented Oct 25, 2022

This PR belongs to a series of proposed changes:

NOTE: This PR is best viewed with whitespace changes hidden


The createSassMigrator utility helps encapsulate two patterns that are common across all our migrators:

  1. Initialising and executing postcss
  2. Ensuring the rules only get executed once.

Best visualized as a diff of what a migration might look like:

-import type {FileInfo} from 'jscodeshift';
-import postcss, {Plugin} from 'postcss';
 import valueParser from 'postcss-value-parser';
+
+import { createSassMigrator } from '../../utilities/sass';
 
-const processed = Symbol('processed');
 
-const plugin = (): Plugin => ({
-  postcssPlugin: 'replace-sass-function',
+export default createSassMigrator('replace-sass-function', () => {
-  Declaration(decl) { 
+  return ((root) => {
+    root.walkDecls((decl) => {
-    // @ts-expect-error - Skip if processed so we don't process it again
-    if (decl[processed]) return;
-
     const parsedValue = valueParser(decl.value);
 
     parsedValue.walk((node) => {
       if (!(node.type === 'function' && node.value === 'hello')) return;
 
       node.value = 'world';
     });
 
     decl.value = parsedValue.toString();
-
-    // @ts-expect-error - Mark the declaration as processed
-    decl[processed] = true;
-  },
+  });
+  };
 });
-
-export default function replaceSassFunction(fileInfo: FileInfo) {
-  return postcss(plugin()).process(fileInfo.source, {
-    syntax: require('postcss-scss'),
-  }).css;
-}

Copy link
Member

@sam-b-rose sam-b-rose left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, this is very simple an what I was imagining our Stylelint rule wrapper would look like 👍

@jesstelford jesstelford merged commit 85c0c32 into main Oct 27, 2022
@jesstelford jesstelford deleted the createsassmigrator branch October 27, 2022 23:57
@github-actions github-actions bot mentioned this pull request Oct 27, 2022
laurkim pushed a commit that referenced this pull request Oct 28, 2022
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## @shopify/polaris-migrator@0.7.0

### Minor Changes

- [#7499](#7499)
[`85c0c3290`](85c0c32)
Thanks [@jesstelford](https://github.com/jesstelford)! - Add
`createSassMigrator` utility to stash common logic, starting with only
parsing each event once.


- [#7541](#7541)
[`141746b5b`](141746b)
Thanks [@jesstelford](https://github.com/jesstelford)! - Internally
setup stylelint metadata for SASS migrations in preparation for
switching to stylelint as our migration runner.

## @shopify/plugin-polaris@0.0.15

### Patch Changes

- Updated dependencies
\[[`85c0c3290`](85c0c32),
[`141746b5b`](141746b)]:
    -   @shopify/polaris-migrator@0.7.0

## @shopify/polaris@10.10.1

### Patch Changes

- [#7561](#7561)
[`fc78fcc48`](fc78fcc)
Thanks [@kyledurand](https://github.com/kyledurand)! - Fix typo in box
property, remove PropsWithChildren

## polaris-for-figma@0.0.28

### Patch Changes

- Updated dependencies
\[[`fc78fcc48`](fc78fcc)]:
    -   @shopify/polaris@10.10.1

## polaris.shopify.com@0.23.3

### Patch Changes

- Updated dependencies
\[[`fc78fcc48`](fc78fcc)]:
    -   @shopify/polaris@10.10.1

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@sam-b-rose sam-b-rose mentioned this pull request Oct 31, 2022
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants