refactor: Upgrade react and react-dom to v16.8 and refactor Avatar (#1171)

j3tan · web-flow · commit 7dd1bdf4a702 · 2019-05-06T18:03:41.000-07:00
* refactor: upgrade react and react-dom to v16.8 (#761) BREAKING CHANGE: peerDep React requirement have been bumped higher * refactor(avatar): Refactor Avatar to use hooks * fix(test): Fixed various test issues, downgraded enzyme back to 3.8.0 * fix(avatar): remove redundant check
diff --git a/package.json b/package.json
@@ -175,7 +175,7 @@
         "deepmerge": "^2.1.1",
         "draft-js": "^0.10.1",
         "enzyme": "3.8.0",
-        "enzyme-adapter-react-16": "^1.10.0",
+        "enzyme-adapter-react-16": "^1.12.1",
         "enzyme-to-json": "^3.3.4",
         "eslint": "^4.19.1",
         "eslint-config-airbnb": "^17.1.0",
@@ -221,10 +221,10 @@
         "properties-parser": "^0.3.1",
         "query-string": "5.1.1",
         "raf": "^3.4.0",
-        "react": "^16.7.0",
+        "react": "^16.8.0",
         "react-animate-height": "^2.0.4",
         "react-beautiful-dnd": "^9.0.2",
-        "react-dom": "^16.7.0",
+        "react-dom": "^16.8.0",
         "react-draggable": "^3.0.4",
         "react-immutable-proptypes": "^2.1.0",
         "react-intl": "^2.3.0",
@@ -270,10 +270,10 @@
         "mousetrap": "^1.6.1",
         "pikaday": "^1.6.1",
         "query-string": "5.1.1",
-        "react": "^16.7.0",
+        "react": "^16.8.0",
         "react-animate-height": "^2.0.4",
         "react-beautiful-dnd": "^9.0.2",
-        "react-dom": "^16.7.0",
+        "react-dom": "^16.8.0",
         "react-draggable": "^3.0.4",
         "react-immutable-proptypes": "^2.1.0",
         "react-intl": "^2.3.0",
diff --git a/src/components/avatar/Avatar.js b/src/components/avatar/Avatar.js
@@ -1,5 +1,5 @@
 // @flow
-import * as React from 'react';
+import React, { useState, useEffect } from 'react';
 import classNames from 'classnames';
 import AvatarImage from './AvatarImage';
 import AvatarInitials from './AvatarInitials';
@@ -30,50 +30,36 @@ type Props = {
     size?: $Keys<typeof SIZES>,
 };
 
-type State = {
-    /** boolean to determine if image did not load correctly */
-    hasImageErrored: boolean,
-};
-
-class Avatar extends React.PureComponent<Props, State> {
-    state = {
-        hasImageErrored: false,
-    };
-
-    componentWillReceiveProps(nextProps: Props) {
-        if (this.state.hasImageErrored && this.props.avatarUrl !== nextProps.avatarUrl) {
-            this.setState({
-                hasImageErrored: false,
-            });
-        }
-    }
+function Avatar({ avatarUrl, className, name, id, size = '' }: Props) {
+    const [hasImageErrored, setHasImageErrored] = useState(false);
+    const classes = classNames(['avatar', className, { [`avatar--${size}`]: SIZES[size] }]);
 
-    onImageError = () => {
-        this.setState({
-            hasImageErrored: true,
-        });
-    };
+    // Reset hasImageErrored state when avatarUrl changes
+    useEffect(() => {
+        setHasImageErrored(false);
+    }, [avatarUrl]);
 
-    render() {
-        const { avatarUrl, className, name, id, size = '' }: Props = this.props;
-        const { hasImageErrored }: State = this.state;
-        const classes = classNames(['avatar', className, { [`avatar--${size}`]: SIZES[size] }]);
-
-        let avatar;
-        if (avatarUrl && !hasImageErrored) {
-            avatar = <AvatarImage onError={this.onImageError} url={avatarUrl} />;
-        } else if (name) {
-            avatar = <AvatarInitials id={id} name={name} />;
-        } else {
-            avatar = <UnknownUserAvatar className="avatar-icon" />;
-        }
-
-        return (
-            <span className={classes} role="presentation">
-                {avatar}
-            </span>
+    let avatar;
+    if (avatarUrl && !hasImageErrored) {
+        avatar = (
+            <AvatarImage
+                onError={() => {
+                    setHasImageErrored(true);
+                }}
+                url={avatarUrl}
+            />
         );
+    } else if (name) {
+        avatar = <AvatarInitials id={id} name={name} />;
+    } else {
+        avatar = <UnknownUserAvatar className="avatar-icon" />;
     }
+
+    return (
+        <span className={classes} role="presentation">
+            {avatar}
+        </span>
+    );
 }
 
 export default Avatar;
diff --git a/src/components/avatar/__tests__/Avatar-test.js b/src/components/avatar/__tests__/Avatar-test.js
@@ -1,7 +1,14 @@
 import React from 'react';
+import { act } from 'react-dom/test-utils';
 
 import Avatar from '../Avatar';
 
+function MockAvatarImage() {
+    return <div className="avatar-image" />;
+}
+
+jest.mock('../AvatarImage', () => MockAvatarImage);
+
 const testDataURI = 'data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7';
 
 describe('components/avatar/Avatar', () => {
@@ -22,7 +29,7 @@ describe('components/avatar/Avatar', () => {
 
     test('should render an AvatarImage when avatarUrl is passed in', () => {
         const wrapper = shallow(<Avatar avatarUrl={testDataURI} />);
-        const avatarImage = wrapper.find('AvatarImage');
+        const avatarImage = wrapper.find(MockAvatarImage);
         expect(avatarImage.length).toEqual(1);
         expect(avatarImage.prop('url')).toEqual(testDataURI);
     });
@@ -51,10 +58,9 @@ describe('components/avatar/Avatar', () => {
 
     test('should fall back to AvatarInitials when there is an error in AvatarImage', () => {
         const wrapper = shallow(<Avatar avatarUrl="http://foo.bar/baz123_invalid" id="1" name="hello world" />);
+        const avatarImage = wrapper.find(MockAvatarImage);
+        avatarImage.prop('onError')();
 
-        wrapper.instance().onImageError();
-        expect(wrapper.state('hasImageErrored')).toEqual(true);
-        wrapper.update();
         const avatarInitials = wrapper.find('AvatarInitials');
         expect(avatarInitials.length).toEqual(1);
     });
@@ -66,18 +72,27 @@ describe('components/avatar/Avatar', () => {
             avatarUrl: 'http://foo.bar/baz123_invalid',
         };
 
-        const wrapper = shallow(<Avatar {...props} />);
-
-        wrapper.instance().onImageError();
-        expect(wrapper.state('hasImageErrored')).toEqual(true);
+        let wrapper;
+        act(() => {
+            wrapper = mount(<Avatar {...props} />);
+        });
+        expect(wrapper.find(MockAvatarImage).length).toEqual(1);
 
-        wrapper.setProps({
-            ...props,
-            avatarUrl: 'http://foo.bar/baz123_invalid_new',
+        act(() => {
+            const avatarImage = wrapper.find(MockAvatarImage);
+            avatarImage.prop('onError')();
         });
         wrapper.update();
-
-        expect(wrapper.state('hasImageErrored')).toEqual(false);
-        expect(wrapper.find('AvatarImage').length).toEqual(1);
+        expect(wrapper.find(MockAvatarImage).length).toEqual(0);
+        expect(wrapper.find('AvatarInitials').length).toEqual(1);
+
+        act(() => {
+            wrapper.setProps({
+                ...props,
+                avatarUrl: 'http://foo.bar/baz123_invalid_new',
+            });
+        });
+        wrapper.update();
+        expect(wrapper.find(MockAvatarImage).length).toEqual(1);
     });
 });
diff --git a/src/elements/content-sidebar/__tests__/ContentSidebar-test.js b/src/elements/content-sidebar/__tests__/ContentSidebar-test.js
@@ -164,10 +164,6 @@ describe('elements/content-sidebar/ContentSidebar', () => {
         });
 
         test('should set the state with the file and view and then call fetchMetadata', () => {
-            wrapper = getWrapper();
-            instance = wrapper.instance();
-            instance.setState = setState;
-
             instance.fetchMetadata = jest.fn();
             instance.fetchFileSuccessCallback(file);
 
diff --git a/src/elements/content-sidebar/activity-feed/task-new/AssigneeStatus.js b/src/elements/content-sidebar/activity-feed/task-new/AssigneeStatus.js
@@ -30,7 +30,7 @@ const StatusIcon = ({ status, ...rest }: { status: TaskCollabStatus }) => {
 };
 
 const AssignmentStatus = React.memo<Props>(({ user, status, getAvatarUrl, className, ...rest }: Props) => (
-    <div className={classNames('bcs-task-assignment-status', className)} {...rest}>
+    <div className={classNames('bcs-task-assignment-status', className)} data-testid="task-assignment-status" {...rest}>
         <Avatar className="bcs-task-assignment-avatar" user={user} getAvatarUrl={getAvatarUrl} />
         <StatusIcon
             status={status}
diff --git a/src/elements/content-sidebar/activity-feed/task-new/Assignees.js b/src/elements/content-sidebar/activity-feed/task-new/Assignees.js
@@ -73,12 +73,7 @@ class Assignees extends React.Component<Props> {
                             />
                         }
                     >
-                        <AssigneeStatus
-                            status={status}
-                            user={target}
-                            getAvatarUrl={getAvatarUrl}
-                            data-testid="task-assignment-status"
-                        />
+                        <AssigneeStatus status={status} user={target} getAvatarUrl={getAvatarUrl} />
                     </Tooltip>
                 );
             });
@@ -90,7 +85,6 @@ class Assignees extends React.Component<Props> {
                         className="bcs-task-assignment-list-item-avatar"
                         user={target}
                         getAvatarUrl={getAvatarUrl}
-                        data-testid="task-assignment-status"
                     />
                     <AssignmentDetails
                         className="bcs-task-assignment-list-item-details"
diff --git a/yarn.lock b/yarn.lock
