From 5d0a9edef388692aedaaf73a2be8b39b007d0945 Mon Sep 17 00:00:00 2001 From: Samuel Reed Date: Fri, 8 Dec 2017 11:49:01 -0600 Subject: [PATCH 1/6] feat: Use fastdom to batch writes and avoid sync layout --- package.json | 4 +++- src/CSSTransition.js | 22 +++------------------- src/utils/ClassHelpers.js | 34 ++++++++++++++++++++++++++++++++++ 3 files changed, 40 insertions(+), 20 deletions(-) create mode 100644 src/utils/ClassHelpers.js diff --git a/package.json b/package.json index 2143116c..3a621a4a 100644 --- a/package.json +++ b/package.json @@ -59,8 +59,10 @@ }, "dependencies": { "dom-helpers": "^3.4.0", + "fastdom": "^1.0.8", "loose-envify": "^1.4.0", - "prop-types": "^15.6.2" + "prop-types": "^15.6.2", + "visibility": "^0.0.0" }, "devDependencies": { "@babel/cli": "^7.4.3", diff --git a/src/CSSTransition.js b/src/CSSTransition.js index 9cc2543e..769012f6 100644 --- a/src/CSSTransition.js +++ b/src/CSSTransition.js @@ -1,14 +1,9 @@ import * as PropTypes from 'prop-types'; -import addOneClass from 'dom-helpers/class/addClass'; - -import removeOneClass from 'dom-helpers/class/removeClass'; import React from 'react'; import Transition from './Transition'; import { classNamesShape } from './utils/PropTypes'; - -const addClass = (node, classes) => node && classes && classes.split(' ').forEach(c => addOneClass(node, c)); -const removeClass = (node, classes) => node && classes && classes.split(' ').forEach(c => removeOneClass(node, c)); +import { addClass, removeClass } from './utils/ClassHelpers'; /** * A transition component inspired by the excellent @@ -90,7 +85,7 @@ class CSSTransition extends React.Component { appearing ? 'appear' : 'enter' ); - this.reflowAndAddClass(node, activeClassName) + addClass(node, activeClassName) if (this.props.onEntering) { this.props.onEntering(node, appearing) @@ -127,7 +122,7 @@ class CSSTransition extends React.Component { onExiting = (node) => { const { activeClassName } = this.getClassNames('exit') - this.reflowAndAddClass(node, activeClassName) + addClass(node, activeClassName) if (this.props.onExiting) { this.props.onExiting(node) @@ -173,17 +168,6 @@ class CSSTransition extends React.Component { doneClassName && removeClass(node, doneClassName); } - reflowAndAddClass(node, className) { - // This is for to force a repaint, - // which is necessary in order to transition styles when adding a class name. - if (className) { - /* eslint-disable no-unused-expressions */ - node && node.scrollTop; - /* eslint-enable no-unused-expressions */ - addClass(node, className); - } - } - render() { const props = { ...this.props }; diff --git a/src/utils/ClassHelpers.js b/src/utils/ClassHelpers.js new file mode 100644 index 00000000..5c30e1bd --- /dev/null +++ b/src/utils/ClassHelpers.js @@ -0,0 +1,34 @@ +import fastdom from 'fastdom'; +import addOneClass from 'dom-helpers/class/addClass'; +import removeOneClass from 'dom-helpers/class/removeClass'; + +export function addClass(node, classes) { + mutateClass(node, classes, addOneClass); +} +export function removeClass(node, classes) { + mutateClass(node, classes, removeOneClass); +} + +let visibilityWatcher; + +function mutateClass(node, classes, fn) { + if (!node) return; + if (classes && typeof classes === 'string') { + const run = () => { + classes.split(' ').forEach(c => fn(node, c)); + }; + // If possible, on browsers, batch these mutations as to avoid synchronous layouts. + // But watch out - if we are batching them, and the page is inactive, we can end up + // hitching up the page for a very long time as these callbacks queue up on + // requestAnimationFrame. So only queue them up if the page is visible. + if (process.browser) { + // eslint-disable-next-line global-require + if (!visibilityWatcher) visibilityWatcher = require('visibility')(); + if (visibilityWatcher.visible()) { + fastdom.mutate(run); + return; + } + } + run(); + } +} From 2541850de34e088f91ecb11dc744014fd197961a Mon Sep 17 00:00:00 2001 From: Samuel Reed Date: Mon, 30 Jul 2018 12:00:46 -0500 Subject: [PATCH 2/6] refactor: remove visibility --- package.json | 3 +-- src/utils/ClassHelpers.js | 18 +++++------------- yarn.lock | 14 ++++++++++++++ 3 files changed, 20 insertions(+), 15 deletions(-) diff --git a/package.json b/package.json index 3a621a4a..550fc079 100644 --- a/package.json +++ b/package.json @@ -61,8 +61,7 @@ "dom-helpers": "^3.4.0", "fastdom": "^1.0.8", "loose-envify": "^1.4.0", - "prop-types": "^15.6.2", - "visibility": "^0.0.0" + "prop-types": "^15.6.2" }, "devDependencies": { "@babel/cli": "^7.4.3", diff --git a/src/utils/ClassHelpers.js b/src/utils/ClassHelpers.js index 5c30e1bd..707ada4c 100644 --- a/src/utils/ClassHelpers.js +++ b/src/utils/ClassHelpers.js @@ -9,26 +9,18 @@ export function removeClass(node, classes) { mutateClass(node, classes, removeOneClass); } -let visibilityWatcher; - function mutateClass(node, classes, fn) { if (!node) return; if (classes && typeof classes === 'string') { - const run = () => { - classes.split(' ').forEach(c => fn(node, c)); - }; + const run = () => classes.split(' ').forEach(c => fn(node, c)); // If possible, on browsers, batch these mutations as to avoid synchronous layouts. // But watch out - if we are batching them, and the page is inactive, we can end up // hitching up the page for a very long time as these callbacks queue up on // requestAnimationFrame. So only queue them up if the page is visible. - if (process.browser) { - // eslint-disable-next-line global-require - if (!visibilityWatcher) visibilityWatcher = require('visibility')(); - if (visibilityWatcher.visible()) { - fastdom.mutate(run); - return; - } + if (process.browser && document.hidden === false) { + fastdom.mutate(run); + } else { + run(); } - run(); } } diff --git a/yarn.lock b/yarn.lock index 262e9476..269e7682 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5506,6 +5506,16 @@ fast-levenshtein@~2.0.4: version "2.0.6" resolved "https://registry.yarnpkg.com/fast-levenshtein/-/fast-levenshtein-2.0.6.tgz#3d8a5c66883a16a30ca8643e851f19baa7797917" +fast-memoize@^2.2.7: + version "2.4.0" + resolved "https://registry.yarnpkg.com/fast-memoize/-/fast-memoize-2.4.0.tgz#2f79eca41c41112b0b70cf53ac3940e206574648" + +fastdom@^1.0.8: + version "1.0.8" + resolved "https://registry.yarnpkg.com/fastdom/-/fastdom-1.0.8.tgz#10f9d36998fd6efae30e529597d788e750c9febb" + dependencies: + strictdom "^1.0.1" + fastparse@^1.1.1: version "1.1.1" resolved "https://registry.yarnpkg.com/fastparse/-/fastparse-1.1.1.tgz#d1e2643b38a94d7583b479060e6c4affc94071f8" @@ -11425,6 +11435,10 @@ strict-uri-encode@^2.0.0: version "2.0.0" resolved "https://registry.yarnpkg.com/strict-uri-encode/-/strict-uri-encode-2.0.0.tgz#b9c7330c7042862f6b142dc274bbcc5866ce3546" +strictdom@^1.0.1: + version "1.0.1" + resolved "https://registry.yarnpkg.com/strictdom/-/strictdom-1.0.1.tgz#189de91649f73d44d59b8432efa68ef9d2659460" + string-length@^2.0.0: version "2.0.0" resolved "https://registry.yarnpkg.com/string-length/-/string-length-2.0.0.tgz#d40dbb686a3ace960c1cffca562bf2c45f8363ed" From 41c08e2e13a79563c15bc443e79e47d291c213bb Mon Sep 17 00:00:00 2001 From: Samuel Reed Date: Fri, 26 Apr 2019 11:54:21 -0500 Subject: [PATCH 3/6] fix(fastdom): fix animations by triggering reflow --- src/utils/ClassHelpers.js | 16 +++++++++++++++- stories/TransitionGroup.js | 5 ++--- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/utils/ClassHelpers.js b/src/utils/ClassHelpers.js index 707ada4c..bed51d7a 100644 --- a/src/utils/ClassHelpers.js +++ b/src/utils/ClassHelpers.js @@ -12,7 +12,14 @@ export function removeClass(node, classes) { function mutateClass(node, classes, fn) { if (!node) return; if (classes && typeof classes === 'string') { - const run = () => classes.split(' ').forEach(c => fn(node, c)); + const run = () => { + // A reflow is necessary to get the browser to respect the transition. However, it doesn't + // need to be done on every single class change, only when the 'active' class is added. + if (classes.indexOf('active') !== -1) { + forceReflow(node); + } + classes.split(' ').forEach(c => fn(node, c)); + } // If possible, on browsers, batch these mutations as to avoid synchronous layouts. // But watch out - if we are batching them, and the page is inactive, we can end up // hitching up the page for a very long time as these callbacks queue up on @@ -24,3 +31,10 @@ function mutateClass(node, classes, fn) { } } } + +// This is for to force a repaint, +// which is necessary in order to transition styles when adding a class name. +function forceReflow(node) { + /* eslint-disable no-unused-expressions */ + node && node.scrollTop; +} diff --git a/stories/TransitionGroup.js b/stories/TransitionGroup.js index 7e8183ef..888d0037 100644 --- a/stories/TransitionGroup.js +++ b/stories/TransitionGroup.js @@ -12,8 +12,7 @@ storiesOf('Css Transition Group', module) .add('Animates on all', () => ( ( Date: Mon, 29 Apr 2019 10:49:58 -0500 Subject: [PATCH 4/6] fix(CSSTransition): don't add undefined class WIthout this patch, try the storybook; you'll see a class of 'undefined' being added to elements on mount because the 'appearClassName' and 'enterClassName' are undefined. --- src/CSSTransition.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/CSSTransition.js b/src/CSSTransition.js index 769012f6..a478e9b2 100644 --- a/src/CSSTransition.js +++ b/src/CSSTransition.js @@ -95,7 +95,7 @@ class CSSTransition extends React.Component { onEntered = (node, appearing) => { const appearClassName = this.getClassNames('appear').doneClassName; const enterClassName = this.getClassNames('enter').doneClassName; - const doneClassName = appearing + const doneClassName = (appearing && appearClassName && enterClassName) ? `${appearClassName} ${enterClassName}` : enterClassName; From ec881aea0708c8a5b3ce2f303e66bdffe42cf48b Mon Sep 17 00:00:00 2001 From: Samuel Reed Date: Mon, 29 Apr 2019 20:02:06 -0700 Subject: [PATCH 5/6] fix(cssTransition): Don't rely on 'active' in className --- src/CSSTransition.js | 2 +- src/utils/ClassHelpers.js | 14 ++++++-------- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/CSSTransition.js b/src/CSSTransition.js index a478e9b2..9ffcb3b5 100644 --- a/src/CSSTransition.js +++ b/src/CSSTransition.js @@ -85,7 +85,7 @@ class CSSTransition extends React.Component { appearing ? 'appear' : 'enter' ); - addClass(node, activeClassName) + addClass(node, activeClassName, true /* reflow */) if (this.props.onEntering) { this.props.onEntering(node, appearing) diff --git a/src/utils/ClassHelpers.js b/src/utils/ClassHelpers.js index bed51d7a..c6815365 100644 --- a/src/utils/ClassHelpers.js +++ b/src/utils/ClassHelpers.js @@ -2,22 +2,20 @@ import fastdom from 'fastdom'; import addOneClass from 'dom-helpers/class/addClass'; import removeOneClass from 'dom-helpers/class/removeClass'; -export function addClass(node, classes) { - mutateClass(node, classes, addOneClass); +export function addClass(node, classes, reflow) { + mutateClass(node, classes, reflow, addOneClass); } -export function removeClass(node, classes) { - mutateClass(node, classes, removeOneClass); +export function removeClass(node, classes, reflow) { + mutateClass(node, classes, reflow, removeOneClass); } -function mutateClass(node, classes, fn) { +function mutateClass(node, classes, reflow, fn) { if (!node) return; if (classes && typeof classes === 'string') { const run = () => { // A reflow is necessary to get the browser to respect the transition. However, it doesn't // need to be done on every single class change, only when the 'active' class is added. - if (classes.indexOf('active') !== -1) { - forceReflow(node); - } + if (reflow) forceReflow(node); classes.split(' ').forEach(c => fn(node, c)); } // If possible, on browsers, batch these mutations as to avoid synchronous layouts. From c0ae113c74433b7f52e124b21b4200bf1a5028a5 Mon Sep 17 00:00:00 2001 From: Samuel Reed Date: Mon, 6 May 2019 13:37:12 -0700 Subject: [PATCH 6/6] perf(classHelpers): reflow all pending nodes completely before class changes Prevents unnecessary context switching for a nearly 40% perf improvement when using "appear" on 50 nodes --- src/utils/ClassHelpers.js | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/utils/ClassHelpers.js b/src/utils/ClassHelpers.js index c6815365..8913766e 100644 --- a/src/utils/ClassHelpers.js +++ b/src/utils/ClassHelpers.js @@ -15,7 +15,9 @@ function mutateClass(node, classes, reflow, fn) { const run = () => { // A reflow is necessary to get the browser to respect the transition. However, it doesn't // need to be done on every single class change, only when the 'active' class is added. - if (reflow) forceReflow(node); + // Batching reflows allows us to avoid read-write-read context switches, by reading + // (node.scrollTop) completely before we write. + if (reflow) emptyReflow(); classes.split(' ').forEach(c => fn(node, c)); } // If possible, on browsers, batch these mutations as to avoid synchronous layouts. @@ -23,6 +25,9 @@ function mutateClass(node, classes, reflow, fn) { // hitching up the page for a very long time as these callbacks queue up on // requestAnimationFrame. So only queue them up if the page is visible. if (process.browser && document.hidden === false) { + // Is this an entering animation where we'll need to force a reflow? + if (reflow) reflowNodes.push(node); + // Schedule the modification for next tick. fastdom.mutate(run); } else { run(); @@ -30,6 +35,13 @@ function mutateClass(node, classes, reflow, fn) { } } +const reflowNodes = []; +// Empty the `reflowNodes` array completely and reflow all of them at once. +function emptyReflow() { + let node; + while (node = reflowNodes.pop()) forceReflow(node); +} + // This is for to force a repaint, // which is necessary in order to transition styles when adding a class name. function forceReflow(node) {