Fix recursive dependency in image_diff

utilsHelper was requiring ImageDiff and vice-versa forming a loop
Break the loop by separating initImageDiff function to separate file

app/assets/javascripts/diff.js

@@ -5,7 +5,7 @@ import { __ } from '~/locale';
 import { getLocationHash } from './lib/utils/url_utility';
 import FilesCommentButton from './files_comment_button';
 import SingleFileDiff from './single_file_diff';
-import imageDiffHelper from './image_diff/helpers/index';
+import initImageDiffHelper from './image_diff/helpers/init_image_diff';
 
 const UNFOLD_COUNT = 20;
 let isBound = false;
@@ -28,7 +28,7 @@ export default class Diff {
       .first()
       .get(0);
     const canCreateNote = firstFile && firstFile.hasAttribute('data-can-create-note');
-    $diffFile.each((index, file) => imageDiffHelper.initImageDiff(file, canCreateNote));
+    $diffFile.each((index, file) => initImageDiffHelper.initImageDiff(file, canCreateNote));
 
     if (!isBound) {
       $(document)

app/assets/javascripts/image_diff/helpers/index.js

@@ -21,5 +21,4 @@ export default {
   resizeCoordinatesToImageElement: utilsHelper.resizeCoordinatesToImageElement,
   generateBadgeFromDiscussionDOM: utilsHelper.generateBadgeFromDiscussionDOM,
   getTargetSelection: utilsHelper.getTargetSelection,
-  initImageDiff: utilsHelper.initImageDiff,
 };

app/assets/javascripts/image_diff/helpers/init_image_diff.js

+import ImageDiff from '../image_diff';
+import ReplacedImageDiff from '../replaced_image_diff';
+import ImageFile from '../../commit/image_file';
+
+function initImageDiff(fileEl, canCreateNote, renderCommentBadge) {
+  const options = {
+    canCreateNote,
+    renderCommentBadge,
+  };
+  let diff;
+
+  // ImageFile needs to be invoked before initImageDiff so that badges
+  // can mount to the correct location
+  new ImageFile(fileEl); // eslint-disable-line no-new
+
+  if (fileEl.querySelector('.diff-file .js-single-image')) {
+    diff = new ImageDiff(fileEl, options);
+    diff.init();
+  } else if (fileEl.querySelector('.diff-file .js-replaced-image')) {
+    diff = new ReplacedImageDiff(fileEl, options);
+    diff.init();
+  }
+
+  return diff;
+}
+
+export default { initImageDiff };

app/assets/javascripts/image_diff/helpers/utils_helper.js

 import ImageBadge from '../image_badge';
-import ImageDiff from '../image_diff';
-import ReplacedImageDiff from '../replaced_image_diff';
-import ImageFile from '../../commit/image_file';
 
 export function resizeCoordinatesToImageElement(imageEl, meta) {
   const { x, y, width, height } = meta;
@@ -70,25 +67,3 @@ export function getTargetSelection(event) {
     },
   };
 }
-
-export function initImageDiff(fileEl, canCreateNote, renderCommentBadge) {
-  const options = {
-    canCreateNote,
-    renderCommentBadge,
-  };
-  let diff;
-
-  // ImageFile needs to be invoked before initImageDiff so that badges
-  // can mount to the correct location
-  new ImageFile(fileEl); // eslint-disable-line no-new
-
-  if (fileEl.querySelector('.diff-file .js-single-image')) {
-    diff = new ImageDiff(fileEl, options);
-    diff.init();
-  } else if (fileEl.querySelector('.diff-file .js-replaced-image')) {
-    diff = new ReplacedImageDiff(fileEl, options);
-    diff.init();
-  }
-
-  return diff;
-}

app/assets/javascripts/image_diff/init_discussion_tab.js

-import imageDiffHelper from './helpers/index';
+import initImageDiffHelper from './helpers/init_image_diff';
 
 export default () => {
   // Always pass can-create-note as false because a user
@@ -8,6 +8,6 @@ export default () => {
 
   const diffFileEls = document.querySelectorAll('.timeline-content .diff-file.js-image-file');
   [...diffFileEls].forEach(diffFileEl =>
-    imageDiffHelper.initImageDiff(diffFileEl, canCreateNote, renderCommentBadge),
+    initImageDiffHelper.initImageDiff(diffFileEl, canCreateNote, renderCommentBadge),
   );
 };

app/assets/javascripts/single_file_diff.js

@@ -5,7 +5,7 @@ import { __ } from './locale';
 import axios from './lib/utils/axios_utils';
 import createFlash from './flash';
 import FilesCommentButton from './files_comment_button';
-import imageDiffHelper from './image_diff/helpers/index';
+import initImageDiffHelper from './image_diff/helpers/init_image_diff';
 import syntaxHighlight from './syntax_highlight';
 
 const WRAPPER = '<div class="diff-content"></div>';
@@ -101,7 +101,7 @@ export default class SingleFileDiff {
         FilesCommentButton.init($file);
 
         const canCreateNote = $file.closest('.files').is('[data-can-create-note]');
-        imageDiffHelper.initImageDiff($file[0], canCreateNote);
+        initImageDiffHelper.initImageDiff($file[0], canCreateNote);
 
         if (cb) cb();
       })

spec/javascripts/image_diff/helpers/init_image_diff_spec.js

+import initImageDiffHelper from '~/image_diff/helpers/init_image_diff';
+import ImageDiff from '~/image_diff/image_diff';
+import ReplacedImageDiff from '~/image_diff/replaced_image_diff';
+
+describe('initImageDiff', () => {
+  let glCache;
+  let fileEl;
+
+  beforeEach(() => {
+    window.gl = window.gl || (window.gl = {});
+    glCache = window.gl;
+    fileEl = document.createElement('div');
+    fileEl.innerHTML = `
+        <div class="diff-file"></div>
+      `;
+
+    spyOn(ReplacedImageDiff.prototype, 'init').and.callFake(() => {});
+    spyOn(ImageDiff.prototype, 'init').and.callFake(() => {});
+  });
+
+  afterEach(() => {
+    window.gl = glCache;
+  });
+
+  it('should initialize ImageDiff if js-single-image', () => {
+    const diffFileEl = fileEl.querySelector('.diff-file');
+    diffFileEl.innerHTML = `
+        <div class="js-single-image">
+        </div>
+      `;
+
+    const imageDiff = initImageDiffHelper.initImageDiff(fileEl, true, false);
+
+    expect(ImageDiff.prototype.init).toHaveBeenCalled();
+    expect(imageDiff.canCreateNote).toEqual(true);
+    expect(imageDiff.renderCommentBadge).toEqual(false);
+  });
+
+  it('should initialize ReplacedImageDiff if js-replaced-image', () => {
+    const diffFileEl = fileEl.querySelector('.diff-file');
+    diffFileEl.innerHTML = `
+        <div class="js-replaced-image">
+        </div>
+      `;
+
+    const replacedImageDiff = initImageDiffHelper.initImageDiff(fileEl, false, true);
+
+    expect(ReplacedImageDiff.prototype.init).toHaveBeenCalled();
+    expect(replacedImageDiff.canCreateNote).toEqual(false);
+    expect(replacedImageDiff.renderCommentBadge).toEqual(true);
+  });
+});

spec/javascripts/image_diff/helpers/utils_helper_spec.js

 import * as utilsHelper from '~/image_diff/helpers/utils_helper';
-import ImageDiff from '~/image_diff/image_diff';
-import ReplacedImageDiff from '~/image_diff/replaced_image_diff';
 import ImageBadge from '~/image_diff/image_badge';
 import * as mockData from '../mock_data';
 
@@ -151,53 +149,4 @@ describe('utilsHelper', () => {
       });
     });
   });
-
-  describe('initImageDiff', () => {
-    let glCache;
-    let fileEl;
-
-    beforeEach(() => {
-      window.gl = window.gl || (window.gl = {});
-      glCache = window.gl;
-      fileEl = document.createElement('div');
-      fileEl.innerHTML = `
-        <div class="diff-file"></div>
-      `;
-
-      spyOn(ReplacedImageDiff.prototype, 'init').and.callFake(() => {});
-      spyOn(ImageDiff.prototype, 'init').and.callFake(() => {});
-    });
-
-    afterEach(() => {
-      window.gl = glCache;
-    });
-
-    it('should initialize ImageDiff if js-single-image', () => {
-      const diffFileEl = fileEl.querySelector('.diff-file');
-      diffFileEl.innerHTML = `
-        <div class="js-single-image">
-        </div>
-      `;
-
-      const imageDiff = utilsHelper.initImageDiff(fileEl, true, false);
-
-      expect(ImageDiff.prototype.init).toHaveBeenCalled();
-      expect(imageDiff.canCreateNote).toEqual(true);
-      expect(imageDiff.renderCommentBadge).toEqual(false);
-    });
-
-    it('should initialize ReplacedImageDiff if js-replaced-image', () => {
-      const diffFileEl = fileEl.querySelector('.diff-file');
-      diffFileEl.innerHTML = `
-        <div class="js-replaced-image">
-        </div>
-      `;
-
-      const replacedImageDiff = utilsHelper.initImageDiff(fileEl, false, true);
-
-      expect(ReplacedImageDiff.prototype.init).toHaveBeenCalled();
-      expect(replacedImageDiff.canCreateNote).toEqual(false);
-      expect(replacedImageDiff.renderCommentBadge).toEqual(true);
-    });
-  });
 });

spec/javascripts/image_diff/init_discussion_tab_spec.js

 import initDiscussionTab from '~/image_diff/init_discussion_tab';
-import imageDiffHelper from '~/image_diff/helpers/index';
+import initImageDiffHelper from '~/image_diff/helpers/init_image_diff';
 
 describe('initDiscussionTab', () => {
   beforeEach(() => {
@@ -12,7 +12,7 @@ describe('initDiscussionTab', () => {
   });
 
   it('should pass canCreateNote as false to initImageDiff', done => {
-    spyOn(imageDiffHelper, 'initImageDiff').and.callFake((diffFileEl, canCreateNote) => {
+    spyOn(initImageDiffHelper, 'initImageDiff').and.callFake((diffFileEl, canCreateNote) => {
       expect(canCreateNote).toEqual(false);
       done();
     });
@@ -21,7 +21,7 @@ describe('initDiscussionTab', () => {
   });
 
   it('should pass renderCommentBadge as true to initImageDiff', done => {
-    spyOn(imageDiffHelper, 'initImageDiff').and.callFake(
+    spyOn(initImageDiffHelper, 'initImageDiff').and.callFake(
       (diffFileEl, canCreateNote, renderCommentBadge) => {
         expect(renderCommentBadge).toEqual(true);
         done();
@@ -32,9 +32,9 @@ describe('initDiscussionTab', () => {
   });
 
   it('should call initImageDiff for each diffFileEls', () => {
-    spyOn(imageDiffHelper, 'initImageDiff').and.callFake(() => {});
+    spyOn(initImageDiffHelper, 'initImageDiff').and.callFake(() => {});
     initDiscussionTab();
 
-    expect(imageDiffHelper.initImageDiff.calls.count()).toEqual(2);
+    expect(initImageDiffHelper.initImageDiff.calls.count()).toEqual(2);
   });
 });