Prevent infinite loop by aborting fetch version

src/pages/Compare/index.spec.tsx

@@ -510,45 +510,22 @@ describe(__filename, () => {
     });
   });
 
-  // This could happen when a keyboard navigation updates the selected path.
-  it('does not dispatch viewVersionFile() on update if the query string contains a `path` that is different than `version.selectedPath`', () => {
-    const { dispatchSpy, root } = loadDiffAndRender();
-
-    const path = 'a/different/file.js';
-    const history = createFakeHistory({
-      location: createFakeLocation({
-        search: queryString.stringify({ path }),
-      }),
-    });
-
-    root.setProps({ history });
-
-    expect(dispatchSpy).not.toHaveBeenCalled();
-  });
-
-  it('does not dispatch viewVersionFile() on update when `path` is equal to the selected path', () => {
-    const { dispatchSpy, root, version } = loadDiffAndRender();
-
-    const history = createFakeHistory({
-      location: createFakeLocation({
-        search: queryString.stringify({ path: version.file.selected_file }),
-      }),
-    });
-
-    root.setProps({ history });
-
-    expect(dispatchSpy).not.toHaveBeenCalled();
-  });
+  it('does not dispatch anything on mount when an API error has occured', () => {
+    const addonId = 9999;
+    const baseVersionId = 1;
+    const headVersionId = baseVersionId + 1;
 
-  it('does not dispatch viewVersionFile() on update when the query string does not contain a `path`', () => {
-    const { dispatchSpy, root } = loadDiffAndRender();
+    const store = configureStore();
+    store.dispatch(versionsActions.beginFetchDiff());
+    // Simulate an API error.
+    store.dispatch(versionsActions.abortFetchDiff());
+    const dispatch = spyOn(store, 'dispatch');
 
-    const history = createFakeHistory({
-      location: createFakeLocation({ search: '' }),
+    render({
+      ...getRouteParams({ addonId, baseVersionId, headVersionId }),
+      store,
     });
 
-    root.setProps({ history });
-
-    expect(dispatchSpy).not.toHaveBeenCalled();
+    expect(dispatch).not.toHaveBeenCalled();
   });
 });

src/pages/Compare/index.tsx

@@ -34,7 +34,6 @@ type PropsFromRouter = {
 type PropsFromState = {
   addonId: number;
   compareInfo: CompareInfo | null | void;
-  isLoading: boolean;
   version: Version | void | null;
 };
 
@@ -58,7 +57,14 @@ export class CompareBase extends React.Component<Props> {
   }
 
   loadData(prevProps?: Props) {
-    const { _fetchDiff, dispatch, history, match, version } = this.props;
+    const {
+      _fetchDiff,
+      compareInfo,
+      dispatch,
+      history,
+      match,
+      version,
+    } = this.props;
     const { addonId, baseVersionId, headVersionId, lang } = match.params;
 
     const oldVersionId = parseInt(baseVersionId, 10);
@@ -75,12 +81,12 @@ export class CompareBase extends React.Component<Props> {
 
     const path = getPathFromQueryString(history);
 
-    if (version === null) {
+    if (compareInfo === null) {
       // An error has occured when fetching the version.
       return;
     }
 
-    if (version === undefined) {
+    if (compareInfo === undefined) {
       dispatch(
         _fetchDiff({
           addonId: parseInt(addonId, 10),
@@ -92,14 +98,13 @@ export class CompareBase extends React.Component<Props> {
       return;
     }
 
-    if (!prevProps) {
+    if (!prevProps || !version) {
       return;
     }
 
     if (
-      (prevProps &&
-        (prevProps.version &&
-          version.selectedPath !== prevProps.version.selectedPath)) ||
+      (prevProps.version &&
+        version.selectedPath !== prevProps.version.selectedPath) ||
       addonId !== prevProps.match.params.addonId ||
       baseVersionId !== prevProps.match.params.baseVersionId ||
       headVersionId !== prevProps.match.params.headVersionId
@@ -115,25 +120,25 @@ export class CompareBase extends React.Component<Props> {
     }
   }
 
-  // When selecting a new file to view, we do not want to preserve the hash in
-  // the URL (this hash highlights a specific line of code).
-  viewVersionFile = (path: string, { preserveHash = false } = {}) => {
+  viewVersionFile = (path: string) => {
     const { _viewVersionFile, dispatch, match } = this.props;
     const { headVersionId } = match.params;
 
     dispatch(
       _viewVersionFile({
         selectedPath: path,
         versionId: parseInt(headVersionId, 10),
-        preserveHash,
+        // When selecting a new file to view, we do not want to preserve the
+        // hash in the URL (this hash highlights a specific line of code).
+        preserveHash: false,
       }),
     );
   };
 
   renderLoadingMessageOrError(message: string) {
-    const { isLoading } = this.props;
+    const { compareInfo } = this.props;
 
-    if (!isLoading) {
+    if (compareInfo === null) {
       return (
         <p className={styles.error}>
           {gettext('Ooops, an error has occured.')}
@@ -190,15 +195,13 @@ const mapStateToProps = (
   const headVersionId = parseInt(match.params.headVersionId, 10);
 
   const { compareInfo } = state.versions;
-  const isLoading = compareInfo === undefined;
 
   // The Compare API returns the version info of the head/newest version.
   const version = getVersionInfo(state.versions, headVersionId);
 
   return {
     addonId,
     compareInfo,
-    isLoading,
     version,
   };
 };

src/reducers/versions.tsx

@@ -615,8 +615,8 @@ export const fetchDiff = ({
     });
 
     if (isErrorResponse(response)) {
-      dispatch(errorsActions.addError({ error: response.error }));
       dispatch(actions.abortFetchDiff());
+      dispatch(errorsActions.addError({ error: response.error }));
     } else {
       dispatch(actions.loadVersionInfo({ version: response }));
       dispatch(