Components: update snackbar to use framer motion instead of react spring #33717
Conversation
|
Size Change: -10.7 kB (-1%) Total Size: 1.07 MB
ℹ️ View Unchanged
|
|
Leaving some notes for myself:
|
|
|
||
| /** | ||
| * Internal dependencies | ||
| */ | ||
| import Snackbar from './'; | ||
|
|
||
| const SNACKBAR_VARIANTS = { |
There was a problem hiding this comment.
I like framer motion's API very much. This variations one is delightful 👏
There was a problem hiding this comment.
Same, this is very readable and easy to parse!
|
I think I have about equivalent animations now in the branch. The one blocker here is the bump in bundle size by 10% in the components bundle. So reading through https://www.framer.com/docs/guide-reduce-bundle-size/ it feels like this optimization should be done at the app level. 🤔 I wonder if the size hit is okay here or we need to hold off swapping in framer motion until more components use pieces of it. It's a bit wasteful otherwise in the component package. |
| @@ -3,19 +3,44 @@ | |||
| */ | |||
| import classnames from 'classnames'; | |||
| import { omit, noop } from 'lodash'; | |||
| import { useTransition, animated } from 'react-spring/web.cjs'; | |||
| import { motion, AnimatePresence } from 'framer-motion'; | |||
There was a problem hiding this comment.
After reading the doc, it seems swapping motion here with m improves the bundle size, did you try it?
There was a problem hiding this comment.
Oh I guess, it would force us to add the LazyMotion provider, which is not great for packages like you said.
|
It's worth mentioning that we use both libraries in other packages and they are bundled with them: gutenberg/packages/edit-post/package.json Line 56 in 85cfab1 By the way, there is a newer version of
There is even a new major version of
I will leave the user experience aside as I don't see much difference between recorded screencasts. We should pick one of those libraries and use it consistently. If we pick
In their docs they advocate for:
If that doesn't work we should consider exposing used APIs from https://github.com/WordPress/gutenberg/blob/trunk/packages/components/src/composite/index.js It isn't great as it gets exposed as public API but we mark them as unstable until we find a better solution: gutenberg/packages/components/src/index.js Lines 43 to 48 in e394488 |
|
Thanks for the reviews!
The react-spring update PR is here, though I think I'll be able to close that in favor of framer motion. Let's hold on this PR and let me queue up PRs for the two other usages of react-spring. We can then see the total bundle size impacts (and land the PRs within a single Gutenberg release). Overall, I'd prefer us using framer-motion for all animations and removing react spring. Usage wise, I'd like us to be able to use the full API over the slimmed down |
|
Also for reference: #33132. |
|
Here's a draft for one the follow ups for Tree Grid: #33765 Looks like a very nice update for higher-level abstractions here (though it also makes the PR larger in scope than I originally intended). |
|
Looking promising! |
|
To unblock this one, let me see if I can pull out framer as a chunk that's only loaded once across packages. What I'm seeing from other usages is about a 10% bump in size for individual packages, since it's 20kb more than react-spring. |
gwwar
force-pushed
the
try/framer-motion
branch
from
9a47c2c
to
b20bceb
Compare
gwwar
requested review from
gziolo,
spacedmonkey and
TimothyBJacobs
as code owners
|
I have a WIP in b20bceb to externalize the motion framer packager, don't merge in the meantime :D |
|
Ready for another round of reviews 👍 Much appreciated if folks more familiar with our dependency management can double check to see if I externalized this one properly. cc @gziolo @youknowriad (or anyone else who's done this in the past). Note that the ~30kb is shuffled to an inline vendor script:
|
|
@gwwar While this works, this make framer motion a public API in WordPress meaning any future upgrades are going to be painful, Can we instead do something similar to what @gziolo shared above reakit #33717 (comment) and expose maybe an |
Right, it's always a major concern that the public API of the 3rd party library changes at some point or we decide that an alternative library would be a better fit. I took a more low-level approach with Reakit, but maybe it'd be better to expose APIs we use under one unstable object. Whatever we decide here, we should adjust the code for Reakit later. Aside: I have just realized that "Compressed Size" GitHub Action should also list vendor files so we could see the full picture 😄 That's why we see only code removed in the comment from the bot: #33717 (comment). Not sure if it's something that we could fix though as most of the vendor files come from WordPress core. A related config line from the GitHub action: |
|
Oh right, not wanting to expose this to Public API 👍 Good point. I'll probably want to add an eslint rule as well to avoid having folks accidentally import a large package in other areas too. In terms of usage a different unstable package might feel better for users. Though in terms of an actual package it makes little sense. import { motion, AnimatePresence } from '@wordpress/unstableAnimation';It is reasonable to lump this together with the component package for now, since presumably folks need components before they can animate it. Two options for this:
import { __unstableMotion as motion, __unstableAnimatePresence as AnimatePresence } from '@wordpress/components`
import { __unstableAnimation as Animation } from '@wordpress/components';
const { motion, AnimatePresence } = Animation; |
gwwar
force-pushed
the
try/framer-motion
branch
from
f510e1f
to
9bb06cd
Compare
|
I went ahead with option 1:
Added ESLint error to avoid folks from importing framer motion directly in other packages: |
Interesting idea, I don't we have explored anything like that yet (publishing an unstable package which I guess means an unstable WP script as well). I don't have a preference between option 1 and 2. This PR is looking good to me. |
|
Thanks for taking another look @youknowriad! I'll land this tomorrow if folks didn't have any follow up feedback. |
gwwar
added
[Package] Edit Post
|
Thanks for the reviews @youknowriad @gziolo @jasmussen ! ✨ |
This PR experiments with using framer motion instead of react spring for the snackbar component. Since Framer Motion is ~30kb gzipped I also
externalized itexported this from@wordpress/componentsonce to avoid bloating each package that it's used in.One benefit here is that framer motion supports
height: autowhich allows us to remove ref and component state management. 🤔There's quite a bundle size jump here to consider for a single component, so I may explore trying to whittle down.Results overall, are are encouraging. I'd like to see if we can update the Persistent List View to use drag and add animations for the drawers opening and closing next.
After - Motion:
after.-.motion.2.mp4
After - Reduce Motion
after.-.reduce.motion.mp4
Before - Motion
before.-.motion.mp4
Before - Reduce Motion
before.-.reduce.motion.mp4
Testing Instructions
In console type:
Or perform an action like publishing or updating a page.
Verify that folks are happy with how framer motion is being exported.