fix: pull writes canonical model files and detects changes reliably

src/commands/pull.ts

@@ -88,17 +88,17 @@ export default createCommand(config, async ({ values }) => {
 		localCustomTypes.map((customType) => customType.model),
 		{
 			getKey: (model) => model.id,
-			equals: (a, b) =>
-				JSON.stringify(canonicalizeCustomType(a)) === JSON.stringify(canonicalizeCustomType(b)),
+			equals: (remote, local) =>
+				JSON.stringify(canonicalizeCustomType(remote)) === JSON.stringify(local),
 		},
 	);
 	const sliceOps = diffArrays(
 		remoteSlices,
 		localSlices.map((slice) => slice.model),
 		{
 			getKey: (model) => model.id,
-			equals: (a, b) =>
-				JSON.stringify(canonicalizeSlice(a)) === JSON.stringify(canonicalizeSlice(b)),
+			equals: (remote, local) =>
+				JSON.stringify(canonicalizeSlice(remote)) === JSON.stringify(local),
 		},
 	);
 

src/commands/sync.ts

@@ -8,6 +8,7 @@ import { getErrorMessage } from "../error";
 import { createCommand, type CommandConfig, CommandError } from "../lib/command";
 import { diffArrays } from "../lib/diff";
 import { getCustomTypes, getSlices } from "../lib/prismic/clients/custom-types";
+import { canonicalizeCustomType, canonicalizeSlice } from "../lib/prismic/models";
 import { completeOnboardingSteps } from "../lib/prismic/onboarding";
 import { getRepositoryName } from "../project";
 import { trackCommandStart, trackCommandEnd } from "../tracking";
@@ -83,7 +84,11 @@ export default createCommand(config, async ({ values }) => {
 
 				const changed: string[] = [];
 
-				const sliceOps = diffArrays(remoteSlices, localSliceModels, { getKey: (m) => m.id });
+				const sliceOps = diffArrays(remoteSlices, localSliceModels, {
+					getKey: (m) => m.id,
+					equals: (remote, local) =>
+						JSON.stringify(canonicalizeSlice(remote)) === JSON.stringify(local),
+				});
 				if (sliceOps.insert.length + sliceOps.update.length + sliceOps.delete.length > 0) {
 					for (const slice of sliceOps.update) {
 						await adapter.updateSlice(slice);
@@ -99,6 +104,8 @@ export default createCommand(config, async ({ values }) => {
 
 				const customTypeOps = diffArrays(remoteCustomTypes, localCustomTypeModels, {
 					getKey: (m) => m.id,
+					equals: (remote, local) =>
+						JSON.stringify(canonicalizeCustomType(remote)) === JSON.stringify(local),
 				});
 				if (
 					customTypeOps.insert.length + customTypeOps.update.length + customTypeOps.delete.length >

src/lib/prismic/models.ts

@@ -1,5 +1,6 @@
 import type {
 	CustomType,
+	DynamicSlices,
 	DynamicWidget,
 	Link,
 	SharedSlice,
@@ -196,8 +197,8 @@ export function canonicalizeSlice(model: SharedSlice): SharedSlice {
 		...sortKeys(model),
 		variations: model.variations.map((variation) => {
 			const sorted = sortKeys(variation);
-			if (sorted.primary) sorted.primary = canonicalizeFields(sorted.primary);
-			if (sorted.items) sorted.items = canonicalizeFields(sorted.items);
+			if (variation.primary) sorted.primary = canonicalizeFields(variation.primary);
+			if (variation.items) sorted.items = canonicalizeFields(variation.items);
 			return sorted;
 		}),
 	};
@@ -207,19 +208,51 @@ function canonicalizeFields<F extends DynamicWidget>(fields: Record<string, F>):
 	return Object.fromEntries(
 		Object.entries(fields).map(([id, field]) => {
 			const sorted = sortKeys(field);
-			if ("config" in sorted && sorted.config) {
-				sorted.config = sortKeys(sorted.config);
-				const group = sorted.config as { fields?: Fields };
-				if (group.fields) group.fields = canonicalizeFields(group.fields);
+			if (
+				field.type === "Group" &&
+				field.config?.fields &&
+				sorted.type === "Group" &&
+				sorted.config?.fields
+			) {
+				sorted.config.fields = canonicalizeFields(field.config.fields);
+			}
+			if (
+				field.type === "Slices" &&
+				field.config?.choices &&
+				sorted.type === "Slices" &&
+				sorted.config?.choices
+			) {
+				sorted.config.choices = canonicalizeChoices(field.config.choices);
+			}
+			return [id, sorted];
+		}),
+	);
+}
+
+type Choices = NonNullable<NonNullable<DynamicSlices["config"]>["choices"]>;
+
+// Entry order of a slice zone's choices is its slice order, and legacy slices
+// hold field maps of their own.
+function canonicalizeChoices(choices: Choices): Choices {
+	return Object.fromEntries(
+		Object.entries(choices).map(([id, choice]) => {
+			const sorted = sortKeys(choice);
+			if (choice.type === "Slice" && sorted.type === "Slice") {
+				if (choice["non-repeat"]) sorted["non-repeat"] = canonicalizeFields(choice["non-repeat"]);
+				if (choice.repeat) sorted.repeat = canonicalizeFields(choice.repeat);
 			}
 			return [id, sorted];
 		}),
 	);
 }
 
 function sortKeys<T>(object: T): T {
+	if (Array.isArray(object)) return object.map(sortKeys) as T;
+	if (object === null || typeof object !== "object") return object;
 	return Object.fromEntries(
-		Object.entries(object as Record<string, unknown>).sort(([a], [b]) => a.localeCompare(b)),
+		Object.entries(object)
+			.sort(([a], [b]) => a.localeCompare(b))
+			.map(([key, value]) => [key, sortKeys(value)]),
 	) as T;
 }
 

test/pull.test.ts

@@ -3,8 +3,17 @@ import { writeFile, mkdir } from "node:fs/promises";
 import { sep } from "node:path";
 import { fileURLToPath } from "node:url";
 import { x } from "tinyexec";
+import { describe } from "vitest";
 
-import { buildCustomType, buildSlice, it } from "./it";
+import {
+	buildCustomType,
+	buildSlice,
+	it,
+	readLocalCustomType,
+	readLocalSlice,
+	writeLocalCustomType,
+	writeLocalSlice,
+} from "./it";
 import {
 	deleteCustomType,
 	deleteSlice,
@@ -250,6 +259,150 @@ it.sequential("removes route when page type is deleted", async ({
 	await expect(project).not.toHaveRoute({ type: customType.id });
 });
 
+describe("with an isolated repository", () => {
+	it.scoped({ isolateRepo: true });
+
+	it("writes canonical model files that later pulls leave untouched", async ({
+		expect,
+		project,
+		prismic,
+		repo,
+		token,
+		host,
+	}) => {
+		const slice = buildSlice({ id: "zeta-slice", name: "ZetaSlice" });
+		slice.variations[0].primary = {
+			title: { type: "Text", config: { placeholder: "Enter a title", label: "Title" } },
+			subtitle: { type: "Text", config: { placeholder: "Enter a subtitle", label: "Subtitle" } },
+		};
+		const customType = buildCustomType({
+			format: "custom",
+			json: {
+				Main: {
+					social_image: {
+						type: "Image",
+						config: {
+							label: "Social image",
+							constraint: { width: 1200, height: 630 },
+							thumbnails: [
+								{ name: "small", width: 100, height: 50 },
+								{ name: "large", width: 400, height: 200 },
+							],
+						},
+					},
+					links: {
+						type: "Group",
+						config: {
+							label: "Links",
+							fields: {
+								url: { type: "Text", config: { placeholder: "", label: "URL" } },
+								label: { type: "Text", config: { placeholder: "", label: "Label" } },
+							},
+						},
+					},
+					slices: {
+						type: "Slices",
+						fieldset: "Slice Zone",
+						config: {
+							choices: {
+								[slice.id]: { type: "SharedSlice" },
+								legacy_banner: {
+									type: "Slice",
+									fieldset: "Legacy banner",
+									"non-repeat": {
+										title: { type: "Text", config: { placeholder: "", label: "Title" } },
+										caption: { type: "Text", config: { placeholder: "", label: "Caption" } },
+									},
+								},
+							},
+						},
+					},
+				},
+				Details: {
+					author: { type: "Text", config: { label: "Author", placeholder: "" } },
+				},
+			},
+		});
+
+		await Promise.all([
+			writeLocalCustomType(project, customType),
+			writeLocalSlice(project, slice),
+			insertCustomType(customType, { repo, token, host }),
+			insertSlice(slice, { repo, token, host }),
+		]);
+
+		const first = await prismic("pull", ["--repo", repo, "--force"]);
+		expect(first.exitCode, first.stderr).toBe(0);
+
+		// oxlint-disable-next-line typescript-eslint/no-explicit-any
+		const writtenType: Record<string, any> = await readLocalCustomType(project, customType.id);
+		// oxlint-disable-next-line typescript-eslint/no-explicit-any
+		const writtenSlice: Record<string, any> | undefined = await readLocalSlice(project, slice.id);
+		if (!writtenSlice) throw new Error(`Slice "${slice.id}" was not pulled.`);
+
+		expect(writtenType).toEqual(customType);
+		expect(Object.keys(writtenType)).toEqual([
+			"format",
+			"id",
+			"json",
+			"label",
+			"repeatable",
+			"status",
+		]);
+		expect(Object.keys(writtenType.json)).toEqual(["Main", "Details"]);
+		expect(Object.keys(writtenType.json.Main)).toEqual(["social_image", "links", "slices"]);
+		expect(Object.keys(writtenType.json.Main.social_image.config)).toEqual([
+			"constraint",
+			"label",
+			"thumbnails",
+		]);
+		expect(Object.keys(writtenType.json.Main.social_image.config.constraint)).toEqual([
+			"height",
+			"width",
+		]);
+		expect(Object.keys(writtenType.json.Main.social_image.config.thumbnails[0])).toEqual([
+			"height",
+			"name",
+			"width",
+		]);
+		expect(Object.keys(writtenType.json.Main.links.config.fields)).toEqual(["url", "label"]);
+		expect(Object.keys(writtenType.json.Main.links.config.fields.url.config)).toEqual([
+			"label",
+			"placeholder",
+		]);
+		expect(Object.keys(writtenType.json.Main.slices)).toEqual(["config", "fieldset", "type"]);
+
+		const choices = writtenType.json.Main.slices.config.choices;
+		expect(Object.keys(choices)).toEqual([slice.id, "legacy_banner"]);
+		expect(Object.keys(choices.legacy_banner)).toEqual(["fieldset", "non-repeat", "type"]);
+		expect(Object.keys(choices.legacy_banner["non-repeat"])).toEqual(["title", "caption"]);
+		expect(Object.keys(choices.legacy_banner["non-repeat"].title.config)).toEqual([
+			"label",
+			"placeholder",
+		]);
+
+		expect(writtenSlice).toEqual(slice);
+		expect(Object.keys(writtenSlice.variations[0])).toEqual([
+			"description",
+			"docURL",
+			"id",
+			"imageUrl",
+			"name",
+			"primary",
+			"version",
+		]);
+		expect(Object.keys(writtenSlice.variations[0].primary)).toEqual(["title", "subtitle"]);
+
+		const second = await prismic("pull", ["--repo", repo]);
+		expect(second.exitCode, second.stderr).toBe(0);
+		expect(second.stdout).toContain("Already up to date.");
+		const typeAfter = await readLocalCustomType(project, customType.id);
+		const sliceAfter = await readLocalSlice(project, slice.id);
+		expect(JSON.stringify(typeAfter)).toBe(JSON.stringify(writtenType));
+		expect(JSON.stringify(sliceAfter)).toBe(JSON.stringify(writtenSlice));
+	});
+});
+
 it.sequential("blocks pull when local model files have uncommitted changes", async ({
 	expect,
 	project,

test/status.test.ts

@@ -1,6 +1,6 @@
 import { describe } from "vitest";
 
-import { buildCustomType, buildSlice, it, readLocalCustomType, writeLocalCustomType } from "./it";
+import { buildCustomType, buildSlice, it, writeLocalCustomType, writeLocalSlice } from "./it";
 import { insertCustomType, insertSlice } from "./prismic";
 
 it("supports --help", async ({ expect, prismic }) => {
@@ -62,52 +62,60 @@ describe("with an isolated repository", () => {
 		expect(stdout).toContain("prismic pull");
 	});
 
-	it("reports in-sync when local only reorders metadata and config keys", async ({
+	it("reports in-sync and push writes nothing when local only reorders keys", async ({
 		expect,
 		project,
 		prismic,
 		repo,
 		token,
 		host,
 	}) => {
-		// A field with multiple config keys, so config key order can be reordered.
 		const customType = buildCustomType({
+			format: "custom",
 			json: {
 				Main: {
 					title: { type: "Text", config: { label: "Title", placeholder: "Enter a title" } },
+					social_image: {
+						type: "Image",
+						config: {
+							label: "Social image",
+							constraint: { width: 1200, height: 630 },
+							thumbnails: [{ name: "small", width: 100, height: 50 }],
+						},
+					},
+					links: {
+						type: "Group",
+						config: {
+							label: "Links",
+							fields: {
+								url: { type: "Text", config: { placeholder: "", label: "URL" } },
+								label: { type: "Text", config: { placeholder: "", label: "Label" } },
+							},
+						},
+					},
 				},
 			},
 		} as Partial<ReturnType<typeof buildCustomType>>);
-		await insertCustomType(customType, { repo, token, host });
+		const slice = buildSlice();
+		slice.variations[0].primary = {
+			title: { type: "Text", config: { placeholder: "Enter a title", label: "Title" } },
+		};
+		expect(Object.keys(customType)).not.toEqual(Object.keys(customType).sort());
 
-		// Pull writes the canonical form to disk.
-		const pull = await prismic("pull", ["--repo", repo]);
-		expect(pull.exitCode, pull.stderr).toBe(0);
+		await writeLocalCustomType(project, customType);
+		await writeLocalSlice(project, slice);
+		await Promise.all([
+			insertCustomType(customType, { repo, token, host }),
+			insertSlice(slice, { repo, token, host }),
+		]);
 
-		// Hand-edit the local file: reverse the order of metadata keys and of each
-		// field's config keys, leaving all values and the field order unchanged.
-		const pulled = await readLocalCustomType(project, customType.id);
-		// Pull writes the canonical (sorted-key) form, not the raw API key order.
-		expect(Object.keys(pulled)).toEqual(Object.keys(pulled).sort());
-		const canonical = JSON.stringify(pulled, null, 2);
-		for (const fields of Object.values(pulled.json)) {
-			for (const field of Object.values(fields)) {
-				const f = field as { config?: Record<string, unknown> };
-				if (f.config) {
-					expect(Object.keys(f.config)).toEqual(Object.keys(f.config).sort());
-					f.config = Object.fromEntries(Object.entries(f.config).reverse());
-				}
-			}
-		}
-		const scrambled = Object.fromEntries(Object.entries(pulled).reverse()) as typeof pulled;
-		// Confirm the hand-edit really produced a non-canonical file.
-		expect(JSON.stringify(scrambled, null, 2)).not.toBe(canonical);
-		await writeLocalCustomType(project, scrambled);
-
-		// Both sides canonicalize equal, so status must report no changes.
 		const { stdout, stderr, exitCode } = await prismic("status", ["--repo", repo]);
 		expect(exitCode, stderr).toBe(0);
 		expect(stdout).toContain("Already up to date.");
+
+		const push = await prismic("push", ["--repo", repo]);
+		expect(push.exitCode, push.stderr).toBe(0);
+		expect(push.stdout).toContain("Already up to date.");
 	});
 
 	it("reports differing models when local and remote disagree", async ({