#4441: Improve animations and dynamically import animation libraries to reduce bundle size #4500

dancastellon · 2018-08-03T20:21:41Z

Resolves #4441
Impact: minor
Type: performance

Issue

We currently have velocity-animate in the client bundle at 94.5kb. We use Velocity for 5 things:

Animate the cart drawer
Flash a green color when input fields change
Animate the sliding out of the admin panel
Animate sliding up & down of admin panel cards
Animating the opacity of the overlay when the orders panel is opened

Solution

Most of the animations are replaceable with CSS. Remove velocity-animate and replace those, using a new component called CSSTransitionGroup and CSS. Animating the height of admin panel cards requires JS. Use the small react-animate-height library for that.

Whether using CSSTransitionGroup or Animate height, the bundle size should not be affected (dynamic imports).

Note: This PR removes velocity-animate and velocity-react from package.json, and adds react-transition-group and react-animate-height - both of which are much smaller then Velocity.

Breaking changes

None

Testing

Visit the homepage and open the cart drawer - confirm improved animation
Click a tag in the nav and confirm cart drawer is still open
Confirm on tag pages you can toggle cart drawer
Change an existing tag's title in the header, confirm that leaving the input after change, or pressing enter after change, the input is flashed green
PDP -> Details - Edit a metafield's key, confirm it flashes green. Confirm same with value
Edit the product's title inline, and confirm it flashes green leaving the input after change, or pressing enter after change
Edit subtitle inline and confirm it flashes green
Edit vendor field inline and confirm it flashes green
Edit a field in the variant form (other than tax code). Confirm it flashes a lighter green color
Confirm admin panel can slide open/closed
Confirm admin panel cards can slide up/down
Click "Orders" and confirm black overlay fades in
Click through to order detail, confirm detail panel slides in
Confirm 10-13 while using an RTL language
Confirm switching from LTR to RTL language, the admin bar is positioned correctly (no empty scrollbar on RTL, no cut-off icons when switching back to LTR)
Confirm double-clicking a product in grid - in edit mode - brings you to PDP without crashing the app or any errors in console.
Confirm none of these packages are in the client bundle: velocity-animate, velocity-react, react-animate-height, react-transition-group. Run with export TOOL_NODE_FLAGS='--max-old-space-size=4096' && METEOR_DISABLE_OPTIMISTIC_CACHING=1 meteor run --production --extra-packages bundle-visualizer

Before

After

…reaction into perf-4441-dancastellon-eliminate-velocity

brent-hoover · 2018-08-09T00:12:32Z

@dancastellon When editing tags I see the green flash when exiting the input but not when entering

brent-hoover · 2018-08-09T00:15:52Z

Seeing this error when I double-click on a product logged in as an admin

TypeError: Cannot read property 'add' of undefined
    at highlightInput (animations.js:11)
    at ProductAdmin.animateFieldFlash (productAdmin.js:188)
    at ProductAdmin.UNSAFE_componentWillReceiveProps (productAdmin.js:145)
    at callComponentWillReceiveProps (modules.js?hash=1106023c7619d36eb7b0e0c97f1437b533dbe087:200916)
    at updateClassInstance (modules.js?hash=1106023c7619d36eb7b0e0c97f1437b533dbe087:201105)
    at updateClassComponent (modules.js?hash=1106023c7619d36eb7b0e0c97f1437b533dbe087:202539)
    at beginWork (modules.js?hash=1106023c7619d36eb7b0e0c97f1437b533dbe087:203210)
    at performUnitOfWork (modules.js?hash=1106023c7619d36eb7b0e0c97f1437b533dbe087:205249)
    at workLoop (modules.js?hash=1106023c7619d36eb7b0e0c97f1437b533dbe087:205288)
    at renderRoot (modules.js?hash=1106023c7619d36eb7b0e0c97f1437b533dbe087:205328)

brent-hoover

Seeing a complete crash of the app when double-clicking on a product. Also it looks like we are only partially eliminating Velocity?

brent-hoover · 2018-08-08T23:57:07Z

imports/plugins/core/checkout/client/containers/cartDrawerContainer.js

+/**
+ * @name composer
+ * @private
+ * @summary Reactive Tracker wrapped function


This doesn't seem like a very good description of this function

brent-hoover · 2018-08-08T23:58:44Z

imports/plugins/core/dashboard/client/components/actionView.js

@@ -407,6 +405,7 @@ class ActionView extends Component {

  renderDetailView() {
    const { actionView } = this.props;
+    const { VelocityTransitionGroup } = this.state;


I can't understand why this is still here if we are eliminating Velocity. Do we need to just rename this?

brent-hoover · 2018-08-09T00:00:47Z

imports/plugins/core/ui/client/components/cards/cardBody.js

@@ -47,6 +51,21 @@ class CardBody extends Component {
  }

  render() {
+    const { VelocityTransitionGroup } = this.state;
+    if (VelocityTransitionGroup === undefined) {
+      import("velocity-react")


Why aren't eliminating this as well?

…nimateHeight

dancastellon · 2018-08-09T21:26:33Z

@zenweasel I originally left the <VelocityTransitionGroup /> component in because it had animations that triggered when components mounted/unmounted - something I thought would be tricky to replace with pure JS & CSS. After some research I found <ReactCSSTransitionGroup />, which allows me to use CSS animations on mount/unmount. It worked for all but 1 animation, the slide up/down on the dashboard panels. For that, I found <AnimateHeight />, and it works smoothly.

The downside is that while this would let us remove velocity-animate and velocity-react from package.json, we'd be adding 2 more libs - react-transition-group and animate-height. I'm wondering if you think it's worth it, or if we should revert back to keeping just velocity-react for the <VelocityTransitionGroup /> which we'd dynamically import (so the client bundle size isn't affected).

…reaction into perf-4441-dancastellon-eliminate-velocity

…p + VelocityTransitionGroup

…ation HOCs

aldeed · 2018-08-14T15:21:14Z

I vote for switching to react-transition-group and react-animate-height. They seem small and specific compared to using velocity. If you were to try to achieve the same thing, you'd essentially rebuild them.

…reaction into perf-4441-dancastellon-eliminate-velocity

dancastellon · 2018-08-14T17:21:53Z

@aldeed I agree, done. react-animate-height is much smaller (7.7kb) than velocity-react (94.5kb).

…leaving input)

brent-hoover · 2018-08-16T22:25:52Z

@dancastellon What's the status on this? Can we quickly make the changes that @aldeed requested?

dancastellon · 2018-08-17T15:35:38Z

@zenweasel This one's ready to go. This morning I fixed a Can't call setState on an unmounted component error and ran through the testing instructions.

spencern · 2018-08-31T00:22:11Z

@dancastellon is there a specific reason you chose to go with v1.2.1 of react-transition-group instead of the latest v2.4.0?
As a result of this, we're loading two versions of this lib right now, v2.4.0 which is required by recompose one of our other dependencies and 1.2.1 which is required because of this PR.

Additionally, we're loading chain-function which is only a dependency of the older v1.2.1 version. I'd like to eliminate this older version unless there's a great reason to keep it.

dancastellon added 7 commits August 3, 2018 16:14

(perf): use pure CSS animations for cart drawer (no jQuery or velocity)

2d6cd4d

(feat): helper function for flashing input animation w/ CSS

9bf5896

(feat): pure CSS animation for highlighting an input

a2c1b8f

(feat): replace metafield velocity animations w/ css

d82c654

(feat): replace tag item velocity animations w/ css

ecfe21c

(feat): replace admin input highlight animations w/ css

09c0e77

(chore): remove velocity-animate from package.json

7540d2f

dancastellon changed the title ~~WIP #4441: Eliminate Velocity dependency by using CSS animations~~ #4441: Eliminate Velocity dependency by using CSS animations Aug 6, 2018

dancastellon requested review from brent-hoover, Akarshit and jamesporl August 6, 2018 21:16

dancastellon added 2 commits August 6, 2018 17:18

Pulled latest from 1.14.0

9cf89f3

Merge branch 'release-1.15.0' of https://github.com/reactioncommerce/…

cb7ba3b

…reaction into perf-4441-dancastellon-eliminate-velocity

dancastellon changed the title ~~#4441: Eliminate Velocity dependency by using CSS animations~~ WIP #4441: Eliminate Velocity dependency by using CSS animations Aug 6, 2018

(style): linting issues

bd1a957

dancastellon changed the title ~~WIP #4441: Eliminate Velocity dependency by using CSS animations~~ #4441: Eliminate Velocity dependency by using CSS animations Aug 6, 2018

dancastellon added 3 commits August 7, 2018 14:41

(perf): dynamically import VelocityTransitionGroup

ee87181

Merge branch 'release-1.15.0' of https://github.com/reactioncommerce/…

c0a94d4

…reaction into perf-4441-dancastellon-eliminate-velocity

(style): correct eslint issues

65ac394

brent-hoover requested changes Aug 9, 2018

View reviewed changes

(style): better comment for cart drawer container's composer function

a630ce3

dancastellon changed the title ~~#4441: Eliminate Velocity dependency by using CSS animations~~ WIP #4441: Eliminate Velocity dependency by using CSS animations Aug 9, 2018

dancastellon added 2 commits August 9, 2018 16:35

(perf): replace VelocityTransitionGroup with CSSTransitionGroup and A…

82761cc

…nimateHeight

(fix): CSSTransitionGroup undefined

76220a7

dancastellon added 3 commits August 10, 2018 09:19

Merge branch 'release-1.15.0' of https://github.com/reactioncommerce/…

46deac2

…reaction into perf-4441-dancastellon-eliminate-velocity

(feat): Animations with dynamically imported HOCs - CSSTransitionGrou…

840a287

…p + VelocityTransitionGroup

(refactor): cleanup css animation code

6fee2fa

(fix): prevent 'can't set state on unmounted component' error on anim…

70c9d74

…ation HOCs

dancastellon changed the title ~~WIP #4441: Eliminate Velocity dependency by using CSS animations~~ WIP #4441: Improve animations and dynamically import animation libraries to reduce bundle size Aug 10, 2018

dancastellon added 2 commits August 10, 2018 12:24

(fix): sidebar unable to be opened, edge condition fix

54a45cf

(style): correct linting issues

f9d42a5

dancastellon changed the title ~~WIP #4441: Improve animations and dynamically import animation libraries to reduce bundle size~~ #4441: Improve animations and dynamically import animation libraries to reduce bundle size Aug 10, 2018

dancastellon added 3 commits August 14, 2018 12:42

Merge branch 'release-1.15.0' of https://github.com/reactioncommerce/…

5b15be9

…reaction into perf-4441-dancastellon-eliminate-velocity

(perf): Use react-animate-height to animate admin sidebar height

e4d8a81

(style): correct linting issues

4a2a7dc

dancastellon changed the title ~~#4441: Improve animations and dynamically import animation libraries to reduce bundle size~~ WIP #4441: Improve animations and dynamically import animation libraries to reduce bundle size Aug 16, 2018

dancastellon added 2 commits August 16, 2018 17:03

Fixed merge conflict

28504e7

fix: don't highlight pdp metadata field twice (after enter key, then …

ac55955

…leaving input)

fix: can't setState error on AnimateHeight

71f6273

dancastellon changed the title ~~WIP #4441: Improve animations and dynamically import animation libraries to reduce bundle size~~ #4441: Improve animations and dynamically import animation libraries to reduce bundle size Aug 17, 2018

style: correct eslint issues

043a735

brent-hoover approved these changes Aug 23, 2018

View reviewed changes

aldeed changed the base branch from release-1.15.0 to release-1.16.0 August 23, 2018 19:24

aldeed merged commit 201a154 into release-1.16.0 Aug 23, 2018

aldeed deleted the perf-4441-dancastellon-eliminate-velocity branch August 23, 2018 19:25

This was referenced Aug 31, 2018

fix: resolve react-transition-group dependencies in package-lock #4590

Merged

Release 1.16.0 #4599

Merged

mikemurray mentioned this pull request May 3, 2019

Upgrade version of velocity-react #4399

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#4441: Improve animations and dynamically import animation libraries to reduce bundle size #4500

#4441: Improve animations and dynamically import animation libraries to reduce bundle size #4500

dancastellon commented Aug 3, 2018 •

edited

Loading

brent-hoover commented Aug 9, 2018

brent-hoover commented Aug 9, 2018

brent-hoover left a comment

brent-hoover Aug 8, 2018

brent-hoover Aug 8, 2018

brent-hoover Aug 9, 2018

dancastellon commented Aug 9, 2018

aldeed commented Aug 14, 2018

dancastellon commented Aug 14, 2018

brent-hoover commented Aug 16, 2018

dancastellon commented Aug 17, 2018

spencern commented Aug 31, 2018

#4441: Improve animations and dynamically import animation libraries to reduce bundle size #4500

#4441: Improve animations and dynamically import animation libraries to reduce bundle size #4500

Conversation

dancastellon commented Aug 3, 2018 • edited Loading

Issue

Solution

Breaking changes

Testing

Before

After

brent-hoover commented Aug 9, 2018

brent-hoover commented Aug 9, 2018

brent-hoover left a comment

Choose a reason for hiding this comment

brent-hoover Aug 8, 2018

Choose a reason for hiding this comment

brent-hoover Aug 8, 2018

Choose a reason for hiding this comment

brent-hoover Aug 9, 2018

Choose a reason for hiding this comment

dancastellon commented Aug 9, 2018

aldeed commented Aug 14, 2018

dancastellon commented Aug 14, 2018

brent-hoover commented Aug 16, 2018

dancastellon commented Aug 17, 2018

spencern commented Aug 31, 2018

dancastellon commented Aug 3, 2018 •

edited

Loading