Skip to content

Commit

Permalink
Fix rename prop migration (Shopify#10215)
Browse files Browse the repository at this point in the history
### WHY are these changes introduced?

`react-rename-component-prop` migration was not working as expected when
there were multiple props on a component

<img width="552" alt="Screenshot 2023-08-24 at 10 12 10 AM"
src="https://github.com/Shopify/polaris/assets/3474483/5ca5de71-f587-49cf-87ea-5fc73d298864">

### WHAT is this pull request doing?

- Adds more test cases
- Fixes migration

<img width="590" alt="Screenshot 2023-08-24 at 10 12 35 AM"
src="https://github.com/Shopify/polaris/assets/3474483/8dc1ee41-1900-4383-81e0-7635b2d2fa0b">

---------

Co-authored-by: Aaron Casanova <32409546+aaronccasanova@users.noreply.github.com>
  • Loading branch information
aveline and aaronccasanova committed Aug 25, 2023
1 parent 2deb216 commit 3d4ef92
Show file tree
Hide file tree
Showing 11 changed files with 61 additions and 91 deletions.
5 changes: 5 additions & 0 deletions .changeset/odd-owls-know.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/polaris-migrator': patch
---

Fixed a bug in the rename prop migration
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import React from 'react';

declare function MyComponent(props: any): JSX.Element;

export function App() {
return (
<MyComponent foo="bar" booleanProp>
Hello world
</MyComponent>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import React from 'react';

declare function MyComponent(props: any): JSX.Element;

export function App() {
return (
<MyComponent foo="bar" variant="boolean-prop-value">
Hello world
</MyComponent>
);
}
Original file line number Diff line number Diff line change
@@ -1,17 +1,7 @@
import React from 'react';

interface MyComponentProps {
prop?: string;
newProp?: string;
children?: React.ReactNode;
}

const Child = (props: {prop: string}) => <>{props.prop}</>;

function MyComponent(props: MyComponentProps) {
const value = props.newProp ?? props.prop;
return <div data-prop={value}>{props.children}</div>;
}
declare function MyComponent(props: any): JSX.Element;
declare function Child(props: any): JSX.Element;

export function App() {
return (
Expand Down
Original file line number Diff line number Diff line change
@@ -1,21 +1,11 @@
import React from 'react';

interface MyComponentProps {
prop?: string;
newProp?: string;
children?: React.ReactNode;
}

const Child = (props: {prop: string}) => <>{props.prop}</>;

function MyComponent(props: MyComponentProps) {
const value = props.newProp ?? props.prop;
return <div data-prop={value}>{props.children}</div>;
}
declare function MyComponent(props: any): JSX.Element;
declare function Child(props: any): JSX.Element;

export function App() {
return (
<MyComponent newProp="new value">
<MyComponent newProp="new-value">
Hello
<Child prop="value" />
</MyComponent>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,21 +1,11 @@
import React from 'react';

interface MyComponentProps {
prop?: string;
newProp?: string;
children?: React.ReactNode;
}

const Child = (props: {prop: string}) => <>{props.prop}</>;

function MyComponent(props: MyComponentProps) {
const value = props.newProp || props.prop;
return <div data-prop={value}>{props.children}</div>;
}
declare function MyComponent(props: any): JSX.Element;
declare function Child(props: any): JSX.Element;

export function App() {
return (
<MyComponent prop="value">
<MyComponent prop="value" foo="bar">
Hello
<Child prop="value" />
</MyComponent>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,21 +1,11 @@
import React from 'react';

interface MyComponentProps {
prop?: string;
newProp?: string;
children?: React.ReactNode;
}

const Child = (props: {prop: string}) => <>{props.prop}</>;

function MyComponent(props: MyComponentProps) {
const value = props.newProp || props.prop;
return <div data-prop={value}>{props.children}</div>;
}
declare function MyComponent(props: any): JSX.Element;
declare function Child(props: any): JSX.Element;

export function App() {
return (
<MyComponent newProp="value">
<MyComponent newProp="value" foo="bar">
Hello
<Child prop="value" />
</MyComponent>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,30 +1,17 @@
import React from 'react';

interface MyComponentProps {
prop?: string;
newProp?: string;
children?: React.ReactNode;
}

const Child = (props: {prop: string}) => <>{props.prop}</>;

function MyComponent(props: MyComponentProps) {
const value = props.newProp ?? props.prop;
return <div data-prop={value}>{props.children}</div>;
}
declare function MyComponent(props: any): JSX.Element;
declare function CompoundComponent(props: any): JSX.Element;
declare function Child(props: any): JSX.Element;

function SubComponent({...props}: any) {
return <div {...props} />;
}
MyComponent.CompoundComponent = CompoundComponent;

export function App() {
return (
<MyComponent>
<MyComponent.SubComponent prop="value" />
<MyComponent.CompoundComponent prop="value" />
Hello
<Child prop="value" />
</MyComponent>
);
}

MyComponent.SubComponent = SubComponent;
Original file line number Diff line number Diff line change
@@ -1,30 +1,17 @@
import React from 'react';

interface MyComponentProps {
prop?: string;
newProp?: string;
children?: React.ReactNode;
}

const Child = (props: {prop: string}) => <>{props.prop}</>;

function MyComponent(props: MyComponentProps) {
const value = props.newProp ?? props.prop;
return <div data-prop={value}>{props.children}</div>;
}
declare function MyComponent(props: any): JSX.Element;
declare function CompoundComponent(props: any): JSX.Element;
declare function Child(props: any): JSX.Element;

function SubComponent({...props}: any) {
return <div {...props} />;
}
MyComponent.CompoundComponent = CompoundComponent;

export function App() {
return (
<MyComponent>
<MyComponent.SubComponent newProp="new value" />
<MyComponent.CompoundComponent newProp="new-value" />
Hello
<Child prop="value" />
</MyComponent>
);
}

MyComponent.SubComponent = SubComponent;
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,25 @@ const fixtures = [
componentName: 'MyComponent',
from: 'prop',
to: 'newProp',
newValue: 'new value',
newValue: 'new-value',
},
},
{
name: 'react-rename-compound-component-prop-with-new-value',
options: {
componentName: 'MyComponent.SubComponent',
componentName: 'MyComponent.CompoundComponent',
from: 'prop',
to: 'newProp',
newValue: 'new value',
newValue: 'new-value',
},
},
{
name: 'react-rename-component-prop-with-boolean',
options: {
componentName: 'MyComponent',
from: 'booleanProp',
to: 'variant',
newValue: 'boolean-prop-value',
},
},
];
Expand Down
2 changes: 1 addition & 1 deletion polaris-migrator/src/utilities/jsx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ export function renameProps(
const isFromProp = (prop: unknown): prop is keyof typeof props =>
Object.keys(props).includes(prop as string);

if (node.type !== 'JSXAttribute' && !isFromProp(node.name.name)) {
if (!(node.type === 'JSXAttribute' && isFromProp(node.name.name))) {
return node;
}

Expand Down

0 comments on commit 3d4ef92

Please sign in to comment.