Correct parseDiff types and fix the fallout

[kumar303]

The type definition of the object returned by parseDiff() (from react-diff-view) is incorrect. By inspecting the ChangeInfo objects, they look more like this:

patch

diff --git a/src/@types/react-diff-view/index.d.ts b/src/@types/react-diff-view/index.d.ts
index 0f07a6d..b888fc8 100644
--- a/src/@types/react-diff-view/index.d.ts
+++ b/src/@types/react-diff-view/index.d.ts
@@ -3,17 +3,34 @@
 declare module 'react-diff-view' {
   type ChangeType = 'delete' | 'insert' | 'normal';
 
-  type ChangeInfo = {
+  type BaseChange = {
     content: string;
-    isDelete?: boolean;
-    isInsert?: boolean;
-    isNormal?: boolean;
-    lineNumber?: number;
-    newLineNumber?: number;
-    oldLineNumber?: number;
     type: ChangeType;
   };
 
+  type ModifiedChange = BaseChange & {
+    lineNumber: number;
+  };
+
+  type DeleteChange = ModifiedChange & {
+    isDelete: boolean;
+    type: 'delete';
+  };
+
+  type InsertChange = ModifiedChange & {
+    isInsert: boolean;
+    type: 'insert';
+  };
+
+  type NoChange = BaseChange & {
+    isNormal: boolean;
+    newLineNumber: number;
+    oldLineNumber: number;
+    type: 'normal';
+  };
+
+  type ChangeInfo = DeleteChange | InsertChange | NoChange;
+
   export type HunkInfo = {
     changes: ChangeInfo[];
     content: string;

Specifically, newLineNumber and oldLineNumber are only present for changes of type=normal. For all other changes, only lineNumber is defined.

This is a problem because we have code that may be working with raw diffs incorrectly. As of https://github.com/mozilla/addons-code-manager/pull/417 this only applies to test code but that's risky since the tests do not reflect reality.

A possible fix:

• Adjust createInternalDiffs() so that it first converts an API response to a realistic parseDiff() object.
• Introduce another conversion that takes a parseDiff() object and returns something easier to work with.
• Introduce a wrapper around parseDiff() for the tests to use.

┆Issue is synchronized with this Jira Task

[mirefly]

Introduce another conversion that takes a parseDiff() object and returns something easier to work with.

Does it mean converting changes with the type DeleteChange | InsertChange | NoChange back to the previous type (the incorrect type)?

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. If you think this bug should stay open, please comment on the issue with further details. Thank you for your contributions.

[bobsilverberg]

As this is a code quality issue, I think we should keep it open. I'm not sure when someone will find time to work on it, but it would be nice to not lose track of.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. If you think this bug should stay open, please comment on the issue with further details. Thank you for your contributions.