Skip to content

Conversation

@joshblack
Copy link
Member

@joshblack joshblack commented Sep 6, 2022

Update circular imports in the project by either updating specific import statements or moving shared code into dedicated modules.

Generally, import statements were rewritten from importing directly from the index.ts file and instead import from the module directly.

There were a handful of scenarios where a cycle was created from the following situation:

  • An item exports a value (like its props)
  • An item imports a subitem
  • The subitem also imports from item

In these situations, I moved the shared code into a discrete module. I'd love feedback on what the best way to group or name these types of code would be 👀 For shared context values, I ended up creating a standalone file for the context. For ActionList, I ended up just using a shared.ts file but it feels like a mixed bag of exports that aren't really related.

Note: these changes only include circular dependencies for code and does not include types

List of circular imports
  • src/index.ts -> src/PageLayout/index.ts -> src/PageLayout/PageLayout.tsx -> src/index.ts
  • src/index.ts -> src/Radio.tsx -> src/RadioGroup.tsx -> src/_CheckboxOrRadioGroup/index.ts -> src/_CheckboxOrRadioGroup/CheckboxOrRadioGroup.tsx -> src/index.ts
  • src/index.ts -> src/Radio.tsx -> src/RadioGroup.tsx -> src/_CheckboxOrRadioGroup/index.ts -> src/_CheckboxOrRadioGroup/CheckboxOrRadioGroup.tsx -> src/_ValidationAnimationContainer.tsx -> src/index.ts
  • src/index.ts -> src/Radio.tsx -> src/RadioGroup.tsx -> src/_CheckboxOrRadioGroup/index.ts -> src/_CheckboxOrRadioGroup/CheckboxOrRadioGroup.tsx -> src/_CheckboxOrRadioGroup/_CheckboxOrRadioGroupCaption.tsx -> src/index.ts
  • src/index.ts -> src/Radio.tsx -> src/RadioGroup.tsx -> src/_CheckboxOrRadioGroup/index.ts -> src/_CheckboxOrRadioGroup/CheckboxOrRadioGroup.tsx -> src/_CheckboxOrRadioGroup/_CheckboxOrRadioGroupLabel.tsx -> src/index.ts
  • src/index.ts -> src/Radio.tsx -> src/RadioGroup.tsx -> src/_CheckboxOrRadioGroup/index.ts -> src/_CheckboxOrRadioGroup/CheckboxOrRadioGroup.tsx -> src/_CheckboxOrRadioGroup/_CheckboxOrRadioGroupValidation.tsx -> src/_InputValidation.tsx -> src/index.ts
  • src/ActionList/Item.tsx -> src/ActionList/Selection.tsx -> src/ActionList/Visuals.tsx -> src/ActionList/Item.tsx
  • src/index.ts -> src/Autocomplete/index.ts -> src/Autocomplete/Autocomplete.tsx -> src/Autocomplete/AutocompleteInput.tsx -> src/TextInput.tsx -> src/_TextInputInnerVisualSlot.tsx -> src/index.ts
  • src/index.ts -> src/Autocomplete/index.ts -> src/Autocomplete/Autocomplete.tsx -> src/Autocomplete/AutocompleteInput.tsx -> src/TextInput.tsx -> src/_TextInputInnerAction.tsx -> src/index.ts
  • src/index.ts -> src/Autocomplete/index.ts -> src/Autocomplete/Autocomplete.tsx -> src/Autocomplete/AutocompleteMenu.tsx -> src/index.ts
  • src/index.ts -> src/AvatarPair.tsx -> src/index.ts
  • src/index.ts -> src/AvatarStack.tsx -> src/index.ts
  • src/index.ts -> src/CheckboxGroup.tsx -> src/index.ts
  • src/index.ts -> src/FormControl/index.ts -> src/FormControl/FormControl.tsx -> src/index.ts
  • src/index.ts -> src/FormControl/index.ts -> src/FormControl/FormControl.tsx -> src/FormControl/_FormControlCaption.tsx -> src/_InputCaption.tsx -> src/index.ts
  • src/index.ts -> src/FormControl/index.ts -> src/FormControl/FormControl.tsx -> src/FormControl/_FormControlLabel.tsx -> src/_InputLabel.tsx -> src/index.ts
  • src/index.ts -> src/FormControl/index.ts -> src/FormControl/FormControl.tsx -> src/FormControl/_FormControlLeadingVisual.tsx -> src/index.ts
  • src/index.ts -> src/FormControl/index.ts -> src/FormControl/FormControl.tsx -> src/drafts/InlineAutocomplete/index.ts -> src/drafts/InlineAutocomplete/InlineAutocomplete.tsx -> src/drafts/InlineAutocomplete/_AutocompleteSuggestions.tsx -> src/index.ts
  • src/index.ts -> src/SegmentedControl/index.ts -> src/SegmentedControl/SegmentedControl.tsx -> src/SegmentedControl/SegmentedControlButton.tsx -> src/index.ts
  • src/index.ts -> src/SegmentedControl/index.ts -> src/SegmentedControl/SegmentedControl.tsx -> src/index.ts
  • src/index.ts -> src/ToggleSwitch.tsx -> src/index.ts
  • src/index.ts -> src/TextInputWithTokens.tsx -> src/Token/Token.tsx -> src/index.ts
  • src/index.ts -> src/Timeline.tsx -> src/index.ts
  • src/index.ts -> src/Token/index.ts -> src/Token/AvatarToken.tsx -> src/index.ts

Testing & Reviewing

One can verify that there are no circular dependencies by running rollup ($(npm bin)/rollup -c). There should be no reported warnings for circular dependencies

@changeset-bot
Copy link

changeset-bot bot commented Sep 6, 2022

⚠️ No Changeset found

Latest commit: 747767c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions
Copy link
Contributor

github-actions bot commented Sep 6, 2022

size-limit report 📦

Path Size
dist/browser.esm.js 76.29 KB (-0.04% 🔽)
dist/browser.umd.js 76.93 KB (-0.01% 🔽)

@joshblack joshblack temporarily deployed to github-pages September 6, 2022 19:33 Inactive
@joshblack joshblack temporarily deployed to github-pages September 6, 2022 19:44 Inactive
@joshblack joshblack changed the title refactor(imports): change circular import references refactor(imports): update circular import references Sep 6, 2022
@joshblack joshblack temporarily deployed to github-pages September 6, 2022 19:55 Inactive
@joshblack joshblack marked this pull request as ready for review September 7, 2022 15:40
@joshblack joshblack requested review from a team and mperrotti September 7, 2022 15:40
@joshblack joshblack temporarily deployed to github-pages September 7, 2022 15:48 Inactive
Copy link
Contributor

@colebemis colebemis left a comment

Choose a reason for hiding this comment

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

Looks great. Thanks for cleaning this up! ✨

@joshblack joshblack temporarily deployed to github-pages September 9, 2022 16:42 Inactive
@joshblack joshblack added the skip changeset This change does not need a changelog label Sep 12, 2022
@joshblack joshblack temporarily deployed to github-pages September 12, 2022 22:31 Inactive
@joshblack joshblack merged commit 8e83ccc into main Sep 12, 2022
@joshblack joshblack deleted the refactor/update-circular-imports branch September 12, 2022 22:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip changeset This change does not need a changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants