Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Renaming and splitting of migrations to new convention #7606

Merged
merged 12 commits into from
Nov 3, 2022
Merged

Renaming and splitting of migrations to new convention #7606

merged 12 commits into from
Nov 3, 2022

Conversation

sam-b-rose
Copy link
Member

@sam-b-rose sam-b-rose commented Nov 1, 2022 •
edited
Loading

WHY are these changes introduced?

Fixes #7477
Fixes #7599

WHAT is this pull request doing?

Updates the current migrations to the following new naming convention with the resulting directory and file structure:

// new naming convention
[type]-[action]-[target]

// migrations
react-rename-component-prop
react-replace-text-components
scss-replace-border
scss-replace-border-radius
scss-replace-border-width
scss-replace-breakpoints
scss-replace-color
scss-replace-duration
scss-replace-easing
scss-replace-font-family
scss-replace-font-size
scss-replace-line-height
scss-replace-spacing
scss-replace-text-emphasis
scss-replace-z-index
styles-tokenize-font
styles-tokenize-motion
styles-tokenize-shape
styles-tokenize-space

polaris-migrator/src/migrations
├── react-rename-component-prop
│   ├── react-rename-component-prop.ts
│   └── tests
│       ├── react-rename-component-prop.input.tsx
│       ├── react-rename-component-prop.output.tsx
│       └── react-rename-component-prop.test.ts
├── react-replace-text-components
│   ├── react-replace-text-components.ts
│   ├── steps
│   │   ├── replace-display-text.ts
│   │   ├── replace-other.ts
│   │   └── replace-text-style.ts
│   └── tests
│       ├── react-replace-text-components.input.tsx
│       ├── react-replace-text-components.output.tsx
│       ├── react-replace-text-components.test.ts
│       ├── with-identifier.input.tsx
│       ├── with-identifier.output.tsx
│       ├── with-relative.input.tsx
│       └── with-relative.output.tsx
├── scss-replace-border
│   ├── scss-replace-border.ts
│   └── tests
│       ├── scss-replace-border.input.scss
│       ├── scss-replace-border.output.scss
│       ├── scss-replace-border.test.ts
│       ├── with-namespace.input.scss
│       └── with-namespace.output.scss
├── scss-replace-border-radius
│   ├── scss-replace-border-radius.ts
│   └── tests
│       ├── scss-replace-border-radius.input.scss
│       ├── scss-replace-border-radius.output.scss
│       ├── scss-replace-border-radius.test.ts
│       ├── with-namespace.input.scss
│       └── with-namespace.output.scss
├── scss-replace-border-width
│   ├── scss-replace-border-width.ts
│   └── tests
│       ├── scss-replace-border-width.input.scss
│       ├── scss-replace-border-width.output.scss
│       ├── scss-replace-border-width.test.ts
│       ├── with-namespace.input.scss
│       └── with-namespace.output.scss
├── scss-replace-breakpoints
│   ├── scss-replace-breakpoints.ts
│   └── tests
│       ├── scss-replace-breakpoints.input.scss
│       ├── scss-replace-breakpoints.output.scss
│       ├── scss-replace-breakpoints.test.ts
│       ├── with-namespace.input.scss
│       └── with-namespace.output.scss
├── scss-replace-color
│   ├── scss-replace-color.ts
│   └── tests
│       ├── scss-replace-color.input.scss
│       ├── scss-replace-color.output.scss
│       ├── scss-replace-color.test.ts
│       ├── with-namespace.input.scss
│       └── with-namespace.output.scss
├── scss-replace-duration
│   ├── scss-replace-duration.ts
│   └── tests
│       ├── scss-replace-duration.input.scss
│       ├── scss-replace-duration.output.scss
│       ├── scss-replace-duration.test.ts
│       ├── with-namespace.input.scss
│       └── with-namespace.output.scss
├── scss-replace-easing
│   ├── scss-replace-easing.ts
│   └── tests
│       ├── scss-replace-easing.input.scss
│       ├── scss-replace-easing.output.scss
│       ├── scss-replace-easing.test.ts
│       ├── with-namespace.input.scss
│       └── with-namespace.output.scss
├── scss-replace-font-family
│   ├── scss-replace-font-family.ts
│   └── tests
│       ├── scss-replace-font-family.input.scss
│       ├── scss-replace-font-family.output.scss
│       ├── scss-replace-font-family.test.ts
│       ├── with-namespace.input.scss
│       └── with-namespace.output.scss
├── scss-replace-font-size
│   ├── scss-replace-font-size.ts
│   └── tests
│       ├── scss-replace-font-size.input.scss
│       ├── scss-replace-font-size.output.scss
│       ├── scss-replace-font-size.test.ts
│       ├── with-namespace.input.scss
│       └── with-namespace.output.scss
├── scss-replace-line-height
│   ├── scss-replace-line-height.ts
│   └── tests
│       ├── scss-replace-line-height.input.scss
│       ├── scss-replace-line-height.output.scss
│       ├── scss-replace-line-height.test.ts
│       ├── with-namespace.input.scss
│       └── with-namespace.output.scss
├── scss-replace-spacing
│   ├── scss-replace-spacing.ts
│   └── tests
│       ├── scss-replace-spacing.input.scss
│       ├── scss-replace-spacing.output.scss
│       ├── scss-replace-spacing.test.ts
│       ├── with-namespace.input.scss
│       └── with-namespace.output.scss
├── scss-replace-text-emphasis
│   ├── scss-replace-text-emphasis.ts
│   └── tests
│       ├── scss-replace-text-emphasis.input.scss
│       ├── scss-replace-text-emphasis.output.scss
│       ├── scss-replace-text-emphasis.test.ts
│       ├── with-namespace.input.scss
│       └── with-namespace.output.scss
├── scss-replace-z-index
│   ├── scss-replace-z-index.ts
│   └── tests
│       ├── scss-replace-z-index.input.scss
│       ├── scss-replace-z-index.output.scss
│       ├── scss-replace-z-index.test.ts
│       ├── with-namespace.input.scss
│       └── with-namespace.output.scss
├── styles-tokenize-font
│   ├── styles-tokenize-font.ts
│   └── tests
│       ├── styles-tokenize-font.input.scss
│       ├── styles-tokenize-font.output.scss
│       ├── styles-tokenize-font.test.ts
│       ├── with-namespace.input.scss
│       └── with-namespace.output.scss
├── styles-tokenize-motion
│   ├── styles-tokenize-motion.ts
│   └── tests
│       ├── styles-tokenize-motion.input.scss
│       ├── styles-tokenize-motion.output.scss
│       └── styles-tokenize-motion.test.ts
├── styles-tokenize-shape
│   ├── styles-tokenize-shape.ts
│   └── tests
│       ├── styles-tokenize-shape.input.scss
│       ├── styles-tokenize-shape.output.scss
│       ├── styles-tokenize-shape.test.ts
│       ├── with-namespace.input.scss
│       └── with-namespace.output.scss
└── styles-tokenize-space
    ├── styles-tokenize-space.ts
    └── tests
        ├── styles-tokenize-space.input.scss
        ├── styles-tokenize-space.output.scss
        ├── styles-tokenize-space.test.ts
        ├── with-namespace.input.scss
        └── with-namespace.output.scss

@sam-b-rose sam-b-rose requested a review from chloerice November 1, 2022 22:08
@sam-b-rose sam-b-rose marked this pull request as draft November 1, 2022 22:11
sam-b-rose and others added 5 commits November 2, 2022 19:21
@sam-b-rose
Initial renaming of migrations to new convention
a9409eb
@chloerice @sam-b-rose
Sass replace border width (#7613)
d31d3ce 
<!--
  ☝️How to write a good PR title:
- Prefix it with [ComponentName] (if applicable), for example: [Button]
  - Start with a verb, for example: Add, Delete, Improve, Fix…
  - Give as much context as necessary and as little as possible
  - Prefix it with [WIP] while it’s a work in progress
-->

### WHY are these changes introduced?

Fixes 

<!--
  Context about the problem that’s being addressed.
-->

### WHAT is this pull request doing?

<!--
  Summary of the changes committed.

Before / after screenshots are appreciated for UI changes. Make sure to
include alt text that describes the screenshot.

If you include an animated gif showing your change, wrapping it in a
details tag is recommended. Gifs usually autoplay, which can cause
accessibility issues for people reviewing your PR:

    <details>
      <summary>Summary of your gif(s)</summary>
      <img src="..." alt="Description of what the gif shows">
    </details>
-->

<!-- ℹ️ Delete the following for small / trivial changes -->

### How to 🎩

🖥 [Local development
instructions](https://github.com/Shopify/polaris/blob/main/README.md#local-development)
🗒 [General tophatting
guidelines](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md)
📄 [Changelog
guidelines](https://github.com/Shopify/polaris/blob/main/.github/CONTRIBUTING.md#changelog)

<!--
  Give as much information as needed to experiment with the component
  in the playground.
-->

<details>
<summary>Copy-paste this code in
<code>playground/Playground.tsx</code>:</summary>

```jsx
import React from 'react';
import {Page} from '../src';

export function Playground() {
  return (
    <Page title="Playground">
      {/* Add the code you want to test in here */}
    </Page>
  );
}
```

</details>

### 🎩 checklist

- [ ] Tested on
[mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing)
- [ ] Tested on [multiple
browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers)
- [ ] Tested for
[accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md)
- [ ] Updated the component's `README.md` with documentation changes
- [ ] [Tophatted
documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md)
changes in the style guide
@sam-b-rose
Rename sass prefix to scss and add react prefix
95d60b2
@sam-b-rose
Split apart font declaration migration
c625e1e
@sam-b-rose
Fix Text component migration tests and add replacement of identifiers
840142e
@sam-b-rose sam-b-rose marked this pull request as ready for review November 3, 2022 04:05
@sam-b-rose sam-b-rose changed the title Initial renaming of migrations to new convention Renaming and splitting of migrations to new convention Nov 3, 2022
@sam-b-rose sam-b-rose self-assigned this Nov 3, 2022
@aaronccasanova aaronccasanova mentioned this pull request Nov 3, 2022
Closed
aaronccasanova
aaronccasanova reviewed Nov 3, 2022
View reviewed changes
polaris-migrator/src/migrations/react-replace-text-components/steps/replace-display-text.ts
Comment on lines +61 to +66
source
.find(j.Identifier)
.filter((path) => path.node.name === localElementName)
.forEach((path) => {
path.node.name = 'Text';
});
Copy link
Member

@aaronccasanova aaronccasanova Nov 3, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I wonder if we should always comment this case. How do we know there isn't an associated object configuring the props e.g.

test(MyComponent, { ...myComponentProps })

Are we just relying on TypeScript to bring these (potential) errors to our attention?

Copy link
Member

Choose a reason for hiding this comment

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

If so, what if a consumer is not using TypeScript?

Copy link
Member Author

Choose a reason for hiding this comment

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

In this scenario with tests, it could be likely or possible the test would fail without fully migrating the appropriate props as well.

But I agree, a comment would be useful. Not sure what we would check for here to trigger the comment 🤔

@sam-b-rose
Merge branch 'main' into rename-migrations
932ad2b 
* main:
  Add flex properties to Inline, use logical property instead of alignY (#7633)
  [polaris.shopify.com] Replace typography with Text component (#7634)
  Add constraints to `stylelint-polaris/coverage` disable comments (#7589)
  Check for target component in Text migrations before modifying file (#7632)
  Replace typography component in stories with Text (#7572)
  Inline remove wrap children in div (#7625)
  Replace typography components with Text (#7530)
  [Layout foundations] Update `AlphaCard` component docs and guidance (#7568)
@sam-b-rose
Update createInlineComment to format text with RegExp
31d6b0b
@sam-b-rose sam-b-rose merged commit cf7badb into main Nov 3, 2022
@sam-b-rose sam-b-rose deleted the rename-migrations branch November 3, 2022 22:47
@github-actions github-actions bot mentioned this pull request Nov 3, 2022
Merged
laurkim pushed a commit that referenced this pull request Nov 4, 2022
@github-actions
Version Packages (#7562)
b6271c4 
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/[email protected]

### Minor Changes

- [#7548](#7548)
[`432bdd5fe`](432bdd5)
Thanks [@anthonymenecola](https://github.com/anthonymenecola)! - add
cancel major icon


- [#7620](#7620)
[`35be8a003`](35be8a0)
Thanks [@rdott](https://github.com/rdott)! - Added inactive location
minor and major icons

## @shopify/[email protected]

### Minor Changes

- [#7403](#7403)
[`8859f5db5`](8859f5d)
Thanks [@jesstelford](https://github.com/jesstelford)! - Introduce
`migrate-motion` migration for migrating `transition`,
`transition-duration`, and `transition-delay` usages of duration values.


- [#7606](#7606)
[`cf7badbd1`](cf7badb)
Thanks [@samrose3](https://github.com/samrose3)! - Renamed and split
migrations based on scope and type (react, scss, and styles)


- [#7543](#7543)
[`8c1989618`](8c19896)
Thanks [@jesstelford](https://github.com/jesstelford)! - Expose
utilities for SASS Migrations to leverage the Suggestion-on-partial-fix
pattern


- [#7529](#7529)
[`3652eb901`](3652eb9)
Thanks [@samrose3](https://github.com/samrose3)! - Add relative option
for replace-text-component migration


- [#7532](#7532)
[`ba576498d`](ba57649)
Thanks [@jesstelford](https://github.com/jesstelford)! - Expose the
.report() method to SASS migrations for easier aggregation of discovered
issues during a migration run.

### Patch Changes

- [#7606](#7606)
[`cf7badbd1`](cf7badb)
Thanks [@samrose3](https://github.com/samrose3)! - Update
`createInlineComment` to format text with RegExp


- [#7606](#7606)
[`cf7badbd1`](cf7badb)
Thanks [@samrose3](https://github.com/samrose3)! - Add support to
replace Identifiers along with JSXIdentifiers for Text migration


- [#7632](#7632)
[`1f2ec8bfe`](1f2ec8b)
Thanks [@samrose3](https://github.com/samrose3)! - Check for targeted
component import before modifying in Text component migration

- Updated dependencies
\[[`6e9edd3b5`](6e9edd3)]:
    -   @shopify/[email protected]

## @shopify/[email protected]

### Minor Changes

- [#7572](#7572)
[`20c8cad81`](20c8cad)
Thanks [@laurkim](https://github.com/laurkim)! - Replaced usage of text
components in component stories with `Text` component


- [#7621](#7621)
[`6e9edd3b5`](6e9edd3)
Thanks [@aveline](https://github.com/aveline)! - - Added border width
prop to `Box`
- Exported color token subset alias types from tokens package and remove
from `Box`


- [#7068](#7068)
[`ccdcea22e`](ccdcea2)
Thanks [@laurkim](https://github.com/laurkim)! - Deprecated
`DisplayText`, `Heading`, `Subheading`, `Caption`, `TextStyle`, and
`VisuallyHidden` components

### Patch Changes

- [#7644](#7644)
[`b3e73ee04`](b3e73ee)
Thanks [@kyledurand](https://github.com/kyledurand)! - Added horizontal
spacing defaults to `Bleed`


- [#7530](#7530)
[`79d92a820`](79d92a8)
Thanks [@samrose3](https://github.com/samrose3)! - Replaced all
typography components with the new `Text` component.
    Added support for `text-inverse` color type on `Text`.
Removed references to the following mixins to use the new `Text` or
tokens directly in classes: `text-style-body`, `text-style-heading`,
`text-style-subheading`, `text-style-caption`, `text-style-button`,
`text-style-button-large`, `text-emphasis-subdued`,
`text-emphasis-strong`, `nav-item-text-attributes`.


- [#7577](#7577)
[`db951f855`](db951f8)
Thanks [@RickyMarou](https://github.com/RickyMarou)! - Page component:
display subtitle even when it's the only header prop set


- [#7633](#7633)
[`1364be7f1`](1364be7)
Thanks [@kyledurand](https://github.com/kyledurand)! - Renamed `alignY`
prop to `alignBlock` on `Inline`
    Added more flex properties to `align` on `Inline`


- [#7443](#7443)
[`7a6fb7c1c`](7a6fb7c)
Thanks [@iAmNathanJ](https://github.com/iAmNathanJ)! - Improve
performance of the Scrollable component with React 18


- [#7625](#7625)
[`9f8b651dd`](9f8b651)
Thanks [@kyledurand](https://github.com/kyledurand)! - Removed wrap
children with div from Inline component


- [#7593](#7593)
[`addd6bcdd`](addd6bc)
Thanks [@kyledurand](https://github.com/kyledurand)! - Improved comments
across layout components, changed default spacing of Inline component to
match AlphaStack


- [#7600](#7600)
[`f006509be`](f006509)
Thanks [@billycai](https://github.com/billycai)! - Add spacing between
title and metadata for Page component


- [#7563](#7563)
[`a9051d678`](a9051d6)
Thanks [@chazdean](https://github.com/chazdean)! - Updated `Inline`
component docs and default prop values


- [#7635](#7635)
[`3cb5377a6`](3cb5377)
Thanks [@iAmNathanJ](https://github.com/iAmNathanJ)! - Fixed Scrollable
component to match existing onScrolledToBottom logic

- Updated dependencies
\[[`432bdd5fe`](432bdd5),
[`6e9edd3b5`](6e9edd3),
[`35be8a003`](35be8a0)]:
    -   @shopify/[email protected]
    -   @shopify/[email protected]

## @shopify/[email protected]

### Minor Changes

- [#7621](#7621)
[`6e9edd3b5`](6e9edd3)
Thanks [@aveline](https://github.com/aveline)! - - Added border width
prop to `Box`
- Exported color token subset alias types from tokens package and remove
from `Box`

## @shopify/[email protected]

### Minor Changes

- [#7551](#7551)
[`d7dc4436f`](d7dc443)
Thanks [@aaronccasanova](https://github.com/aaronccasanova)! - Add
`stylelint-polaris/coverage` rule

### Patch Changes

- [#7589](#7589)
[`b7b0ef5a9`](b7b0ef5)
Thanks [@aaronccasanova](https://github.com/aaronccasanova)! - Add
constraints to `stylelint-polaris/coverage` disable comments

- Updated dependencies
\[[`6e9edd3b5`](6e9edd3)]:
    -   @shopify/[email protected]

## @shopify/[email protected]

### Patch Changes

- Updated dependencies
\[[`8859f5db5`](8859f5d),
[`cf7badbd1`](cf7badb),
[`cf7badbd1`](cf7badb),
[`cf7badbd1`](cf7badb),
[`8c1989618`](8c19896),
[`3652eb901`](3652eb9),
[`1f2ec8bfe`](1f2ec8b),
[`ba576498d`](ba57649)]:
    -   @shopify/[email protected]

## [email protected]

### Minor Changes

- [#7068](#7068)
[`ccdcea22e`](ccdcea2)
Thanks [@laurkim](https://github.com/laurkim)! - Deprecated
`DisplayText`, `Heading`, `Subheading`, `Caption`, `TextStyle`, and
`VisuallyHidden` pages and removed examples


- [#7609](#7609)
[`343865159`](3438651)
Thanks [@sarahill](https://github.com/sarahill)! - Added new type style
guidance and info to typography docs

### Patch Changes

- [#7634](#7634)
[`4db441756`](4db4417)
Thanks [@laurkim](https://github.com/laurkim)! - Replaced usage of
typography components (`DisplayText`, `Heading`, `Subheading`,
`Caption`, `VisuallyHidden`, `TextStyle`) with the new `Text` component


- [#7604](#7604)
[`aa82c82ff`](aa82c82)
Thanks [@chazdean](https://github.com/chazdean)! - Updated `Inline`
component doc vertical alignment example


- [#7568](#7568)
[`ab0cf251f`](ab0cf25)
Thanks [@chazdean](https://github.com/chazdean)! - Updated `AlphaCard`
component guidance and examples


- [#7633](#7633)
[`1364be7f1`](1364be7)
Thanks [@kyledurand](https://github.com/kyledurand)! - Renamed `alignY`
prop to `alignBlock` on `Inline`
    Added more flex properties to `align` on `Inline`


- [#7527](#7527)
[`924e9e5cd`](924e9e5)
Thanks [@chazdean](https://github.com/chazdean)! - Update `Columns`
component docs


- [#7596](#7596)
[`749ee31ee`](749ee31)
Thanks [@kyledurand](https://github.com/kyledurand)! - Fixed home promo
image layout


- [#7563](#7563)
[`a9051d678`](a9051d6)
Thanks [@chazdean](https://github.com/chazdean)! - Updated `Inline`
component docs and default prop values


- [#7566](#7566)
[`567822218`](5678222)
Thanks [@kyledurand](https://github.com/kyledurand)! - Bumped nextjs


- [#7602](#7602)
[`9931ce0b4`](9931ce0)
Thanks [@kyledurand](https://github.com/kyledurand)! - Bumped nextjs to
13.0.1


- [#7571](#7571)
[`4c5ccc8fa`](4c5ccc8)
Thanks [@chazdean](https://github.com/chazdean)! - Updated `Bleed`
component guidance and examples

- Updated dependencies
\[[`b3e73ee04`](b3e73ee),
[`20c8cad81`](20c8cad),
[`79d92a820`](79d92a8),
[`db951f855`](db951f8),
[`432bdd5fe`](432bdd5),
[`6e9edd3b5`](6e9edd3),
[`ccdcea22e`](ccdcea2),
[`1364be7f1`](1364be7),
[`7a6fb7c1c`](7a6fb7c),
[`9f8b651dd`](9f8b651),
[`addd6bcdd`](addd6bc),
[`f006509be`](f006509),
[`a9051d678`](a9051d6),
[`3cb5377a6`](3cb5377),
[`35be8a003`](35be8a0)]:
    -   @shopify/[email protected]
    -   @shopify/[email protected]
    -   @shopify/[email protected]

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aaronccasanova aaronccasanova aaronccasanova approved these changes

@chloerice chloerice Awaiting requested review from chloerice

Assignees

@sam-b-rose sam-b-rose

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Split up replace-border-declarations migration Align communication and documentation of migrations
3 participants
@sam-b-rose @aaronccasanova @chloerice