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

[Feature Request] Async framer-motion loading #3340

Closed
luzat opened this issue Jun 27, 2024 · 6 comments · Fixed by #3361
Closed

[Feature Request] Async framer-motion loading #3340

luzat opened this issue Jun 27, 2024 · 6 comments · Fixed by #3361
Assignees
Labels
✨ Type: Enhancement New enhancement on existing codebase

Comments

@luzat
Copy link

luzat commented Jun 27, 2024

Is your feature request related to a problem? Please describe.

Just adding a NextUI button increases bundle size a lot. framer-motion contributes 212 KB (uncompressed) in a production build here. It would be nice if a that could be reduced or loaded asynchronously.

A second problem is, that Framer Motion is also be loaded if animations are disabled with disableAnimation={true}.

Describe the solution you'd like

According to the framer Motion guide it's possible to load features asynchronously using a dynamic import. For many use cases quick initial loading would be preferable over having animations loaded right away.

#2929 already did most of the work required from what I can tell. In a test, I did replace the domAnimation in the ripple effect by a dynamic port and Vite did split the 212 KB framer-motion chunk into 59 KB to be loaded initially and a large 150 KB (or 45 KB with Brotli) chunk that can be loaded asynchronously. I feel that this is a huge amount. Also, if the dynamic import is never invoked when disableAnimation={true}, this code would never have to be downloaded.

I do not know if any other components would not work with that pattern or if there are cases when you might want to load the features synchronously. There could be at least two ways to go about this:

  • Allow users to choose between a dynamic or static import: possibly by importing and using a separate package which reexports domAnimation as a dynamic/static import and pass that to NextUI?)
  • Allow users to at least await the dynamic import: either a setting or just an await import('@nextui/dom-animation') or similar

Even if code splitting won't work in all cases/components, this might be a helpful optimization for some projects.

Describe alternatives you've considered

  • usage of a smaller library
  • dynamically importing all of NextUI; problematic and impractical

Screenshots or Videos

No response

Copy link

linear bot commented Jun 27, 2024

@wingkwong
Copy link
Member

Thanks for the info. We're also investigating this issue as well.

@wingkwong wingkwong self-assigned this Jun 27, 2024
@wingkwong
Copy link
Member

@luzat can you also share what tool you used and how you recorded the size?

@luzat
Copy link
Author

luzat commented Jun 28, 2024

@wingkwong I set up a basic Remix v2 project with Vite and mostly default settings. I manually added NextUI according to the documentation with single components (only Button and its animation dependencies). I added a single button to the _index route.

I checked the bundles that were generated with NODE_ENV=production npx vite-bundle-visualizer. The _index route included 212 KB from framer-motion. After patching the ripple effect to dynamically import domAnimation the _index route shrank by 150 KB and a separate chunk of that size, which contained a large part of framer-motion, was created instead. The _index route still dynamically loaded that chunk, but did not need that code for the initial render.

I did not test with disabled animations, but in that case the import should have been skipped, given that the animation is included conditionally, and 45 to 150 KB saved.

@jrgarciadev jrgarciadev assigned winchesHe and unassigned wingkwong Jun 29, 2024
@jrgarciadev jrgarciadev added 🐛 Type: Bug Something isn't working Improvement labels Jun 29, 2024 — with Linear
@winchesHe winchesHe assigned wingkwong and unassigned winchesHe Jun 30, 2024
@wingkwong wingkwong added ✨ Type: Enhancement New enhancement on existing codebase and removed 🐛 Type: Bug Something isn't working Improvement labels Jun 30, 2024
@xiaoluoer
Copy link

I seem to be getting this error: Warning: React does not recognize the disableAnimation prop on a DOM element. If you intentionally want it to appear in the DOM as a custom attribute, spell it as lowercase disableanimation instead. If you
accidentally passed it from a parent component, remove it from the DOM element. The reason is caused by the Avatar component
截屏2024-07-01 16 52 45

@wingkwong
Copy link
Member

wingkwong commented Jul 1, 2024

@xiaoluoer this is another issue which I've fixed already. See here. The fix will be available in the next bug fix release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ Type: Enhancement New enhancement on existing codebase
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants