fix(perf+size): Performance and code size improvements

* chore(rename): use a more specific variable name

* chore(Container): remove clear and count methods

* chore(Container): replace class with regular functions

* chore(rename): Container is now a plain JS file

* chore(createObserver): remove callback param, move callback fn

* fix(warning): hide from production code

* fix(reobserve): reobserve always only once; reduce code size

* chore(syntax): use similar style for all non-React methods

* chore(performance): use shorter and faster code for unmatched

* chore(syntax): use shorter code to detect undefined/null

* chore(syntax): swap findObserverElement arguments around

* chore(performance): use faster for loop; use consistent naming

* chore(syntax): use shorter code to achieve same result

* chore(performance): use shorter syntax and mutate options object

* chore(rename): use more generic name

Author: Merri

src/IntersectionObserver.js

@@ -3,27 +3,11 @@ import { findDOMNode } from 'react-dom';
 import PropTypes from 'prop-types';
 import invariant from 'invariant';
 import warning from 'warning';
-import IntersectionObserverContainer from './IntersectionObserverContainer';
-import { isDOMTypeElement, shallowCompareOptions } from './utils';
-
-/**
- * The Intersection Observer API callback that is called whenever one element,
- * called the target, intersects either the device viewport or a specified element.
- * Also will get caled whenever the visibility of the target element changes and
- * crosses desired amounts of intersection with the root.
- * @param {array} changes
- * @param {IntersectionObserver} observer
- */
-export function callback(changes, observer) {
-    changes.forEach(entry => {
-        const instance = IntersectionObserverContainer.findElement(entry, observer);
-        if (instance) {
-            instance.handleChange(entry);
-        }
-    });
-}
+import { createObserver, observeElement, unobserveElement } from './observer';
+import { isDOMTypeElement, shallowCompare } from './utils';
 
 const observerOptions = ['root', 'rootMargin', 'threshold'];
+const observerProps = ['disabled'].concat(observerOptions);
 const objectProto = Object.prototype;
 
 export default class IntersectionObserver extends React.Component {
@@ -86,18 +70,12 @@ export default class IntersectionObserver extends React.Component {
     };
 
     get options() {
-        return observerOptions.reduce((prev, key) => {
+        return observerOptions.reduce((options, key) => {
             if (objectProto.hasOwnProperty.call(this.props, key)) {
-                let value = this.props[key];
-                if (key === 'root' && objectProto.toString.call(this.props[key]) === '[object String]') {
-                    value = document.querySelector(value);
-                }
-                return {
-                    ...prev,
-                    [key]: value,
-                };
+                const useQuery = key === 'root' && objectProto.toString.call(this.props[key]) === '[object String]';
+                options[key] = useQuery ? document.querySelector(this.props[key]) : this.props[key];
             }
-            return prev;
+            return options;
         }, {});
     }
 
@@ -116,50 +94,42 @@ export default class IntersectionObserver extends React.Component {
                 this.unobserve();
             }
         }
-        warning(
-            !this.props.hasOwnProperty('onlyOnce'),
-            'ReactIntersectionObserver: [deprecation] Use the second argument of onChange to unobserve a target instead of onlyOnce. This prop will be removed in the next major version.',
-        );
+        // eslint-disable-next-line no-undef
+        if (process.env.NODE_ENV !== 'production') {
+            warning(
+                !this.props.hasOwnProperty('onlyOnce'),
+                'ReactIntersectionObserver: [deprecation] Use the second argument of onChange to unobserve a target instead of onlyOnce. This prop will be removed in the next major version.',
+            );
+        }
     };
 
     handleNode = target => {
         if (typeof this.props.children.ref === 'function') {
             this.props.children.ref(target);
         }
-        if (this.renderedTarget && target && this.renderedTarget !== target) {
+        /**
+         * This is a bit ugly: would like to use getSnapshotBeforeUpdate(), but we do not want to depend on
+         * react-lifecycles-compat to support React versions prior to 16.3 as this extra boolean gets the job done.
+         */
+        this.targetChanged = (this.renderedTarget && target) != null && this.renderedTarget !== target;
+        if (this.targetChanged) {
             this.unobserve();
-            this.targetChanged = true;
-        } else {
-            this.targetChanged = false;
         }
         this.target = target;
     };
 
-    compareObserverProps(prevProps) {
-        return observerOptions
-            .concat(['disabled'])
-            .some(option => shallowCompareOptions(this.props[option], prevProps[option]));
-    }
-
-    observe() {
+    observe = () => {
         this.target = isDOMTypeElement(this.target) ? this.target : findDOMNode(this.target);
-        this.observer = IntersectionObserverContainer.create(callback, this.options);
-        IntersectionObserverContainer.observe(this);
-    }
+        this.observer = createObserver(this.options);
+        observeElement(this);
+    };
 
     unobserve = () => {
         if (this.target != null) {
-            IntersectionObserverContainer.unobserve(this);
+            unobserveElement(this);
         }
     };
 
-    reobserve() {
-        this.unobserve();
-        if (!this.props.disabled) {
-            this.observe();
-        }
-    }
-
     componentDidMount() {
         // eslint-disable-next-line no-undef
         if (process.env.NODE_ENV !== 'production' && parseInt(React.version, 10) < 16) {
@@ -174,8 +144,16 @@ export default class IntersectionObserver extends React.Component {
     }
 
     componentDidUpdate(prevProps) {
-        if (this.targetChanged || this.compareObserverProps(prevProps)) {
-            this.reobserve();
+        const propsChanged = observerProps.some(prop => shallowCompare(this.props[prop], prevProps[prop]));
+
+        if (propsChanged) {
+            this.unobserve();
+        }
+
+        if (this.targetChanged || propsChanged) {
+            if (!this.props.disabled) {
+                this.observe();
+            }
         }
     }
 

src/IntersectionObserverContainer.js

@@ -2,8 +2,8 @@
 import 'intersection-observer';
 import React from 'react';
 import renderer from 'react-test-renderer';
-import IntersectionObserver, { callback } from '../IntersectionObserver';
-import IntersectionObserverContainer from '../IntersectionObserverContainer';
+import IntersectionObserver from '../IntersectionObserver';
+import { callback, findObserverElement, observerElementsMap } from '../observer';
 
 function mockUtilsFunctions() {
     const utils = require.requireActual('../utils');
@@ -30,7 +30,7 @@ afterAll(() => {
 });
 
 afterEach(() => {
-    IntersectionObserverContainer.clear();
+    observerElementsMap.clear();
 });
 
 test('throws when the property children is not an only child', () => {
@@ -93,7 +93,7 @@ test("should save target in the observer targets' list on mount", () => {
     );
     const tree = renderer.create(component, { createNodeMock: () => target });
     const observer = tree.getInstance().observer;
-    const retrieved = IntersectionObserverContainer.findElement({ target }, observer);
+    const retrieved = findObserverElement(observer, { target });
 
     expect(retrieved).toEqual(tree.getInstance());
 });
@@ -108,7 +108,7 @@ test("should remove target from the observer targets' list on umount", () => {
     const instance = tree.getInstance();
     const observer = instance.observer;
     tree.unmount();
-    const retrieved = IntersectionObserverContainer.findElement({ target }, observer);
+    const retrieved = findObserverElement(observer, { target });
 
     expect(retrieved).toBeNull();
 });
@@ -123,14 +123,16 @@ describe('update', () => {
         const tree = renderer.create(component, { createNodeMock: () => target });
         const instance = tree.getInstance();
 
-        const spy = jest.spyOn(instance, 'reobserve');
+        const spy1 = jest.spyOn(instance, 'observe');
+        const spy2 = jest.spyOn(instance, 'unobserve');
 
         tree.update(
             <IntersectionObserver onChange={noop} rootMargin="20% 10%">
                 <span />
             </IntersectionObserver>,
         );
-        expect(spy).toBeCalled();
+        expect(spy1).toHaveBeenCalledTimes(1);
+        expect(spy2).toHaveBeenCalledTimes(1);
     });
 
     test('should cleanup when tree reconciliation has led to a full rebuild', () => {
@@ -165,7 +167,7 @@ describe('update', () => {
             </IntersectionObserver>,
         );
 
-        expect(spy1).toHaveBeenCalledTimes(2);
+        expect(spy1).toHaveBeenCalledTimes(1);
         expect(spy2).toHaveBeenCalledTimes(1);
         expect(instance.target).toBe(target);
     });
@@ -249,7 +251,7 @@ describe('update', () => {
     });
 
     test('should be defensive against unobserving nullified nodes', () => {
-        const spy = jest.spyOn(IntersectionObserverContainer, 'unobserve');
+        const sizeAfterObserving = observerElementsMap.size + 1;
         const component = (
             <IntersectionObserver onChange={noop}>
                 <span />
@@ -261,7 +263,7 @@ describe('update', () => {
         tree.getInstance().target = null;
         tree.getInstance().unobserve();
 
-        expect(spy).not.toBeCalled();
+        expect(observerElementsMap.size).toBe(sizeAfterObserving);
     });
 
     test('should not reobserve on a second render after root changed the first time', () => {
@@ -281,7 +283,8 @@ describe('update', () => {
             },
         });
         const instance = tree.getInstance();
-        const spy = jest.spyOn(instance, 'reobserve');
+        const spy1 = jest.spyOn(instance, 'observe');
+        const spy2 = jest.spyOn(instance, 'unobserve');
 
         tree.update(
             <IntersectionObserver onChange={noop}>
@@ -295,7 +298,8 @@ describe('update', () => {
             </IntersectionObserver>,
         );
 
-        expect(spy).toHaveBeenCalledTimes(1);
+        expect(spy1).toHaveBeenCalledTimes(1);
+        expect(spy2).toHaveBeenCalledTimes(1);
     });
 });
 
@@ -312,7 +316,7 @@ describe('callback', () => {
         const instance = renderer.create(component, { createNodeMock: () => target1 }).getInstance();
         renderer.create(React.cloneElement(component), { createNodeMock: () => target2 });
 
-        expect(IntersectionObserverContainer.count()).toBe(1);
+        expect(observerElementsMap.size).toBe(1);
 
         const boundingClientRect = {};
         const intersectionRect = {};
@@ -431,10 +435,10 @@ describe('handleChange', () => {
                     <span />
                 </IntersectionObserver>
             );
-            const spy = jest.spyOn(IntersectionObserverContainer, 'observe');
+            const sizeBefore = observerElementsMap.size;
             renderer.create(component, { createNodeMock: () => target });
 
-            expect(spy).not.toBeCalled();
+            expect(observerElementsMap.size).toBe(sizeBefore);
         });
 
         test('should observe if not disabled', () => {
@@ -443,10 +447,10 @@ describe('handleChange', () => {
                     <span />
                 </IntersectionObserver>
             );
-            const spy = jest.spyOn(IntersectionObserverContainer, 'observe');
+            const sizeAfterObserving = observerElementsMap.size + 1;
             renderer.create(component, { createNodeMock: () => target }).getInstance();
 
-            expect(spy).toBeCalled();
+            expect(observerElementsMap.size).toBe(sizeAfterObserving);
         });
 
         test('should observe if no longer disabled', () => {

src/__tests__/IntersectionObserver.spec.js

@@ -1,5 +1,5 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`#findElement given an entry without target property throws 1`] = `"Cannot read property 'observe' of undefined"`;
+exports[`#findObserverElement given an entry without target property throws 1`] = `"Cannot read property 'observe' of undefined"`;
 
 exports[`#getPooled throws if rootMargin cannot be parsed 1`] = `"rootMargin must be a string literal containing pixels and/or percent values"`;