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

Codemod: remove Section and Items imports if not used elsewhere in file #6908

Merged
merged 9 commits into from
Aug 22, 2024
Merged

Codemod: remove Section and Items imports if not used elsewhere in file #6908

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
1d76772
remove Section or Items import if not used in file
reidbarber Aug 19, 2024
3af3bc6
check scope instead
reidbarber Aug 19, 2024
0033d0e
add new test
reidbarber Aug 19, 2024
679ca19
Merge branch 'main' into codemod-remove-items-sections-imports
reidbarber Aug 19, 2024
f5baae9
cleanup
reidbarber Aug 20, 2024
03febae
Merge branch 'main' into codemod-remove-items-sections-imports
reidbarber Aug 21, 2024
e367422
add extra test
reidbarber Aug 22, 2024
4f39c27
Merge branch 'codemod-remove-items-sections-imports' of https://githu…
reidbarber Aug 22, 2024
eb24843
Merge branch 'main' into codemod-remove-items-sections-imports
snowystinger Aug 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions packages/dev/codemods/src/s1-to-s2/__tests__/__snapshots__/actionmenu.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

exports[`Dynamic - no change 1`] = `
"import { MenuItem, ActionMenu } from "@react-spectrum/s2";
import { Item } from '@adobe/react-spectrum';
let actionMenuItems = [
{name: 'Cut'},
{name: 'Copy'},
Expand All @@ -19,7 +18,6 @@ let actionMenuItems = [

exports[`Static - no change 1`] = `
"import { MenuItem, ActionMenu } from "@react-spectrum/s2";
import { Item } from '@adobe/react-spectrum';

<div>
<ActionMenu>
Expand Down
9 changes: 0 additions & 9 deletions packages/dev/codemods/src/s1-to-s2/__tests__/__snapshots__/breadcrumbs.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

exports[`Comments out autoFocusCurrent 1`] = `
"import { Breadcrumb, Breadcrumbs } from "@react-spectrum/s2";
import { Item } from '@adobe/react-spectrum';

// TODO(S2-upgrade): S2 Breadcrumbs no longer includes a nav element by default. You can wrap the Breadcrumbs component in a nav element if needed.
// TODO(S2-upgrade): autoFocusCurrent has not been implemented yet.
Expand All @@ -15,7 +14,6 @@ import { Item } from '@adobe/react-spectrum';

exports[`Comments out isMultiline 1`] = `
"import { Breadcrumb, Breadcrumbs } from "@react-spectrum/s2";
import { Item } from '@adobe/react-spectrum';

// TODO(S2-upgrade): S2 Breadcrumbs no longer includes a nav element by default. You can wrap the Breadcrumbs component in a nav element if needed.
// TODO(S2-upgrade): isMultiline has not been implemented yet.
Expand All @@ -28,7 +26,6 @@ import { Item } from '@adobe/react-spectrum';

exports[`Comments out showRoot 1`] = `
"import { Breadcrumb, Breadcrumbs } from "@react-spectrum/s2";
import { Item } from '@adobe/react-spectrum';

// TODO(S2-upgrade): S2 Breadcrumbs no longer includes a nav element by default. You can wrap the Breadcrumbs component in a nav element if needed.
// TODO(S2-upgrade): showRoot has not been implemented yet.
Expand All @@ -43,7 +40,6 @@ import { Item } from '@adobe/react-spectrum';

exports[`Dynamic - Renames Item to Breadcrumb 1`] = `
"import { Breadcrumb, Breadcrumbs } from "@react-spectrum/s2";
import { Item } from '@adobe/react-spectrum';
let folders = [
{id: 1, label: 'Home'},
{id: 2, label: 'Trendy'},
Expand All @@ -59,7 +55,6 @@ let folders = [

exports[`Dynamic - Renames key to id 1`] = `
"import { Breadcrumb, Breadcrumbs } from "@react-spectrum/s2";
import { Item } from '@adobe/react-spectrum';
const items = [
{id: 1, name: 'News'},
{id: 2, name: 'Travel'},
Expand All @@ -84,7 +79,6 @@ const items = [

exports[`Leaves a comment if size prop contains "S" 1`] = `
"import { Breadcrumb, Breadcrumbs } from "@react-spectrum/s2";
import { Item } from '@adobe/react-spectrum';

// TODO(S2-upgrade): S2 Breadcrumbs no longer includes a nav element by default. You can wrap the Breadcrumbs component in a nav element if needed.
<Breadcrumbs // TODO(S2-upgrade): size="S" is no longer supported. You'll need to update this manually.
Expand All @@ -96,7 +90,6 @@ size={true ? 'M' : 'S'}>

exports[`Removes size="S" 1`] = `
"import { Breadcrumb, Breadcrumbs } from "@react-spectrum/s2";
import { Item } from '@adobe/react-spectrum';

// TODO(S2-upgrade): S2 Breadcrumbs no longer includes a nav element by default. You can wrap the Breadcrumbs component in a nav element if needed.
<Breadcrumbs>
Expand All @@ -107,7 +100,6 @@ import { Item } from '@adobe/react-spectrum';

exports[`Static - Renames Item to Breadcrumb and adds import 1`] = `
"import { Breadcrumb, Breadcrumbs } from "@react-spectrum/s2";
import { Item } from '@adobe/react-spectrum';

<div>
// TODO(S2-upgrade): S2 Breadcrumbs no longer includes a nav element by default. You can wrap the Breadcrumbs component in a nav element if needed.
Expand All @@ -121,7 +113,6 @@ import { Item } from '@adobe/react-spectrum';

exports[`Static - Renames key to id 1`] = `
"import { Breadcrumb, Breadcrumbs } from "@react-spectrum/s2";
import { Item } from '@adobe/react-spectrum';
let externalKey = 'travel';
<div>
// TODO(S2-upgrade): S2 Breadcrumbs no longer includes a nav element by default. You can wrap the Breadcrumbs component in a nav element if needed.
Expand Down
10 changes: 0 additions & 10 deletions packages/dev/codemods/src/s1-to-s2/__tests__/__snapshots__/combobox.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

exports[`Dynamic - Renames Item to ComboBoxItem 1`] = `
"import { ComboBoxItem, ComboBox } from "@react-spectrum/s2";
import { Item } from '@adobe/react-spectrum';
let options = [
{id: 1, name: 'Aerospace'},
{id: 2, name: 'Mechanical'}
Expand All @@ -18,7 +17,6 @@ let options = [

exports[`Dynamic - Renames key to id 1`] = `
"import { ComboBoxItem, ComboBox } from "@react-spectrum/s2";
import { Item } from '@adobe/react-spectrum';
const items = [
{id: 1, name: 'News'},
{id: 2, name: 'Travel'},
Expand Down Expand Up @@ -46,7 +44,6 @@ const items = [

exports[`Removes isQuiet 1`] = `
"import { ComboBoxItem, ComboBox } from "@react-spectrum/s2";
import { Item } from '@adobe/react-spectrum';
let isQuiet = true;
let props = {isQuiet: true};
<div>
Expand Down Expand Up @@ -80,7 +77,6 @@ let props = {isQuiet: true};

exports[`Removes placeholder 1`] = `
"import { ComboBoxItem, ComboBox } from "@react-spectrum/s2";
import { Item } from '@adobe/react-spectrum';
let placeholder = 'is this actually removed?';
let props = {placeholder: 'is this actually removed?'};
<div>
Expand All @@ -106,7 +102,6 @@ let props = {placeholder: 'is this actually removed?'};

exports[`Static - Converts menuWidth to px value 1`] = `
"import { ComboBoxItem, ComboBox } from "@react-spectrum/s2";
import { Item } from '@adobe/react-spectrum';
let menuWidth = 'size-10';
let props = {menuWidth: 'size-10'};
<div>
Expand Down Expand Up @@ -142,7 +137,6 @@ let props = {menuWidth: 'size-10'};

exports[`Static - Renames Item to ComboBoxItem 1`] = `
"import { ComboBoxItem, ComboBox } from "@react-spectrum/s2";
import { Item } from '@adobe/react-spectrum';
<div>
<ComboBox label="Favorite Animal">
<ComboBoxItem>Red Panda</ComboBoxItem>
Expand All @@ -153,7 +147,6 @@ import { Item } from '@adobe/react-spectrum';

exports[`Static - Renames key to id 1`] = `
"import { ComboBoxItem, ComboBox } from "@react-spectrum/s2";
import { Item } from '@adobe/react-spectrum';
let externalKey = 'travel';
<div>
<ComboBox label="Favorite Animal">
Expand All @@ -165,7 +158,6 @@ let externalKey = 'travel';

exports[`changes validationState to isInvalid or nothing 1`] = `
"import { ComboBoxItem, ComboBox } from "@react-spectrum/s2";
import { Item } from '@adobe/react-spectrum';
let validationState = 'invalid';
let props = {validationState: 'invalid'};
<div>
Expand Down Expand Up @@ -199,7 +191,6 @@ let props = {validationState: 'invalid'};

exports[`changes validationState to isInvalid or nothing 2`] = `
"import { ComboBoxItem, ComboBox } from "@react-spectrum/s2";
import { Item } from '@adobe/react-spectrum';
let validationState = 'invalid';
let props = {validationState: 'invalid'};
<div>
Expand All @@ -226,7 +217,6 @@ let props = {validationState: 'invalid'};

exports[`handles sections 1`] = `
"import { ComboBoxSection, ComboBoxItem, ComboBox } from "@react-spectrum/s2";
import { Section, Item } from '@adobe/react-spectrum';
<ComboBox>
<ComboBoxSection><Header>Section title</Header>
<ComboBoxItem>Item one</ComboBoxItem>
Expand Down
19 changes: 18 additions & 1 deletion packages/dev/codemods/src/s1-to-s2/__tests__/__snapshots__/imports.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,24 @@ import { StatusLight } from "@react-spectrum/s2";
</>"
`;

exports[`should remove unused Item/Section import even if name taken in different scope 1`] = `
"import { MenuSection, MenuItem, Menu } from "@react-spectrum/s2";

function foo() {
let Item = 'something else';
let Section = 'something else';
}

<div>
<Menu aria-label="Text">
<MenuSection><Header>Styles</Header>
<MenuItem id="bold">Bold</MenuItem>
<MenuItem id="underline">Underline</MenuItem>
</MenuSection>
</Menu>
</div>"
`;

exports[`should replace named imports 1`] = `
"import { Button } from "@react-spectrum/s2";

Expand All @@ -114,7 +132,6 @@ exports[`should replace named imports from individual packages 1`] = `

exports[`should use unique alias if newly imported component name is already in scope 1`] = `
"import { Tag as Tag1, TagGroup } from "@react-spectrum/s2";
import { Item } from '@adobe/react-spectrum';

const Tag = 'something else';

Expand Down
6 changes: 0 additions & 6 deletions packages/dev/codemods/src/s1-to-s2/__tests__/__snapshots__/menu.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

exports[`Dynamic - Renames Item to MenuItem with Submenu, Section to MenuSection 1`] = `
"import { MenuItem, MenuSection, Menu, MenuTrigger, SubmenuTrigger, Button } from "@react-spectrum/s2";
import { Item, Section } from '@adobe/react-spectrum';
const items = [
{ id: 'copy', name: 'Copy' },
{ id: 'cut', name: 'Cut' },
Expand Down Expand Up @@ -68,7 +67,6 @@ const items = [

exports[`Dynamic - Renames key to id 1`] = `
"import { MenuItem, Menu, MenuTrigger, SubmenuTrigger, Button } from "@react-spectrum/s2";
import { Item } from '@adobe/react-spectrum';
Copy link
Member

Choose a reason for hiding this comment

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

This is a lot of cases where we're removing the Item or Section from tests. This seems like a great change.
I found the two following tests where we aren't able to determine the "item" and leave the imports. Isn't there a third "leave the import" case where someone does one section that is static and a second section that is dynamic? Like the following.

<MenuTrigger>
    <Button>Edit</Button>
    <Menu>
      <Section key={section.key} title="Static Menu Items">
        <Item key="cut">Cut</Item>
        <Item key="copy">Copy</Item>
      </Section> 
      <Section key={section.key} title="Social Media Sites">
        {items}
      </Section

    </Menu>
  </MenuTrigger>

Copy link
Member Author

Choose a reason for hiding this comment

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

Just tried this locally, and it kept the Item import. Are you seeing something different?

Before:

import {Menu, Section, Item, MenuTrigger, Button} from '@adobe/react-spectrum';

let items = [
  {key: 'facebook', name: 'Facebook'},
  {key: 'twitter', name: 'Twitter'},
  {key: 'instagram', name: 'Instagram'},
].map(item => <Item key={item.key}>{item.name}</Item>);

<div>
  <MenuTrigger>
      <Button>Edit</Button>
      <Menu>
        <Section key={section.key} title="Static Menu Items">
          <Item key="cut">Cut</Item>
          <Item key="copy">Copy</Item>
        </Section> 
        <Section key={section.key} title="Social Media Sites">
          {items}
        </Section>
      </Menu>
    </MenuTrigger>
</div>

After:

import { MenuSection, MenuItem, Menu, MenuTrigger, Button } from "@react-spectrum/s2";
import { Item } from '@adobe/react-spectrum';

let items = [
  {key: 'facebook', name: 'Facebook'},
  {key: 'twitter', name: 'Twitter'},
  {key: 'instagram', name: 'Instagram'},
].map(item => // TODO(S2-upgrade): Couldn't automatically detect what type of collection component this is rendered in. You'll need to update this manually.
<Item key={item.key}>{item.name}</Item>);

<div>
  <MenuTrigger>
      <Button>Edit</Button>
      <Menu>
        <MenuSection id={section.key}><Header>Static Menu Items</Header>
          <MenuItem id="cut">Cut</MenuItem>
          <MenuItem id="copy">Copy</MenuItem>
        </MenuSection> 
        <MenuSection id={section.key}><Header>Social Media Sites</Header>
          {items}
        </MenuSection>
      </Menu>
    </MenuTrigger>
</div>

Copy link
Member

Choose a reason for hiding this comment

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

I just wanted that in a test. :)

const items = [
{ key: 'copy', name: 'Copy' },
{ key: 'cut', name: 'Cut' },
Expand Down Expand Up @@ -164,8 +162,6 @@ exports[`Static - Renames Item to MenuItem, Section to MenuSection 1`] = `
Header,
Heading,
} from "@react-spectrum/s2";

import { Item, Section } from '@adobe/react-spectrum';
<div>
<MenuTrigger>
<Button>Edit</Button>
Expand Down Expand Up @@ -199,8 +195,6 @@ exports[`Static - Renames key to id 1`] = `
Header,
Heading,
} from "@react-spectrum/s2";

import { Item, Section } from '@adobe/react-spectrum';
<div>
<MenuTrigger>
<Button>Edit</Button>
Expand Down
2 changes: 0 additions & 2 deletions packages/dev/codemods/src/s1-to-s2/__tests__/__snapshots__/multi-collection.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,6 @@ exports[`Static - Renames Item to Breadcrumb and adds import 1`] = `
Heading,
} from "@react-spectrum/s2";

import { Item, Section } from '@adobe/react-spectrum';

<div>
// TODO(S2-upgrade): S2 Breadcrumbs no longer includes a nav element by default. You can wrap the Breadcrumbs component in a nav element if needed.
<Breadcrumbs>
Expand Down
7 changes: 0 additions & 7 deletions packages/dev/codemods/src/s1-to-s2/__tests__/__snapshots__/picker.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

exports[`Dynamic - Renames Item to PickerItem 1`] = `
"import { PickerItem, Picker } from "@react-spectrum/s2";
import { Item } from '@adobe/react-spectrum';
let options = [
{id: 1, name: 'Chocolate'},
{id: 2, name: 'Vanilla'}
Expand All @@ -28,7 +27,6 @@ let options = [

exports[`Static - Converts menuWidth to px value 1`] = `
"import { PickerItem, Picker } from "@react-spectrum/s2";
import { Item } from '@adobe/react-spectrum';
let menuWidth = 'size-10';
let props = {menuWidth: 'size-10'};
<div>
Expand Down Expand Up @@ -64,7 +62,6 @@ let props = {menuWidth: 'size-10'};

exports[`Static - Removes isQuiet 1`] = `
"import { PickerItem, Picker } from "@react-spectrum/s2";
import { Item } from '@adobe/react-spectrum';
let isQuiet = true;
let props = {isQuiet: true};
<div>
Expand Down Expand Up @@ -94,7 +91,6 @@ let props = {isQuiet: true};

exports[`Static - Renames Item to PickerItem 1`] = `
"import { PickerItem, Picker } from "@react-spectrum/s2";
import { Item } from '@adobe/react-spectrum';
<div>
<Picker label="Ice Cream">
<PickerItem>Chocolate</PickerItem>
Expand All @@ -105,7 +101,6 @@ import { Item } from '@adobe/react-spectrum';

exports[`Static - Renames key to id 1`] = `
"import { PickerItem, Picker } from "@react-spectrum/s2";
import { Item } from '@adobe/react-spectrum';
<div>
<Picker label="Ice Cream">
<PickerItem id="chocolate">Chocolate</PickerItem>
Expand All @@ -116,7 +111,6 @@ import { Item } from '@adobe/react-spectrum';

exports[`changes validationState to isInvalid or nothing 1`] = `
"import { PickerItem, Picker } from "@react-spectrum/s2";
import { Item } from '@adobe/react-spectrum';
let validationState = 'invalid';
let props = {validationState: 'invalid'};
<div>
Expand Down Expand Up @@ -147,7 +141,6 @@ let props = {validationState: 'invalid'};

exports[`handles sections 1`] = `
"import { PickerSection, PickerItem, Picker } from "@react-spectrum/s2";
import { Section, Item } from '@adobe/react-spectrum';
<Picker>
<PickerSection><Header>Section title</Header>
<PickerItem>Item one</PickerItem>
Expand Down
3 changes: 0 additions & 3 deletions packages/dev/codemods/src/s1-to-s2/__tests__/__snapshots__/tabs.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ let items = [

exports[`Remove isEmphasized 1`] = `
"import { Tab, TabPanel, Tabs, TabList } from "@react-spectrum/s2";
import { Item } from '@adobe/react-spectrum';
let isEmphasized = true;
let props = {isEmphasized: true};
<div>
Expand Down Expand Up @@ -65,7 +64,6 @@ let props = {isEmphasized: true};

exports[`Remove isQuiet 1`] = `
"import { Tab, TabPanel, Tabs, TabList } from "@react-spectrum/s2";
import { Item } from '@adobe/react-spectrum';
let isQuiet = true;
let props = {isQuiet: true};
<div>
Expand Down Expand Up @@ -100,7 +98,6 @@ let props = {isQuiet: true};

exports[`Update to use new API 1`] = `
"import { Tab, TabPanel, Tabs, TabList } from "@react-spectrum/s2";
import { Item } from '@adobe/react-spectrum';

<Tabs aria-label="History of Ancient Rome"><TabList>
<Tab id="FoR">Founding of Rome</Tab>
Expand Down
5 changes: 0 additions & 5 deletions packages/dev/codemods/src/s1-to-s2/__tests__/__snapshots__/taggroup.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

exports[`Dynamic - Renames Item to Tag 1`] = `
"import { Tag, TagGroup } from "@react-spectrum/s2";
import { Item } from '@adobe/react-spectrum';
const items = [
{id: 1, name: 'News'},
{id: 2, name: 'Travel'},
Expand All @@ -19,7 +18,6 @@ const items = [

exports[`Dynamic - Renames key to id 1`] = `
"import { Tag, TagGroup } from "@react-spectrum/s2";
import { Item } from '@adobe/react-spectrum';
const items = [
{id: 1, name: 'News'},
{id: 2, name: 'Travel'},
Expand All @@ -42,7 +40,6 @@ const items = [

exports[`Static - Renames Item to Tag 1`] = `
"import { Tag, TagGroup } from "@react-spectrum/s2";
import { Item } from '@adobe/react-spectrum';

<div>
<TagGroup aria-label="Static TagGroup items example">
Expand All @@ -56,7 +53,6 @@ import { Item } from '@adobe/react-spectrum';

exports[`Static - Renames key to id 1`] = `
"import { Tag, TagGroup } from "@react-spectrum/s2";
import { Item } from '@adobe/react-spectrum';

let externalKey = 'travel';
<div>
Expand All @@ -71,7 +67,6 @@ let externalKey = 'travel';

exports[`changes validationState to isInvalid or nothing 1`] = `
"import { Tag, TagGroup } from "@react-spectrum/s2";
import { Item } from '@adobe/react-spectrum';

let validationState = 'invalid';
let props = {validationState: 'invalid'};
Expand Down
18 changes: 18 additions & 0 deletions packages/dev/codemods/src/s1-to-s2/__tests__/imports.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,3 +127,21 @@ import {Button} from "@react-spectrum/s2";
<StatusLight variant="positive">Test</StatusLight>
</>
`);

test('should remove unused Item/Section import even if name taken in different scope', `
import {Menu, Section, Item} from '@adobe/react-spectrum';

function foo() {
let Item = 'something else';
let Section = 'something else';
}

<div>
<Menu aria-label="Text">
<Section title="Styles">
<Item key="bold">Bold</Item>
<Item key="underline">Underline</Item>
</Section>
</Menu>
</div>
`);
Loading