-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
A11y: Improve dist size from React Aria #32594
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.
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
Conversation
…. temporarily disable imports from `react-aria-components`
WalkthroughThis PR standardizes ARIA-related imports and dependencies. It adds ESLint no-restricted-imports rules to block direct imports from react-aria, react-stately, and react-aria-components, guiding usage to scoped packages and patched-dist entrypoints. Dependencies are updated: root removes react-aria and react-stately; core adds scoped @react-aria/, @react-stately/, and @react-types/shared, and switches react-aria-components to a patch link. Component files update imports to use @react-aria/interactions, @react-aria/overlays, @react-aria/tabs, @react-stately/, @react-types/shared, and react-aria-components/patched-dist/ paths. No runtime logic or public API changes. Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment |
|
View your CI Pipeline Execution ↗ for commit 4a7671c
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
code/.yarn/patches/react-aria-components-npm-1.12.2-6c5dcdafab.patchis excluded by!**/.yarn/**code/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (19)
code/.eslintrc.js(1 hunks)code/core/package.json(2 hunks)code/core/src/components/components/Modal/Modal.styled.tsx(1 hunks)code/core/src/components/components/Modal/Modal.tsx(1 hunks)code/core/src/components/components/Popover/WithPopover.tsx(1 hunks)code/core/src/components/components/Select/Select.tsx(1 hunks)code/core/src/components/components/Tabs/StatelessTab.tsx(1 hunks)code/core/src/components/components/Tabs/StatelessTabList.tsx(1 hunks)code/core/src/components/components/Tabs/StatelessTabPanel.tsx(1 hunks)code/core/src/components/components/Tabs/StatelessTabsView.tsx(1 hunks)code/core/src/components/components/Tabs/TabList.tsx(1 hunks)code/core/src/components/components/Tabs/TabPanel.stories.tsx(1 hunks)code/core/src/components/components/Tabs/TabPanel.tsx(1 hunks)code/core/src/components/components/Tabs/TabsView.tsx(1 hunks)code/core/src/components/components/shared/overlayHelpers.tsx(1 hunks)code/core/src/components/components/tooltip/WithTooltip.tsx(1 hunks)code/core/src/manager/components/preview/Preview.tsx(1 hunks)code/core/src/manager/components/preview/Toolbar.tsx(1 hunks)code/package.json(0 hunks)
💤 Files with no reviewable changes (1)
- code/package.json
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Adhere to ESLint and Prettier rules across all JS/TS source files
Files:
code/core/src/components/components/Tabs/StatelessTabsView.tsxcode/core/src/components/components/Tabs/StatelessTabList.tsxcode/core/src/manager/components/preview/Toolbar.tsxcode/core/src/components/components/tooltip/WithTooltip.tsxcode/core/src/components/components/Tabs/TabList.tsxcode/core/src/components/components/Tabs/TabPanel.stories.tsxcode/core/src/components/components/Tabs/StatelessTab.tsxcode/core/src/components/components/shared/overlayHelpers.tsxcode/core/src/components/components/Tabs/StatelessTabPanel.tsxcode/core/src/components/components/Tabs/TabPanel.tsxcode/core/src/components/components/Popover/WithPopover.tsxcode/core/src/components/components/Modal/Modal.tsxcode/core/src/manager/components/preview/Preview.tsxcode/core/src/components/components/Modal/Modal.styled.tsxcode/core/src/components/components/Select/Select.tsxcode/core/src/components/components/Tabs/TabsView.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Fix type errors and prefer precise typings instead of using any or suppressions, consistent with strict mode
Files:
code/core/src/components/components/Tabs/StatelessTabsView.tsxcode/core/src/components/components/Tabs/StatelessTabList.tsxcode/core/src/manager/components/preview/Toolbar.tsxcode/core/src/components/components/tooltip/WithTooltip.tsxcode/core/src/components/components/Tabs/TabList.tsxcode/core/src/components/components/Tabs/TabPanel.stories.tsxcode/core/src/components/components/Tabs/StatelessTab.tsxcode/core/src/components/components/shared/overlayHelpers.tsxcode/core/src/components/components/Tabs/StatelessTabPanel.tsxcode/core/src/components/components/Tabs/TabPanel.tsxcode/core/src/components/components/Popover/WithPopover.tsxcode/core/src/components/components/Modal/Modal.tsxcode/core/src/manager/components/preview/Preview.tsxcode/core/src/components/components/Modal/Modal.styled.tsxcode/core/src/components/components/Select/Select.tsxcode/core/src/components/components/Tabs/TabsView.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (17)
code/.eslintrc.js (1)
41-58: ESLint rules align well with the PR objectives.The three no-restricted-imports rules correctly enforce the migration strategy: blocking top-level
react-ariaandreact-statelyimports while directing developers to scoped packages, and steeringreact-aria-componentsusage to patched-dist entrypoints for better tree-shaking.code/core/src/components/components/tooltip/WithTooltip.tsx (1)
7-11: Import migration looks correct.The changes properly split the imports:
Focusablemoved to@react-aria/interactions(correct scoped package)- Tooltip components moved to
react-aria-components/patched-dist/Tooltip(aligns with ESLint rules and PR objectives)Component behavior and type safety are preserved.
code/core/src/components/components/Tabs/StatelessTabsView.tsx (1)
7-7: Tabs import correctly migrated to patched-dist.The import path change to
react-aria-components/patched-dist/Tabsaligns with the PR's tree-shaking optimization strategy and ESLint rules.code/core/src/components/components/shared/overlayHelpers.tsx (1)
5-5: Portal provider import correctly migrated to scoped package.The change from
react-ariato@react-aria/overlaysforUNSAFE_PortalProvideris correct and aligns with the PR's migration to scoped packages.code/core/src/components/components/Tabs/StatelessTabList.tsx (1)
8-8: TabList import correctly migrated to patched-dist.The import path change to
react-aria-components/patched-dist/Tabsis consistent with the other Tabs component migrations in this PR and aligns with the tree-shaking optimization goals.code/core/src/components/components/Tabs/TabPanel.stories.tsx (1)
4-4: LGTM! Import path correctly updated to scoped package.The
TabListStatetype import has been correctly migrated from the umbrellareact-statelypackage to the specific@react-stately/tabspackage, reducing bundle size as intended by this PR.code/core/src/manager/components/preview/Preview.tsx (1)
11-11: LGTM! Type import correctly migrated to scoped package.The
TabListStatetype import has been properly updated to use@react-stately/tabs, consistent with the bundle size optimization strategy.code/core/src/components/components/Tabs/StatelessTab.tsx (1)
4-4: LGTM! Component import correctly uses patched distribution path.The
Tabcomponent import has been updated to use thereact-aria-components/patched-dist/Tabspath, avoiding the barrel file and reducing bundle size as intended.code/core/src/components/components/Tabs/StatelessTabPanel.tsx (1)
6-6: LGTM! TabPanel import correctly migrated to patched distribution.The
TabPanelcomponent import now uses thereact-aria-components/patched-dist/Tabspath, consistent with the bundle size optimization strategy and avoiding barrel file imports.code/core/src/manager/components/preview/Toolbar.tsx (1)
8-8: LGTM! Type import properly updated to scoped package.The
TabListStatetype import has been correctly migrated to@react-stately/tabs, aligning with the PR's bundle size optimization objectives.code/core/src/components/components/Modal/Modal.tsx (1)
5-7: LGTM! Import migration looks correct.The imports have been successfully migrated from top-level packages to scoped packages and patched-dist entrypoints, consistent with the PR's bundle-size optimization goal.
code/core/src/components/components/Select/Select.tsx (1)
9-12: LGTM! Scoped package imports correctly applied.The overlay and interaction utilities have been successfully migrated to their respective scoped packages, maintaining the same API surface.
code/core/src/components/components/Modal/Modal.styled.tsx (1)
9-10: LGTM! Patched-dist imports correctly applied.The Heading and Text components have been successfully migrated to patched-dist entrypoints, avoiding the barrel file import as intended.
code/core/src/components/components/Tabs/TabPanel.tsx (1)
6-8: LGTM! Scoped package imports correctly applied.The tab-related imports have been successfully migrated to their respective scoped packages, maintaining type safety and runtime behavior.
code/core/src/components/components/Tabs/TabsView.tsx (1)
6-8: LGTM! Scoped package imports correctly applied.The tabs state and collection imports have been successfully migrated to their respective scoped packages, maintaining the same functionality.
code/core/package.json (1)
232-239: LGTM: Scoped package additions align with bundle-size optimization.The added scoped packages (@react-aria/interactions, @react-aria/overlays, @react-aria/tabs, @react-stately/overlays, @react-stately/tabs, @react-types/shared) use appropriate version ranges and match the latest stable releases. This granular approach should improve tree-shaking and reduce bundle size.
code/core/src/components/components/Tabs/TabList.tsx (1)
8-10: Verify migrated imports from scoped packages
Ensure@react-aria/tabs,@react-stately/tabs, and@react-types/sharedare declared in package.json and actually exportuseTab,useTabList,TabListState, andNodebefore merging.
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 44 | 44 | 0 |
| Self size | 34.77 MB | 31.46 MB | 🎉 -3.31 MB 🎉 |
| Dependency size | 17.33 MB | 17.33 MB | 0 B |
| Bundle Size Analyzer | Link | Link |
@storybook/cli
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 188 | 188 | 0 |
| Self size | 905 KB | 905 KB | 🎉 -84 B 🎉 |
| Dependency size | 84.42 MB | 81.11 MB | 🎉 -3.31 MB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/codemod
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 170 | 170 | 0 |
| Self size | 35 KB | 35 KB | 0 B |
| Dependency size | 80.85 MB | 77.54 MB | 🎉 -3.31 MB 🎉 |
| Bundle Size Analyzer | Link | Link |
create-storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 45 | 45 | 0 |
| Self size | 1.55 MB | 1.55 MB | 0 B |
| Dependency size | 52.10 MB | 48.79 MB | 🎉 -3.31 MB 🎉 |
| Bundle Size Analyzer | node | node |
Sidnioulz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Brilliant work! -3.31MB on the bundle size!
Telescopes on top of #32458
Reduce the bundle size of core, by fixing React Aria usage and internals. The current structure of the React Aria packages is not fully optimised for tree-shakeability, at least esbuild doesn't remove any dead code from it, which is too costly for us. This PR removes the dead code "manually" by using precise imports.
What I did
react-aria,react-statelyandreact-typesto specific packages under eg.@react-aria/overlaysreact-aria-componentspackage to:package.json#exportsto export component files directly instead of via the barrel filereact-ariaorreact-stately, but instead from sub-packagesThis should all be temporary, as the upstream packages have a plan to do this natively: adobe/react-spectrum#8797
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
Documentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 Canary release
This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/coreteam here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>Summary by CodeRabbit
Refactor
Chores