Skip to content

Conversation

isaacbowen
Copy link
Contributor

WHY are these changes introduced?

Currently, npx @shopify/polaris-migrator react-replace-text-components transforms <DisplayText> into <Text as="p">. On build, this results in a type error:

Type error: Property 'variant' is missing in type '{ children: string; as: "p"; }' but required in type 'TextProps'.

The error is sane; the issue is that the migrator doesn't have a default case for DisplayText elements that omit the (optional) size attribute.

WHAT is this pull request doing?

Adding a default case for DisplayText components that don't have a size. With this change, <DisplayText> becomes <Text variant="headingXl" as="p">, which does successfully build.

@github-actions github-actions bot added the cla-needed Added by a bot. Contributor needs to sign the CLA Agreement. label Nov 9, 2022
@github-actions github-actions bot removed the cla-needed Added by a bot. Contributor needs to sign the CLA Agreement. label Nov 9, 2022
@isaacbowen
Copy link
Contributor Author

@samrose3 Tagging you since it looks like you were in here recently, and maybe you'll know what should happen next? Also hello from an until-very-recently Denverite!

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 bug catch and clean fix. Thanks for the contribution, @isaacbowen! ⛰️ 🏂 ⛷️

Going to go ahead and approve, but would be awesome if you could add a simple test case to the react-replace-text-components.input.tsx and react-replace-text-components.output.tsx 💯

@sam-b-rose
Copy link
Member

Also, could you add a changeset as well bumping this package. A patch update should be good 👍 Run the following command from the root directory to create a new changeset:

yarn changeset

@isaacbowen isaacbowen changed the title Add the (missing) case when migrating DisplayText to Text when size is not specified Add the (missing) case for migrating DisplayText to Text when size is not specified Nov 9, 2022
@isaacbowen
Copy link
Contributor Author

Going to go ahead and approve, but would be awesome if you could add a simple test case to the react-replace-text-components.input.tsx and react-replace-text-components.output.tsx 💯

Done in 4c47887!

Also, could you add a changeset as well bumping this package. A patch update should be good 👍

Done in 788c3ae! First time using a system like this, so obvs let me know if I've botched it. :)

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.

Looks good! Thanks for the updates 🚀

@sam-b-rose sam-b-rose merged commit 566dc20 into Shopify:main Nov 9, 2022
@isaacbowen isaacbowen deleted the default-displaytext-size branch November 9, 2022 03:18
@sam-b-rose
Copy link
Member

Thank you again for the contribution @isaacbowen This fix should be available on the next release once the Version Packages PR is merged.

@github-actions github-actions bot mentioned this pull request Nov 9, 2022
@isaacbowen
Copy link
Contributor Author

Wow, this turned around so much more quickly than I think I expected. :D Thanks a million, @samrose3! Hugely appreciate your time. :)

laurkim pushed a commit that referenced this pull request Nov 15, 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.9.0

### Minor Changes

- [#7648](#7648)
[`c08780522`](c087805)
Thanks [@samrose3](https://github.com/samrose3)! - Added `animation`
properties to tokenize motion migration

### Patch Changes

- [#7690](#7690)
[`2ce4d38a2`](2ce4d38)
Thanks [@samrose3](https://github.com/samrose3)! - Ignore replacing type
imports and warn when encountering unknown props for Text component
migration


- [#7679](#7679)
[`566dc20c2`](566dc20)
Thanks [@isaacbowen](https://github.com/isaacbowen)! - Add the (missing)
case for migrating DisplayText to Text when size is not specified

## @shopify/polaris@10.12.0

### Minor Changes

- [#7684](#7684)
[`2e5f8348b`](2e5f834)
Thanks [@ananyaneogi](https://github.com/ananyaneogi)! - Update the
`CalloutCard` `title` prop to accept a ReactNode type


- [#7588](#7588)
[`c1c8f73b0`](c1c8f73)
Thanks [@rcaplanshopify](https://github.com/rcaplanshopify)! - Added
optional `captureOverscroll` prop to `Popover`


- [#6986](#6986)
[`270887fcf`](270887f)
Thanks [@danielle-dsouza](https://github.com/danielle-dsouza)! -
Clicking on the modal backdrop triggers the pressed state of the modal's
close button

### Patch Changes

- [#7693](#7693)
[`bdf6fd31d`](bdf6fd3)
Thanks [@philschoefer](https://github.com/philschoefer)! - Fixed a bug
where `DataTable` summary row would not properly inherit type defined in
`columnContentTypes` prop


- [#7578](#7578)
[`217f2f8ed`](217f2f8)
Thanks [@haskelash-shopify](https://github.com/haskelash-shopify)! -
Hide bulk actions popup when all items are deselected.


- [#7710](#7710)
[`6b915e01e`](6b915e0)
Thanks [@laurkim](https://github.com/laurkim)! - Fixed visual spacing
regressions on `ActionList` and `Modal.Header`

## @shopify/plugin-polaris@0.0.17

### Patch Changes

- Updated dependencies
\[[`2ce4d38a2`](2ce4d38),
[`c08780522`](c087805),
[`566dc20c2`](566dc20)]:
    -   @shopify/polaris-migrator@0.9.0

## polaris.shopify.com@0.25.0

### Minor Changes

- [#7721](#7721)
[`f490f8e7c`](f490f8e)
Thanks [@jjgali](https://github.com/jjgali)! - Updated and realigned
values in Unit of measurement abbreviations table


- [#7661](#7661)
[`e1a3c62d2`](e1a3c62)
Thanks [@jjgali](https://github.com/jjgali)! - Added keywords to Naming
page.


- [#7712](#7712)
[`88e1d875f`](88e1d87)
Thanks [@jjgali](https://github.com/jjgali)! - Fixed abbrevation for
pounds from lbs to lb.

### Patch Changes

- [#7711](#7711)
[`3984f26b6`](3984f26)
Thanks [@nayeobkim](https://github.com/nayeobkim)! - Updated the related
components copy in `AlphaStack`


- [#7569](#7569)
[`ffe1e9230`](ffe1e92)
Thanks [@chazdean](https://github.com/chazdean)! - Updated `Tiles`
component guidance and examples


- [#7698](#7698)
[`f0e75bd0c`](f0e75bd)
Thanks [@nayeobkim](https://github.com/nayeobkim)! - Updated the Related
components links in Columns


- [#7713](#7713)
[`b649af84f`](b649af8)
Thanks [@nayeobkim](https://github.com/nayeobkim)! - Updated the related
components links in `Tiles`


- [#7594](#7594)
[`faf3dc646`](faf3dc6)
Thanks [@chazdean](https://github.com/chazdean)! - Updated `Box`
component guidance and examples


- [#7659](#7659)
[`d28259746`](d282597)
Thanks [@kyledurand](https://github.com/kyledurand)! - Added global
styles from deps instead of linking unpkg

- Updated dependencies
\[[`bdf6fd31d`](bdf6fd3),
[`217f2f8ed`](217f2f8),
[`2e5f8348b`](2e5f834),
[`c1c8f73b0`](c1c8f73),
[`270887fcf`](270887f),
[`6b915e01e`](6b915e0)]:
    -   @shopify/polaris@10.12.0

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
juzser pushed a commit to juzser/polaris that referenced this pull request Jul 27, 2023
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.9.0

### Minor Changes

- [Shopify#7648](Shopify#7648)
[`c08780522`](Shopify@c087805)
Thanks [@samrose3](https://github.com/samrose3)! - Added `animation`
properties to tokenize motion migration

### Patch Changes

- [Shopify#7690](Shopify#7690)
[`2ce4d38a2`](Shopify@2ce4d38)
Thanks [@samrose3](https://github.com/samrose3)! - Ignore replacing type
imports and warn when encountering unknown props for Text component
migration


- [Shopify#7679](Shopify#7679)
[`566dc20c2`](Shopify@566dc20)
Thanks [@isaacbowen](https://github.com/isaacbowen)! - Add the (missing)
case for migrating DisplayText to Text when size is not specified

## @shopify/polaris@10.12.0

### Minor Changes

- [Shopify#7684](Shopify#7684)
[`2e5f8348b`](Shopify@2e5f834)
Thanks [@ananyaneogi](https://github.com/ananyaneogi)! - Update the
`CalloutCard` `title` prop to accept a ReactNode type


- [Shopify#7588](Shopify#7588)
[`c1c8f73b0`](Shopify@c1c8f73)
Thanks [@rcaplanshopify](https://github.com/rcaplanshopify)! - Added
optional `captureOverscroll` prop to `Popover`


- [Shopify#6986](Shopify#6986)
[`270887fcf`](Shopify@270887f)
Thanks [@danielle-dsouza](https://github.com/danielle-dsouza)! -
Clicking on the modal backdrop triggers the pressed state of the modal's
close button

### Patch Changes

- [Shopify#7693](Shopify#7693)
[`bdf6fd31d`](Shopify@bdf6fd3)
Thanks [@philschoefer](https://github.com/philschoefer)! - Fixed a bug
where `DataTable` summary row would not properly inherit type defined in
`columnContentTypes` prop


- [Shopify#7578](Shopify#7578)
[`217f2f8ed`](Shopify@217f2f8)
Thanks [@haskelash-shopify](https://github.com/haskelash-shopify)! -
Hide bulk actions popup when all items are deselected.


- [Shopify#7710](Shopify#7710)
[`6b915e01e`](Shopify@6b915e0)
Thanks [@laurkim](https://github.com/laurkim)! - Fixed visual spacing
regressions on `ActionList` and `Modal.Header`

## @shopify/plugin-polaris@0.0.17

### Patch Changes

- Updated dependencies
\[[`2ce4d38a2`](Shopify@2ce4d38),
[`c08780522`](Shopify@c087805),
[`566dc20c2`](Shopify@566dc20)]:
    -   @shopify/polaris-migrator@0.9.0

## polaris.shopify.com@0.25.0

### Minor Changes

- [Shopify#7721](Shopify#7721)
[`f490f8e7c`](Shopify@f490f8e)
Thanks [@jjgali](https://github.com/jjgali)! - Updated and realigned
values in Unit of measurement abbreviations table


- [Shopify#7661](Shopify#7661)
[`e1a3c62d2`](Shopify@e1a3c62)
Thanks [@jjgali](https://github.com/jjgali)! - Added keywords to Naming
page.


- [Shopify#7712](Shopify#7712)
[`88e1d875f`](Shopify@88e1d87)
Thanks [@jjgali](https://github.com/jjgali)! - Fixed abbrevation for
pounds from lbs to lb.

### Patch Changes

- [Shopify#7711](Shopify#7711)
[`3984f26b6`](Shopify@3984f26)
Thanks [@nayeobkim](https://github.com/nayeobkim)! - Updated the related
components copy in `AlphaStack`


- [Shopify#7569](Shopify#7569)
[`ffe1e9230`](Shopify@ffe1e92)
Thanks [@chazdean](https://github.com/chazdean)! - Updated `Tiles`
component guidance and examples


- [Shopify#7698](Shopify#7698)
[`f0e75bd0c`](Shopify@f0e75bd)
Thanks [@nayeobkim](https://github.com/nayeobkim)! - Updated the Related
components links in Columns


- [Shopify#7713](Shopify#7713)
[`b649af84f`](Shopify@b649af8)
Thanks [@nayeobkim](https://github.com/nayeobkim)! - Updated the related
components links in `Tiles`


- [Shopify#7594](Shopify#7594)
[`faf3dc646`](Shopify@faf3dc6)
Thanks [@chazdean](https://github.com/chazdean)! - Updated `Box`
component guidance and examples


- [Shopify#7659](Shopify#7659)
[`d28259746`](Shopify@d282597)
Thanks [@kyledurand](https://github.com/kyledurand)! - Added global
styles from deps instead of linking unpkg

- Updated dependencies
\[[`bdf6fd31d`](Shopify@bdf6fd3),
[`217f2f8ed`](Shopify@217f2f8),
[`2e5f8348b`](Shopify@2e5f834),
[`c1c8f73b0`](Shopify@c1c8f73),
[`270887fcf`](Shopify@270887f),
[`6b915e01e`](Shopify@6b915e0)]:
    -   @shopify/polaris@10.12.0

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants