-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Create @shopify/polaris-migrator package
#6701
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
Conversation
3db0d76 to
c855b1c
Compare
@shopify/polaris-migrator package
c855b1c to
aee2299
Compare
6474907 to
8315b33
Compare
8315b33 to
0c3aae5
Compare
|
/snapit |
|
🫰✨ Thanks @samrose3! Your snapshots have been published to npm. Test the snapshots by updating your yarn add @shopify/[email protected]yarn add @shopify/[email protected] |
0c3aae5 to
bee4966
Compare
|
/snapit |
|
🫰✨ Thanks @samrose3! Your snapshots have been published to npm. Test the snapshots by updating your yarn add @shopify/[email protected]yarn add @shopify/[email protected] |
bee4966 to
18e114a
Compare
BPScott
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heck yeah I love me some codemods!
Dropped some comments around the place. Noteably I think it'd be very valuable to use postcss-value-parser to get a better AST for handling transforms of prop values.
| postcssPlugin: 'ReplaceSassSpacing', | ||
| Declaration(decl) { | ||
| decl.value = decl.value.replace( | ||
| createRegexFromMap(spacingMap), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a fan of the usage of regex here. It looks like it can match false positives like my-spacing(), and miss variations in whitespace/quoting like spacing( none ), spacing('none') and spacing( "none" ) (admittedly as we use prettier our whitespace should be pretty conventional so this shouldn't be a huuuge issue for our codebases).
I'd expect us to have a better AST here. There should be AST nodes for stuff like "this is a function with a name and these specific arguments" - you should be able to be able to say "only act when you find a function name, whose name is "spacing" and has these specific values as the first argument". We should leverage a tokeniser, rather than trying to fuzzy match ourselves. It looks like this behaviour of transforming values into "this is a function call" etc is provided by postcss-value-parser
stylelint has a demo of using postcss-value-parser https://github.com/stylelint/stylelint/blob/main/lib/rules/function-allowed-list/index.js#L36
polaris-migrator/src/migrations/replace-text-component/tests/replace-text-component.input.tsx
Show resolved
Hide resolved
|
Closing in favor of new iteration in #6852 |
Closes #6943 Adds the beginnings of the `@shopify/polaris-migrator` package. This package applies code transformations to help update Polaris apps and the Polaris codebase. ## Usage ```sh npx @shopify/polaris-migrator <migration> <path> ``` Run tests for `@shopify/polaris-migrator` ```sh yarn test --filter=@shopify/polaris-migrator ``` Original PR branch: #6701
Adds the beginnings of the
@shopify/polaris-migratorpackage. This package applies code transformations to help updatePolaris apps and the Polaris codebase.
Usage
Run tests for
@shopify/polaris-migratoryarn test --filter=@shopify/polaris-migrator