Skip to content

Commit 3c15bbb

Browse files
improve diff lines ordering in remote deploy config diffing logic (#10401)
1 parent bd21fc5 commit 3c15bbb

File tree

3 files changed

+94
-2
lines changed

3 files changed

+94
-2
lines changed

.changeset/loud-insects-drive.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"wrangler": patch
3+
---
4+
5+
improve diff lines ordering in remote deploy config diffing logic

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,8 +164,8 @@ describe("getRemoteConfigsDiff", () => {
164164
}
165165
},
166166
\\"account_id\\": \\"account-id-123\\",
167-
- \\"workers_dev\\": true,
168167
+ \\"workers_dev\\": true
168+
- \\"workers_dev\\": true,
169169
- \\"kv_namespaces\\": [
170170
- {
171171
- \\"binding\\": \\"MY_KV\\",

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

Lines changed: 88 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,93 @@ export class Diff {
191191
}
192192
}
193193

194+
/**
195+
* Results but refined to be printed/stringified.
196+
*
197+
* In particular the results returned here are ordered to try to avoid cases
198+
* in which an addition/removal is incorrectly split.
199+
*
200+
* For example, the standard results can produce something like:
201+
* ```
202+
* ...
203+
* - "vars": {
204+
* + "vars": {},
205+
* - "MY_VAR": "variable set in the dash"
206+
* - },
207+
* ...
208+
* ```
209+
* (notice how the first removal is separated from the last two,
210+
* making the diff much less readable).
211+
* Such change in the refined results will instead look like:
212+
* ```
213+
* ...
214+
* + "vars": {},
215+
* - "vars": {
216+
* - "MY_VAR": "variable set in the dash"
217+
* - },
218+
* ...
219+
* ```
220+
*/
221+
get #resultsForPrint() {
222+
const results = [
223+
...this.#results.filter((r) => !!r.value && r.value !== "\n"),
224+
];
225+
226+
const swapLines = (i: number, j: number) => {
227+
const tmp = results[i];
228+
results[i] = results[j];
229+
results[j] = tmp;
230+
};
231+
232+
const numOfLines = (str: string) => str.split("\n").length;
233+
234+
const isLoneResult = (index: number, target: -1 | 1): boolean => {
235+
const currentIdx = index;
236+
const adjacentIdx = currentIdx + target;
237+
const nextIdx = currentIdx + target + target;
238+
239+
// If any of the results we need to analize is not present we return false
240+
if (!results[adjacentIdx] || !results[nextIdx] || !results[nextIdx]) {
241+
return false;
242+
}
243+
244+
const previousIdx = index - target;
245+
246+
const isAlternation = (type: "added" | "removed") =>
247+
results[currentIdx][type] === true &&
248+
results[previousIdx]?.[type] !== results[currentIdx][type] &&
249+
results[adjacentIdx][type === "added" ? "removed" : "added"] === true &&
250+
results[nextIdx][type] === true;
251+
252+
// If there isn't an alternation between added and removed results then we return false
253+
if (!isAlternation("added") && !isAlternation("removed")) {
254+
return false;
255+
}
256+
257+
// We might have found a lone result but to make sure we need to check that the next index
258+
// contains multiple lines while the current and adjacent ones both only contain one
259+
return (
260+
numOfLines(results[currentIdx].value ?? "") === 1 &&
261+
numOfLines(results[adjacentIdx].value ?? "") === 1 &&
262+
numOfLines(results[nextIdx].value ?? "") > 1
263+
);
264+
};
265+
266+
for (let i = 0; i < results.length; i++) {
267+
if (isLoneResult(i, +1)) {
268+
swapLines(i, i + 1);
269+
continue;
270+
}
271+
272+
if (isLoneResult(i, -1)) {
273+
swapLines(i, i - 1);
274+
continue;
275+
}
276+
}
277+
278+
return results;
279+
}
280+
194281
toString(
195282
options: {
196283
// Number of lines of context to print before and after each diff segment
@@ -203,7 +290,7 @@ export class Diff {
203290
let state: "init" | "diff" = "init";
204291
const context: string[] = [];
205292

206-
for (const result of this.#results) {
293+
for (const result of this.#resultsForPrint) {
207294
if (result.value === undefined) {
208295
continue;
209296
}

0 commit comments

Comments
 (0)