improve diff lines ordering in remote deploy config diffing logic (#10401)

Author: dario-piotrowicz

.changeset/loud-insects-drive.md

@@ -0,0 +1,5 @@
+---
+"wrangler": patch
+---
+
+improve diff lines ordering in remote deploy config diffing logic

packages/wrangler/src/__tests__/deploy/config-diffs.test.ts

@@ -164,8 +164,8 @@ describe("getRemoteConfigsDiff", () => {
 			      }
 			    },
 			    \\"account_id\\": \\"account-id-123\\",
-			-   \\"workers_dev\\": true,
 			+   \\"workers_dev\\": true
+			-   \\"workers_dev\\": true,
 			-   \\"kv_namespaces\\": [
 			-     {
 			-       \\"binding\\": \\"MY_KV\\",

packages/wrangler/src/cloudchamber/helpers/diff.ts

@@ -191,6 +191,93 @@ export class Diff {
 		}
 	}
 
+	/**
+	 * Results but refined to be printed/stringified.
+	 *
+	 * In particular the results returned here are ordered to try to avoid cases
+	 * in which an addition/removal is incorrectly split.
+	 *
+	 * For example, the standard results can produce something like:
+	 * ```
+	 *  ...
+	 * - "vars": {
+	 * + "vars": {},
+	 * -   "MY_VAR": "variable set in the dash"
+	 * - },
+	 *  ...
+	 * ```
+	 * (notice how the first removal is separated from the last two,
+	 * making the diff much less readable).
+	 * Such change in the refined results will instead look like:
+	 * ```
+	 *  ...
+	 * + "vars": {},
+	 * - "vars": {
+	 * -   "MY_VAR": "variable set in the dash"
+	 * - },
+	 *  ...
+	 * ```
+	 */
+	get #resultsForPrint() {
+		const results = [
+			...this.#results.filter((r) => !!r.value && r.value !== "\n"),
+		];
+
+		const swapLines = (i: number, j: number) => {
+			const tmp = results[i];
+			results[i] = results[j];
+			results[j] = tmp;
+		};
+
+		const numOfLines = (str: string) => str.split("\n").length;
+
+		const isLoneResult = (index: number, target: -1 | 1): boolean => {
+			const currentIdx = index;
+			const adjacentIdx = currentIdx + target;
+			const nextIdx = currentIdx + target + target;
+
+			// If any of the results we need to analize is not present we return false
+			if (!results[adjacentIdx] || !results[nextIdx] || !results[nextIdx]) {
+				return false;
+			}
+
+			const previousIdx = index - target;
+
+			const isAlternation = (type: "added" | "removed") =>
+				results[currentIdx][type] === true &&
+				results[previousIdx]?.[type] !== results[currentIdx][type] &&
+				results[adjacentIdx][type === "added" ? "removed" : "added"] === true &&
+				results[nextIdx][type] === true;
+
+			// If there isn't an alternation between added and removed results then we return false
+			if (!isAlternation("added") && !isAlternation("removed")) {
+				return false;
+			}
+
+			// We might have found a lone result but to make sure we need to check that the next index
+			// contains multiple lines while the current and adjacent ones both only contain one
+			return (
+				numOfLines(results[currentIdx].value ?? "") === 1 &&
+				numOfLines(results[adjacentIdx].value ?? "") === 1 &&
+				numOfLines(results[nextIdx].value ?? "") > 1
+			);
+		};
+
+		for (let i = 0; i < results.length; i++) {
+			if (isLoneResult(i, +1)) {
+				swapLines(i, i + 1);
+				continue;
+			}
+
+			if (isLoneResult(i, -1)) {
+				swapLines(i, i - 1);
+				continue;
+			}
+		}
+
+		return results;
+	}
+
 	toString(
 		options: {
 			// Number of lines of context to print before and after each diff segment
@@ -203,7 +290,7 @@ export class Diff {
 		let state: "init" | "diff" = "init";
 		const context: string[] = [];
 
-		for (const result of this.#results) {
+		for (const result of this.#resultsForPrint) {
 			if (result.value === undefined) {
 				continue;
 			}