Skip to content

Conversation

SimenB
Copy link

@SimenB SimenB commented Sep 16, 2020

Closes N/A

✅ Pull Request Checklist:

  • Included link to corresponding React Spectrum GitHub Issue.
  • Added/updated unit tests and storybook for this change (for new code or code which already has tests).
  • Filled out test instructions.
  • Updated documentation (if it already exists for this component).
  • Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

Not sure if this is something you want to do, but to me it seems practical for @react-aria/switch to export the type of its arguments. Might not fit into what you wanna do? Feel free to close this if you don't want it 🙂

And thank you so much for this project, it's awesome!

🧢 Your Project:

@devongovett
Copy link
Member

Thanks for the PR! Just trying to understand the need for this. Is it to avoid adding a dependency on @react-types/switch?

@SimenB
Copy link
Author

SimenB commented Sep 18, 2020

Yeah, essentially. 🙂 E.g. looking at https://react-spectrum.adobe.com/react-aria/useSwitch.html, the install instructions is just yarn add @react-aria/switch, but the example imports @react-aria/visually-hidden @react-stately/toggle and @react-aria/focus - additionally for TS you'd want to import the @react-types/switch package. (As an aside, the example is missing the @react-aria/switch import, not sure why?)

While @react-aria/visually-hidden and @react-aria/focus feels out of scope (as they are used for styling), the other packages are already dependencies of @react-aria/switch and I think it makes sense for that package to re-export all the pieces you need regardless when using the package. They are already dependencies, so I think the only difference in practice would be how many packages consumers would have to install:

"@react-aria/toggle": "^3.1.0",
"@react-stately/toggle": "^3.1.0",
"@react-types/switch": "^3.1.0"

@snowystinger
Copy link
Member

I think this has been addressed with #3326

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.

4 participants