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.

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

Improve tree shaking #6064

Merged
merged 12 commits into from
Apr 20, 2024
Merged

Improve tree shaking #6064

merged 12 commits into from
Apr 20, 2024

Conversation

devongovett
Copy link
Member

Fixes #5798, closes #1669

This uses the new @parcel/bundler-library plugin (parcel-bundler/parcel#9489) to build our packages for publishing to npm. This outputs many more files into the dist directory (one per input file) rather than bundling into a single dist file. That way, tree shaking is much more effective when consuming our libraries due to the sideEffects field in package.json, which allows a bundler to remove an entire file when nothing it it is used. Without this we rely on the minifier to analyze this which is difficult and more error prone.

This mostly improves react-aria-components, which contains many components in a single package, rather than in individual packages. One example app I tested locally got 4x smaller after this change, and no longer included large things like drag and drop and table even when they weren't used.

@rspbot
Copy link

rspbot commented Mar 16, 2024

@rspbot
Copy link

rspbot commented Mar 18, 2024

@rspbot
Copy link

rspbot commented Apr 17, 2024

@rspbot
Copy link

rspbot commented Apr 17, 2024

1 similar comment
@rspbot
Copy link

rspbot commented Apr 17, 2024

@devongovett
Copy link
Member Author

Devonbot:

Verdaccio builds:
CRA Test App
NextJS Test App
RAC Tailwind Example
RAC Spectrum + Tailwind Example
CRA Test App Size
NextJS App Size
Publish stats
Size diff since last release
Docs
Storybook

@devongovett devongovett marked this pull request as ready for review April 17, 2024 18:48
@rspbot
Copy link

rspbot commented Apr 17, 2024

Copy link
Member

@LFDanLu LFDanLu left a comment

Choose a reason for hiding this comment

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

The built example apps seem to work just fine still, is there a particular app configuration/setup that would easily demonstrate the build size improvement? I tried with my own nextJS/vite app locally w/ verdaccio locally but didn't see much of a difference in the build size but my test apps are quite simple

@devongovett
Copy link
Member Author

It would mainly affect RAC since RSP is already split per component (in multiple packages). Since RAC is only a monopackage, that's where you'd see a difference.

@LFDanLu
Copy link
Member

LFDanLu commented Apr 18, 2024

Right, I pulled in just RAC TableView + Button into those test apps thinking the bundle size prior to building verdaccio on this branch would be dramatically larger since it would be pulling in everything (?) but they were still pretty similar before and after :/

@devongovett
Copy link
Member Author

@LFDanLu Depends how many components are being used I guess. Looking at the RAC + tailwind example, the version from this branch downloads 652kb vs 708kb on main. Inspecting the JS code you can see that for example the dnd hooks are no longer included in this version for example. But since it uses many of the components there's less of a difference than in a simpler app with only a few components.

@devongovett
Copy link
Member Author

The RAC Spectrum + Tailwind example shows a much bigger difference since it only uses a few components: 892kb on main vs 245kb on this pr

Copy link
Member

@LFDanLu LFDanLu left a comment

Choose a reason for hiding this comment

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

Thanks for the links, verified the differences in the sources JS, LGTM.

@rspbot
Copy link

rspbot commented Apr 19, 2024

@rspbot
Copy link

rspbot commented Apr 20, 2024

@rspbot
Copy link

rspbot commented Apr 20, 2024

## API Changes

unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any', access: 'private' }
unknown top level export { type: 'any', access: 'private' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'identifier', name: 'Column' }
unknown top level export { type: 'identifier', name: 'Column' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }

@react-aria/i18n

useMessageFormatter

changed by:

  • FormatMessage
-
+useMessageFormatter {
+  strings: LocalizedStrings
+  returnVal: undefined
+}

FormatMessage

-
+FormatMessage {
+  F: undefined
+}

it changed:

  • useMessageFormatter

@devongovett devongovett merged commit c5e4b37 into main Apr 20, 2024
25 checks passed
@devongovett devongovett deleted the library-bundler branch April 20, 2024 00:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incompatible with chunking, large initial bundle sizes Revisit bundling strategy
5 participants