From 1f1f89b0627cc90ecd5bd95012afa29aefd5cc22 Mon Sep 17 00:00:00 2001
From: Nicolas Gallagher <nicolasgallagher@gmail.com>
Date: Fri, 1 Dec 2017 17:52:47 -0800
Subject: [PATCH] [fix] Image 'onLoad' callback on update

'onLoad' should not be called when a component updates, if the 'uri' is
unchanged.

Fixes a regression introduced by
92952ee7460977d79ef27413a351796e80cd927f
---
 src/components/Image/__tests__/index-test.js | 77 +++++++++++++-------
 src/components/Image/index.js                |  3 -
 2 files changed, 49 insertions(+), 31 deletions(-)

diff --git a/src/components/Image/__tests__/index-test.js b/src/components/Image/__tests__/index-test.js
index 61b73074..95932198 100644
--- a/src/components/Image/__tests__/index-test.js
+++ b/src/components/Image/__tests__/index-test.js
@@ -87,6 +87,55 @@ describe('components/Image', () => {
     expect(component.find('img').prop('draggable')).toBe(false);
   });
 
+  describe('prop "onLoad"', () => {
+    test('is called after image is loaded from network', () => {
+      jest.useFakeTimers();
+      ImageLoader.load = jest.fn().mockImplementation((_, onLoad, onError) => {
+        onLoad();
+      });
+      const onLoadStub = jest.fn();
+      shallow(<Image onLoad={onLoadStub} source="https://test.com/img.jpg" />);
+      jest.runOnlyPendingTimers();
+      expect(ImageLoader.load).toBeCalled();
+      expect(onLoadStub).toBeCalled();
+    });
+
+    test('is called after image is loaded from cache', () => {
+      jest.useFakeTimers();
+      ImageLoader.load = jest.fn().mockImplementation((_, onLoad, onError) => {
+        onLoad();
+      });
+      const onLoadStub = jest.fn();
+      const uri = 'https://test.com/img.jpg';
+      shallow(<Image onLoad={onLoadStub} source={uri} />);
+      ImageUriCache.add(uri);
+      jest.runOnlyPendingTimers();
+      expect(ImageLoader.load).not.toBeCalled();
+      expect(onLoadStub).toBeCalled();
+      ImageUriCache.remove(uri);
+    });
+
+    test('is called on update if "uri" is different', () => {
+      jest.useFakeTimers();
+      const onLoadStub = jest.fn();
+      const uri = 'https://test.com/img.jpg';
+      const component = mount(<Image onLoad={onLoadStub} source={uri} />);
+      component.setProps({ source: `https://blah.com/img.png` });
+      jest.runOnlyPendingTimers();
+      expect(onLoadStub.mock.calls.length).toBe(2);
+    });
+
+    test('is not called on update if "uri" is the same', () => {
+      jest.useFakeTimers();
+      const onLoadStub = jest.fn();
+      const uri = 'https://test.com/img.jpg';
+      const component = mount(<Image onLoad={onLoadStub} source={uri} />);
+      component.setProps({ resizeMode: 'stretch' });
+      jest.runOnlyPendingTimers();
+      expect(onLoadStub.mock.calls.length).toBe(1);
+    });
+  });
+
   describe('prop "resizeMode"', () => {
     [
       Image.resizeMode.contain,
@@ -156,34 +205,6 @@ describe('components/Image', () => {
     expect(component.prop('testID')).toBe('testID');
   });
 
-  describe('prop "onLoad"', () => {
-    test('fires after image is loaded', () => {
-      jest.useFakeTimers();
-      ImageLoader.load = jest.fn().mockImplementation((_, onLoad, onError) => {
-        onLoad();
-      });
-      const onLoadStub = jest.fn();
-      shallow(<Image onLoad={onLoadStub} source="https://test.com/img.jpg" />);
-      jest.runOnlyPendingTimers();
-      expect(ImageLoader.load).toBeCalled();
-      expect(onLoadStub).toBeCalled();
-    });
-
-    test('fires even if the image is cached', () => {
-      jest.useFakeTimers();
-      ImageLoader.load = jest.fn().mockImplementation((_, onLoad, onError) => {
-        onLoad();
-      });
-      const onLoadStub = jest.fn();
-      const uri = 'https://test.com/img.jpg';
-      shallow(<Image onLoad={onLoadStub} source={uri} />);
-      ImageUriCache.add(uri);
-      jest.runOnlyPendingTimers();
-      expect(ImageLoader.load).not.toBeCalled();
-      expect(onLoadStub).toBeCalled();
-    });
-  });
-
   test('passes other props through to underlying View', () => {
     const fn = () => {};
     const component = shallow(<Image onResponderGrant={fn} />);
diff --git a/src/components/Image/index.js b/src/components/Image/index.js
index 6c608fea..c01e3eea 100644
--- a/src/components/Image/index.js
+++ b/src/components/Image/index.js
@@ -149,9 +149,6 @@ class Image extends Component {
   componentDidUpdate() {
     if (this._imageState === STATUS_PENDING) {
       this._createImageLoader();
-    } else if (this._imageState === STATUS_LOADED) {
-      const { onLoad } = this.props;
-      onLoad && onLoad();
     }
   }