Merge pull request #9 from billmalarky/issue-6-remove-makecancelable-reimplement-patch

Patch for issue 6 and 2.

Author: billmalarky

lib/imageCacheHoc.js

@@ -21,7 +21,6 @@ import FileSystemFactory, { FileSystem } from '../lib/FileSystem';
 import traverse from 'traverse';
 import validator from 'validator';
 import uuid from 'react-native-uuid';
-import makeCancelable from 'makecancelable';
 
 export default function imageCacheHoc(Image, options = {}) {
 
@@ -99,6 +98,9 @@ export default function imageCacheHoc(Image, options = {}) {
       // Assign component unique ID for cache locking.
       this.componentId = uuid.v4();
 
+      // Track component mount status to avoid calling setState() on unmounted component.
+      this._isMounted = false;
+
       // Set default options
       this.options = {
         validProtocols: options.validProtocols || ['https'],
@@ -140,30 +142,35 @@ export default function imageCacheHoc(Image, options = {}) {
     // See: https://reactjs.org/docs/react-component.html#componentdidmount
     async componentDidMount() {
 
+      // Track component mount status to avoid calling setState() on unmounted component.
+      this._isMounted = true;
+
       // Add a cache lock to file with this name (prevents concurrent <CacheableImage> components from pruning a file with this name from cache).
-      let fileName = await this.fileSystem.getFileNameFromUrl(traverse(this.props).get(['source', 'uri']));
+      const fileName = await this.fileSystem.getFileNameFromUrl(traverse(this.props).get(['source', 'uri']));
       FileSystem.lockCacheFile(fileName, this.componentId);
 
       // Check local fs for file, fallback to network and write file to disk if local file not found.
-      // This must be cancelable in case component is unmounted before request completes.
-      let permanent = this.props.permanent ? true : false;
+      const permanent = this.props.permanent ? true : false;
+      let localFilePath = null;
+      try {
+        localFilePath = await this.fileSystem.getLocalFilePathFromUrl(traverse(this.props).get(['source', 'uri']), permanent);
+      } catch (error) {
+        console.error(error); // eslint-disable-line no-console
+      }
 
-      this.cancelLocalFilePathRequest = makeCancelable(
-        this.fileSystem.getLocalFilePathFromUrl(traverse(this.props).get(['source', 'uri']), permanent),
-        localFilePath => this.setState({ localFilePath }),
-        error => console.error(error) // eslint-disable-line no-console
-      );
+      // Check component is still mounted to avoid calling setState() on components that were quickly
+      // mounted then unmounted before componentDidMount() finishes.
+      // See: https://github.com/billmalarky/react-native-image-cache-hoc/issues/6#issuecomment-354490597
+      if (this._isMounted && localFilePath) {
+        this.setState({ localFilePath });
+      }
 
     }
 
     async componentWillUnmount() {
 
-      // Cancel pending setState() actions.
-      // NOTE: must check this.cancelLocalFilePathRequest is set to avoid edge case where component is mounted then immediately unmounted before componentDidMount() fires.
-      if (this.cancelLocalFilePathRequest) {
-        this.cancelLocalFilePathRequest();
-      }
-
+      // Track component mount status to avoid calling setState() on unmounted component.
+      this._isMounted = false;
 
       // Remove component cache lock on associated image file on component teardown.
       let fileName = await this.fileSystem.getFileNameFromUrl(traverse(this.props).get(['source', 'uri']));

package.json

@@ -42,7 +42,7 @@
     "react-test-renderer": "^16.0.0",
     "should": "^13.1.2",
     "should-sinon": "^0.0.6",
-    "sinon": "^4.1.2"
+    "sinon": "^4.1.3"
   },
   "jest": {
     "preset": "react-native",
@@ -53,7 +53,6 @@
   "dependencies": {
     "buffer": "^5.0.8",
     "crypto-js": "^3.1.9-1",
-    "makecancelable": "^1.0.0",
     "path": "^0.12.7",
     "prop-types": "^15.6.0",
     "react-native-fetch-blob": "0.10.8",

tests/CacheableImage.test.js

@@ -262,24 +262,37 @@ describe('CacheableImage', function() {
 
   });
 
-  it('setState() actions on component mount should create cancelable functions and use on unmount.', async () => {
+  it('Verify component is actually still mounted before calling setState() in componentDidMount().', async () => {
 
     // Set up mocks
     const FileSystem = require('../lib/FileSystem').default;
     FileSystem.prototype.getLocalFilePathFromUrl = jest.fn();
     FileSystem.prototype.getLocalFilePathFromUrl.mockReturnValue(new Promise((resolve) => {
-      resolve(mockData.basePath + '/react-native-image-cache-hoc/permanent/cd7d2199cd8e088cdfd9c99fc6359666adc36289.png');
+      setTimeout(() => {
+        resolve(mockData.basePath + '/react-native-image-cache-hoc/permanent/cd7d2199cd8e088cdfd9c99fc6359666adc36289.png');
+      }, 1000); // Mock 1 second delay for this async function to complete.
     }));
 
     const CacheableImage = imageCacheHoc(Image);
     const cacheableImage = new CacheableImage(mockData.mockCacheableImageProps);
 
-    // Mount and unmount
-    await cacheableImage.componentDidMount();
-    cacheableImage.cancelLocalFilePathRequest(); // Call it once manually to actually cancel local file path request
-    cacheableImage.cancelLocalFilePathRequest = sinon.spy(); // Set up a spy to make sure it gets called in componentWillUnmount().
-    await cacheableImage.componentWillUnmount();
-    cacheableImage.cancelLocalFilePathRequest.should.be.calledOnce();
+    // Ensure that if component is mounted then immediately unmounted before componentDidMount() finishes
+    // executing, setState() will not be called by an unmounted component when componentDidMount() resumes execution after
+    // completing async work.
+    // See: https://github.com/billmalarky/react-native-image-cache-hoc/issues/6#issuecomment-354490597
+    cacheableImage.setState = sinon.spy(); // Mock setState with function tracker to ensure it doesn't get called on unmounted component.
+    cacheableImage.componentDidMount();
+    cacheableImage._isMounted.should.be.true();
+    cacheableImage.componentWillUnmount();
+    cacheableImage._isMounted.should.be.false();
+
+    // Wait for componentDidMount() to complete execution.
+    await new Promise((resolve) => {
+      setTimeout(resolve, 2000);
+    });
+
+    // Ensure that setState() was not called on unmounted component.
+    cacheableImage.setState.should.not.be.called();
 
   });
 

yarn.lock

@@ -3031,8 +3031,8 @@ lolex@^1.6.0:
   resolved "https://registry.yarnpkg.com/lolex/-/lolex-1.6.0.tgz#3a9a0283452a47d7439e72731b9e07d7386e49f6"
 
 lolex@^2.2.0:
-  version "2.3.0"
-  resolved "https://registry.yarnpkg.com/lolex/-/lolex-2.3.0.tgz#d6bad0f0aa5caebffcfebb09fb2caa89baaff51c"
+  version "2.3.1"
+  resolved "https://registry.yarnpkg.com/lolex/-/lolex-2.3.1.tgz#3d2319894471ea0950ef64692ead2a5318cff362"
 
 longest@^1.0.1:
   version "1.0.1"
@@ -3055,10 +3055,6 @@ macos-release@^1.0.0:
   version "1.1.0"
   resolved "https://registry.yarnpkg.com/macos-release/-/macos-release-1.1.0.tgz#831945e29365b470aa8724b0ab36c8f8959d10fb"
 
-makecancelable@^1.0.0:
-  version "1.0.0"
-  resolved "https://registry.yarnpkg.com/makecancelable/-/makecancelable-1.0.0.tgz#c7e2606e59db7a4bf8098ff5b52d7f13173499e5"
-
 [email protected]:
   version "1.0.11"
   resolved "https://registry.yarnpkg.com/makeerror/-/makeerror-1.0.11.tgz#e01a5c9109f2af79660e4e8b9587790184f5a96c"
@@ -4274,17 +4270,17 @@ simple-plist@^0.2.1:
     bplist-parser "0.1.1"
     plist "2.0.1"
 
-sinon@^4.1.2:
-  version "4.1.2"
-  resolved "https://registry.yarnpkg.com/sinon/-/sinon-4.1.2.tgz#65610521d926fb53742dd84cd599f0b89a82f440"
+sinon@^4.1.3:
+  version "4.1.3"
+  resolved "https://registry.yarnpkg.com/sinon/-/sinon-4.1.3.tgz#fc599eda47ed9f1a694ce774b94ab44260bd7ac5"
   dependencies:
     diff "^3.1.0"
     formatio "1.2.0"
     lodash.get "^4.4.2"
     lolex "^2.2.0"
     nise "^1.2.0"
     supports-color "^4.4.0"
-    type-detect "^4.0.0"
+    type-detect "^4.0.5"
 
 slash@^1.0.0:
   version "1.0.0"
@@ -4616,7 +4612,7 @@ type-check@~0.3.2:
   dependencies:
     prelude-ls "~1.1.2"
 
-type-detect@^4.0.0:
+type-detect@^4.0.5:
   version "4.0.5"
   resolved "https://registry.yarnpkg.com/type-detect/-/type-detect-4.0.5.tgz#d70e5bc81db6de2a381bcaca0c6e0cbdc7635de2"