Merge pull request #4101 from matrix-org/t3chguy/leaks

Transition BaseAvatar to hooks

.eslintignore.errorfiles

@@ -6,7 +6,6 @@ src/components/structures/RoomView.js
 src/components/structures/ScrollPanel.js
 src/components/structures/SearchBox.js
 src/components/structures/UploadBar.js
-src/components/views/avatars/BaseAvatar.js
 src/components/views/avatars/MemberAvatar.js
 src/components/views/create_room/RoomAlias.js
 src/components/views/dialogs/SetPasswordDialog.js

package.json

@@ -141,6 +141,7 @@
     "flow-parser": "^0.57.3",
     "glob": "^5.0.14",
     "jest": "^24.9.0",
+    "jest-canvas-mock": "^2.2.0",
     "lolex": "^5.1.2",
     "matrix-mock-request": "^1.2.3",
     "matrix-react-test-utils": "^0.2.2",
@@ -160,6 +161,7 @@
     "testMatch": [
       "<rootDir>/test/**/*-test.js"
     ],
+    "setupFiles": ["jest-canvas-mock"],
     "setupFilesAfterEnv": [
       "<rootDir>/test/setupTests.js"
     ],

src/components/views/avatars/BaseAvatar.js

@@ -2,7 +2,7 @@
 Copyright 2015, 2016 OpenMarket Ltd
 Copyright 2018 New Vector Ltd
 Copyright 2019 Michael Telatynski <7t3chguy@gmail.com>
-Copyright 2019 The Matrix.org Foundation C.I.C.
+Copyright 2019, 2020 The Matrix.org Foundation C.I.C.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -17,220 +17,192 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import React from 'react';
+import React, {useCallback, useContext, useEffect, useMemo, useState} from 'react';
 import PropTypes from 'prop-types';
-import createReactClass from 'create-react-class';
 import * as AvatarLogic from '../../../Avatar';
 import SettingsStore from "../../../settings/SettingsStore";
 import AccessibleButton from '../elements/AccessibleButton';
 import MatrixClientContext from "../../../contexts/MatrixClientContext";
+import {useEventEmitter} from "../../../hooks/useEventEmitter";
 import {toPx} from "../../../utils/units";
 
-export default createReactClass({
+const useImageUrl = ({url, urls, idName, name, defaultToInitialLetter}) => {
-    displayName: 'BaseAvatar',
+    const [imageUrls, setUrls] = useState([]);
+    const [urlsIndex, setIndex] = useState();
 
-    propTypes: {
+    const onError = () => {
-        name: PropTypes.string.isRequired, // The name (first initial used as default)
+        const nextIndex = urlsIndex + 1;
-        idName: PropTypes.string, // ID for generating hash colours
+        if (nextIndex < imageUrls.length) {
-        title: PropTypes.string, // onHover title text
+            // try the next one
-        url: PropTypes.string, // highest priority of them all, shortcut to set in urls[0]
+            setIndex(nextIndex);
-        urls: PropTypes.array, // [highest_priority, ... , lowest_priority]
-        width: PropTypes.number,
-        height: PropTypes.number,
-        // XXX resizeMethod not actually used.
-        resizeMethod: PropTypes.string,
-        defaultToInitialLetter: PropTypes.bool, // true to add default url
-        inputRef: PropTypes.oneOfType([
-            // Either a function
-            PropTypes.func,
-            // Or the instance of a DOM native element
-            PropTypes.shape({ current: PropTypes.instanceOf(Element) }),
-        ]),
-    },
-
-    statics: {
-        contextType: MatrixClientContext,
-    },
-
-    getDefaultProps: function() {
-        return {
-            width: 40,
-            height: 40,
-            resizeMethod: 'crop',
-            defaultToInitialLetter: true,
-        };
-    },
-
-    getInitialState: function() {
-        return this._getState(this.props);
-    },
-
-    componentDidMount() {
-        this.unmounted = false;
-        this.context.on('sync', this.onClientSync);
-    },
-
-    componentWillUnmount() {
-        this.unmounted = true;
-        this.context.removeListener('sync', this.onClientSync);
-    },
-
-    // TODO: [REACT-WARNING] Replace with appropriate lifecycle event
-    UNSAFE_componentWillReceiveProps: function(nextProps) {
-        // work out if we need to call setState (if the image URLs array has changed)
-        const newState = this._getState(nextProps);
-        const newImageUrls = newState.imageUrls;
-        const oldImageUrls = this.state.imageUrls;
-        if (newImageUrls.length !== oldImageUrls.length) {
-            this.setState(newState); // detected a new entry
-        } else {
-            // check each one to see if they are the same
-            for (let i = 0; i < newImageUrls.length; i++) {
-                if (oldImageUrls[i] !== newImageUrls[i]) {
-                    this.setState(newState); // detected a diff
-                    break;
-                }
-            }
         }
-    },
+    };
 
-    onClientSync: function(syncState, prevState) {
+    const defaultImageUrl = useMemo(() => AvatarLogic.defaultAvatarUrlForString(idName || name), [idName, name]);
-        if (this.unmounted) return;
 
-        // Consider the client reconnected if there is no error with syncing.
+    useEffect(() => {
-        // This means the state could be RECONNECTING, SYNCING, PREPARED or CATCHUP.
-        const reconnected = syncState !== "ERROR" && prevState !== syncState;
-        if (reconnected &&
-            // Did we fall back?
-            this.state.urlsIndex > 0
-        ) {
-            // Start from the highest priority URL again
-            this.setState({
-                urlsIndex: 0,
-            });
-        }
-    },
-
-    _getState: function(props) {
         // work out the full set of urls to try to load. This is formed like so:
-        // imageUrls: [ props.url, props.urls, default image ]
+        // imageUrls: [ props.url, ...props.urls, default image ]
 
-        let urls = [];
+        let _urls = [];
         if (!SettingsStore.getValue("lowBandwidth")) {
-            urls = props.urls || [];
+            _urls = urls || [];
 
-            if (props.url) {
+            if (url) {
-                urls.unshift(props.url); // put in urls[0]
+                _urls.unshift(url); // put in urls[0]
             }
         }
 
-        let defaultImageUrl = null;
+        if (defaultToInitialLetter) {
-        if (props.defaultToInitialLetter) {
+            _urls.push(defaultImageUrl); // lowest priority
-            defaultImageUrl = AvatarLogic.defaultAvatarUrlForString(
-                props.idName || props.name,
-            );
-            urls.push(defaultImageUrl); // lowest priority
         }
 
         // deduplicate URLs
-        urls = Array.from(new Set(urls));
+        _urls = Array.from(new Set(_urls));
 
-        return {
+        setIndex(0);
-            imageUrls: urls,
+        setUrls(_urls);
-            defaultImageUrl: defaultImageUrl,
+    }, [url, ...(urls || [])]); // eslint-disable-line react-hooks/exhaustive-deps
-            urlsIndex: 0,
-        };
-    },
 
-    onError: function(ev) {
+    const cli = useContext(MatrixClientContext);
-        const nextIndex = this.state.urlsIndex + 1;
+    const onClientSync = useCallback((syncState, prevState) => {
-        if (nextIndex < this.state.imageUrls.length) {
+        // Consider the client reconnected if there is no error with syncing.
-            // try the next one
+        // This means the state could be RECONNECTING, SYNCING, PREPARED or CATCHUP.
-            this.setState({
+        const reconnected = syncState !== "ERROR" && prevState !== syncState;
-                urlsIndex: nextIndex,
+        if (reconnected && urlsIndex > 0 ) { // Did we fall back?
-            });
+            // Start from the highest priority URL again
+            setIndex(0);
         }
-    },
+    }, [urlsIndex]);
+    useEventEmitter(cli, "sync", onClientSync);
 
-    render: function() {
+    const imageUrl = imageUrls[urlsIndex];
-        const imageUrl = this.state.imageUrls[this.state.urlsIndex];
+    return [imageUrl, imageUrl === defaultImageUrl, onError];
+};
 
-        const {
+const BaseAvatar = (props) => {
-            name, idName, title, url, urls, width, height, resizeMethod,
+    const {
-            defaultToInitialLetter, onClick, inputRef,
+        name,
-            ...otherProps
+        idName,
-        } = this.props;
+        title,
+        url,
+        urls,
+        width=40,
+        height=40,
+        resizeMethod="crop", // eslint-disable-line no-unused-vars
+        defaultToInitialLetter=true,
+        onClick,
+        inputRef,
+        ...otherProps
+    } = props;
+
+    const [imageUrl, isDefault, onError] = useImageUrl({url, urls, idName, name, defaultToInitialLetter});
+
+    if (isDefault) {
+        const initialLetter = AvatarLogic.getInitialLetter(name);
+        const textNode = (
+            <span
+                className="mx_BaseAvatar_initial"
+                aria-hidden="true"
+                style={{
+                    fontSize: toPx(width * 0.65),
+                    width: toPx(width),
+                    lineHeight: toPx(height),
+                }}
+            >
+                { initialLetter }
+            </span>
+        );
+        const imgNode = (
+            <img
+                className="mx_BaseAvatar_image"
+                src={imageUrl}
+                alt=""
+                title={title}
+                onError={onError}
+                style={{
+                    width: toPx(width),
+                    height: toPx(height),
+                }}
+                aria-hidden="true" />
+        );
 
-        if (imageUrl === this.state.defaultImageUrl) {
-            const initialLetter = AvatarLogic.getInitialLetter(name);
-            const textNode = (
-                <span className="mx_BaseAvatar_initial" aria-hidden="true"
-                    style={{
-                        fontSize: toPx(width * 0.65),
-                        width: toPx(width),
-                        lineHeight: toPx(height),
-                    }}
-                >
-                    { initialLetter }
-                </span>
-            );
-            const imgNode = (
-                <img className="mx_BaseAvatar_image" src={imageUrl}
-                    alt="" title={title} onError={this.onError}
-                    aria-hidden="true"
-                    style={{
-                        width: toPx(width),
-                        height: toPx(height)
-                    }} />
-            );
-            if (onClick != null) {
-                return (
-                    <AccessibleButton element='span' className="mx_BaseAvatar"
-                        onClick={onClick} inputRef={inputRef} {...otherProps}
-                    >
-                        { textNode }
-                        { imgNode }
-                    </AccessibleButton>
-                );
-            } else {
-                return (
-                    <span className="mx_BaseAvatar" ref={inputRef} {...otherProps}>
-                        { textNode }
-                        { imgNode }
-                    </span>
-                );
-            }
-        }
         if (onClick != null) {
             return (
                 <AccessibleButton
-                    className="mx_BaseAvatar mx_BaseAvatar_image"
+                    {...otherProps}
-                    element='img'
+                    element="span"
-                    src={imageUrl}
+                    className="mx_BaseAvatar"
                     onClick={onClick}
-                    onError={this.onError}
-                    style={{
-                        width: toPx(width),
-                        height: toPx(height),
-                    }}
-                    title={title} alt=""
                     inputRef={inputRef}
-                    {...otherProps} />
+                >
+                    { textNode }
+                    { imgNode }
+                </AccessibleButton>
             );
         } else {
             return (
-                <img
+                <span className="mx_BaseAvatar" ref={inputRef} {...otherProps}>
-                    className="mx_BaseAvatar mx_BaseAvatar_image"
+                    { textNode }
-                    src={imageUrl}
+                    { imgNode }
-                    onError={this.onError}
+                </span>
-                    style={{
-                        width: toPx(width),
-                        height: toPx(height),
-                    }}
-                    title={title} alt=""
-                    ref={inputRef}
-                    {...otherProps} />
             );
         }
-    },
+    }
-});
+
+    if (onClick != null) {
+        return (
+            <AccessibleButton
+                className="mx_BaseAvatar mx_BaseAvatar_image"
+                element='img'
+                src={imageUrl}
+                onClick={onClick}
+                onError={onError}
+                style={{
+                    width: toPx(width),
+                    height: toPx(height),
+                }}
+                title={title} alt=""
+                inputRef={inputRef}
+                {...otherProps} />
+        );
+    } else {
+        return (
+            <img
+                className="mx_BaseAvatar mx_BaseAvatar_image"
+                src={imageUrl}
+                onError={onError}
+                style={{
+                    width: toPx(width),
+                    height: toPx(height),
+                }}
+                title={title} alt=""
+                ref={inputRef}
+                {...otherProps} />
+        );
+    }
+};
+
+BaseAvatar.displayName = "BaseAvatar";
+
+BaseAvatar.propTypes = {
+    name: PropTypes.string.isRequired, // The name (first initial used as default)
+    idName: PropTypes.string, // ID for generating hash colours
+    title: PropTypes.string, // onHover title text
+    url: PropTypes.string, // highest priority of them all, shortcut to set in urls[0]
+    urls: PropTypes.array, // [highest_priority, ... , lowest_priority]
+    width: PropTypes.number,
+    height: PropTypes.number,
+    // XXX resizeMethod not actually used.
+    resizeMethod: PropTypes.string,
+    defaultToInitialLetter: PropTypes.bool, // true to add default url
+    onClick: PropTypes.func,
+    inputRef: PropTypes.oneOfType([
+        // Either a function
+        PropTypes.func,
+        // Or the instance of a DOM native element
+        PropTypes.shape({ current: PropTypes.instanceOf(Element) }),
+    ]),
+};
+
+export default BaseAvatar;

test/components/views/messages/TextualBody-test.js

@@ -205,9 +205,9 @@ describe("<TextualBody />", () => {
             expect(content.html()).toBe('<span class="mx_EventTile_body markdown-body" dir="auto">' +
                 'Hey <span>' +
                 '<a class="mx_Pill mx_UserPill" title="@user:server">' +
-                '<img class="mx_BaseAvatar mx_BaseAvatar_image" src="mxc://avatar.url/image.png" ' +
+                '<img class="mx_BaseAvatar mx_BaseAvatar_image" ' +
                 'style="width: 16px; height: 16px;" ' +
-                'title="@member:domain.bla" alt="" aria-hidden="true">Member</a>' +
+                'title="@member:domain.bla" alt="" aria-hidden="true" src="mxc://avatar.url/image.png">Member</a>' +
                 '</span></span>');
         });
     });

yarn.lock

@@ -2501,6 +2501,11 @@ color-convert@^1.9.0:
   dependencies:
     color-name "1.1.3"
 
+color-convert@~0.5.0:
+  version "0.5.3"
+  resolved "https://registry.yarnpkg.com/color-convert/-/color-convert-0.5.3.tgz#bdb6c69ce660fadffe0b0007cc447e1b9f7282bd"
+  integrity sha1-vbbGnOZg+t/+CwAHzER+G59ygr0=
+
 color-name@1.1.3:
   version "1.1.3"
   resolved "https://registry.yarnpkg.com/color-name/-/color-name-1.1.3.tgz#a7d0558bd89c42f795dd42328f740831ca53bc25"
@@ -2754,6 +2759,11 @@ cssesc@^3.0.0:
   resolved "https://registry.yarnpkg.com/cssesc/-/cssesc-3.0.0.tgz#37741919903b868565e1c09ea747445cd18983ee"
   integrity sha512-/Tb/JcjK111nNScGob5MNtsntNM1aCNUDipB/TkwZFhyDrrE47SOx/18wF2bbjgc3ZzCSKW1T5nt5EbFoAz/Vg==
 
+cssfontparser@^1.2.1:
+  version "1.2.1"
+  resolved "https://registry.yarnpkg.com/cssfontparser/-/cssfontparser-1.2.1.tgz#f4022fc8f9700c68029d542084afbaf425a3f3e3"
+  integrity sha1-9AIvyPlwDGgCnVQghK+69CWj8+M=
+
 cssom@0.3.x, "cssom@>= 0.3.2 < 0.4.0":
   version "0.3.8"
   resolved "https://registry.yarnpkg.com/cssom/-/cssom-0.3.8.tgz#9f1276f5b2b463f2114d3f2c75250af8c1a36f4a"
@@ -4978,6 +4988,14 @@ istanbul-reports@^2.2.6:
   dependencies:
     html-escaper "^2.0.0"
 
+jest-canvas-mock@^2.2.0:
+  version "2.2.0"
+  resolved "https://registry.yarnpkg.com/jest-canvas-mock/-/jest-canvas-mock-2.2.0.tgz#45fbc58589c6ce9df50dc90bd8adce747cbdada7"
+  integrity sha512-DcJdchb7eWFZkt6pvyceWWnu3lsp5QWbUeXiKgEMhwB3sMm5qHM1GQhDajvJgBeiYpgKcojbzZ53d/nz6tXvJw==
+  dependencies:
+    cssfontparser "^1.2.1"
+    parse-color "^1.0.0"
+
 jest-changed-files@^24.9.0:
   version "24.9.0"
   resolved "https://registry.yarnpkg.com/jest-changed-files/-/jest-changed-files-24.9.0.tgz#08d8c15eb79a7fa3fc98269bc14b451ee82f8039"
@@ -6435,6 +6453,13 @@ parse-asn1@^5.0.0:
     pbkdf2 "^3.0.3"
     safe-buffer "^5.1.1"
 
+parse-color@^1.0.0:
+  version "1.0.0"
+  resolved "https://registry.yarnpkg.com/parse-color/-/parse-color-1.0.0.tgz#7b748b95a83f03f16a94f535e52d7f3d94658619"
+  integrity sha1-e3SLlag/A/FqlPU15S1/PZRlhhk=
+  dependencies:
+    color-convert "~0.5.0"
+
 parse-entities@^1.0.2, parse-entities@^1.1.0:
   version "1.2.2"
   resolved "https://registry.yarnpkg.com/parse-entities/-/parse-entities-1.2.2.tgz#c31bf0f653b6661354f8973559cb86dd1d5edf50"