diff --git a/src/commands/pull.ts b/src/commands/pull.ts index d0aa8fd..85ff1c2 100644 --- a/src/commands/pull.ts +++ b/src/commands/pull.ts @@ -88,8 +88,7 @@ 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( @@ -97,8 +96,7 @@ export default createCommand(config, async ({ values }) => { 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), }, ); diff --git a/src/commands/sync.ts b/src/commands/sync.ts index 20c79c4..2174292 100644 --- a/src/commands/sync.ts +++ b/src/commands/sync.ts @@ -9,6 +9,7 @@ import { createCommand, type CommandConfig, CommandError } from "../lib/command" import { diffArrays } from "../lib/diff"; import { getCustomTypes, getSlices } from "../lib/prismic/clients/custom-types"; import { completeOnboardingStepsSilently } from "../lib/prismic/clients/repository"; +import { canonicalizeCustomType, canonicalizeSlice } from "../lib/prismic/models"; 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 > diff --git a/src/lib/prismic/models.ts b/src/lib/prismic/models.ts index 4de3aec..52db618 100644 --- a/src/lib/prismic/models.ts +++ b/src/lib/prismic/models.ts @@ -196,8 +196,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 +207,28 @@ function canonicalizeFields(fields: Record): 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); } return [id, sorted]; }), ); } +// Sorts keys recursively. Entry order of field maps encodes field position, +// so callers restore those from the unsorted input. function sortKeys(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).sort(([a], [b]) => a.localeCompare(b)), + Object.entries(object) + .sort(([a], [b]) => a.localeCompare(b)) + .map(([key, value]) => [key, sortKeys(value)]), ) as T; } diff --git a/test/it.ts b/test/it.ts index 813fb6f..e3a3513 100644 --- a/test/it.ts +++ b/test/it.ts @@ -218,6 +218,22 @@ export function buildSlice(overrides?: Partial): SharedSlice { }; } +// Reverses object key order at every depth, except field maps (`json` tabs, +// group `fields`, and slice variation `primary`/`items`), whose entry order is +// position-significant. +export function scramble(value: T, keepOrder = 0): T { + if (Array.isArray(value)) return value.map((child) => scramble(child)) as T; + if (value === null || typeof value !== "object") return value; + const entries = Object.entries(value).map(([key, child]) => [ + key, + scramble( + child, + key === "json" ? 2 : ["fields", "primary", "items"].includes(key) ? 1 : keepOrder - 1, + ), + ]); + return Object.fromEntries(keepOrder > 0 ? entries : entries.reverse()); +} + export async function writeLocalCustomType(project: URL, model: CustomType): Promise { const path = new URL(`customtypes/${model.id}/index.json`, project); await mkdir(new URL(".", path), { recursive: true }); diff --git a/test/pull.test.ts b/test/pull.test.ts index a306739..a9fd662 100644 --- a/test/pull.test.ts +++ b/test/pull.test.ts @@ -1,10 +1,11 @@ import { pascalCase } from "change-case"; -import { writeFile, mkdir } from "node:fs/promises"; +import { readFile, 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, scramble } from "./it"; import { deleteCustomType, deleteSlice, @@ -250,6 +251,103 @@ 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, + }) => { + // Nested config objects with unsorted keys, plus field maps (tab, group, + // slice primary) whose entry order must be kept as-is. + const customType = buildCustomType({ + json: { + Main: { + 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: { label: "URL", placeholder: "" } }, + label: { type: "Text", config: { label: "Label", placeholder: "" } }, + }, + }, + }, + }, + }, + } as Partial>); + const slice = buildSlice(); + slice.variations[0].primary = { + title: { type: "Text", config: { placeholder: "Enter a title", label: "Title" } }, + subtitle: { type: "Text", config: { placeholder: "Enter a subtitle", label: "Subtitle" } }, + }; + + await Promise.all([ + insertCustomType(customType, { repo, token, host }), + insertSlice(slice, { repo, token, host }), + ]); + + const first = await prismic("pull", ["--repo", repo]); + expect(first.exitCode, first.stderr).toBe(0); + + const typePath = new URL(`customtypes/${customType.id}/index.json`, project); + const slicePath = new URL(`slices/${pascalCase(slice.name)}/model.json`, project); + const pulledType = await readFile(typePath, "utf8"); + const pulledSlice = await readFile(slicePath, "utf8"); + + // Metadata and config keys are sorted; field order is kept. + const writtenType = JSON.parse(pulledType); + expect(Object.keys(writtenType.json.Main)).toEqual(["social_image", "links"]); + expect(Object.keys(writtenType.json.Main.social_image.config.constraint)).toEqual([ + "height", + "width", + ]); + expect(Object.keys(writtenType.json.Main.links.config.fields)).toEqual(["url", "label"]); + const writtenSlice = JSON.parse(pulledSlice); + expect(Object.keys(writtenSlice.variations[0].primary)).toEqual(["title", "subtitle"]); + + // A second pull with no changes on either side must not touch the files. + const second = await prismic("pull", ["--repo", repo]); + expect(second.exitCode, second.stderr).toBe(0); + expect(second.stdout).toContain("Already up to date."); + expect(await readFile(typePath, "utf8")).toBe(pulledType); + expect(await readFile(slicePath, "utf8")).toBe(pulledSlice); + + // Files with non-canonical key order count as updates. This project has + // no git repo to protect local edits, so a plain pull refuses; --force + // writes the files back in canonical form. + const scrambledType = JSON.stringify(scramble(JSON.parse(pulledType)), null, 2); + const scrambledSlice = JSON.stringify(scramble(JSON.parse(pulledSlice)), null, 2); + expect(scrambledType).not.toBe(pulledType); + expect(scrambledSlice).not.toBe(pulledSlice); + await writeFile(typePath, scrambledType); + await writeFile(slicePath, scrambledSlice); + + const blocked = await prismic("pull", ["--repo", repo]); + expect(blocked.exitCode).toBe(1); + expect(blocked.stderr).toContain("--force"); + + const rewrite = await prismic("pull", ["--repo", repo, "--force"]); + expect(rewrite.exitCode, rewrite.stderr).toBe(0); + expect(rewrite.stdout).toContain("updated 1, deleted 0 types"); + expect(rewrite.stdout).toContain("updated 1, deleted 0 slices"); + expect(await readFile(typePath, "utf8")).toBe(pulledType); + expect(await readFile(slicePath, "utf8")).toBe(pulledSlice); + }); +}); + it.sequential("blocks pull when local model files have uncommitted changes", async ({ expect, project, diff --git a/test/status.test.ts b/test/status.test.ts index a4dca38..e5d2947 100644 --- a/test/status.test.ts +++ b/test/status.test.ts @@ -1,6 +1,15 @@ import { describe } from "vitest"; -import { buildCustomType, buildSlice, it, readLocalCustomType, writeLocalCustomType } from "./it"; +import { + buildCustomType, + buildSlice, + it, + readLocalCustomType, + readLocalSlice, + scramble, + writeLocalCustomType, + writeLocalSlice, +} from "./it"; import { insertCustomType, insertSlice } from "./prismic"; it("supports --help", async ({ expect, prismic }) => { @@ -62,7 +71,7 @@ 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, @@ -70,44 +79,67 @@ describe("with an isolated repository", () => { token, host, }) => { - // A field with multiple config keys, so config key order can be reordered. const customType = buildCustomType({ 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: { label: "URL", placeholder: "" } }, + label: { type: "Text", config: { label: "Label", placeholder: "" } }, + }, + }, + }, }, }, } as Partial>); - await insertCustomType(customType, { repo, token, host }); + const slice = buildSlice(); + slice.variations[0].primary = { + title: { type: "Text", config: { placeholder: "Enter a title", label: "Title" } }, + }; + await Promise.all([ + insertCustomType(customType, { repo, token, host }), + insertSlice(slice, { repo, token, host }), + ]); // Pull writes the canonical form to disk. const pull = await prismic("pull", ["--repo", repo]); expect(pull.exitCode, pull.stderr).toBe(0); - // 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 }; - 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; + // Hand-edit the local files: reverse key order at every depth, leaving all + // values and the field order unchanged. + const pulledType = await readLocalCustomType(project, customType.id); + const scrambledType = scramble(pulledType); // Confirm the hand-edit really produced a non-canonical file. - expect(JSON.stringify(scrambled, null, 2)).not.toBe(canonical); - await writeLocalCustomType(project, scrambled); + expect(JSON.stringify(scrambledType, null, 2)).not.toBe(JSON.stringify(pulledType, null, 2)); + await writeLocalCustomType(project, scrambledType); + + const pulledSlice = await readLocalSlice(project, slice.id); + if (!pulledSlice) throw new Error(`Slice "${slice.id}" was not pulled.`); + const scrambledSlice = scramble(pulledSlice); + expect(JSON.stringify(scrambledSlice, null, 2)).not.toBe(JSON.stringify(pulledSlice, null, 2)); + await writeLocalSlice(project, scrambledSlice); // 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."); + + // Push uses the same comparison, so it must not update the remote models. + 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 ({ diff --git a/test/sync.test.ts b/test/sync.test.ts index 1bfb8da..cacf941 100644 --- a/test/sync.test.ts +++ b/test/sync.test.ts @@ -36,5 +36,17 @@ describe("with an isolated repository", () => { await expect(project).toContainCustomType(customType); await expect(project).toContainSlice(slice); + + // A later remote change must only re-sync models that actually changed. + // The custom type's local file has canonical key order while the remote + // returns its own order, so this fails if sync compares raw JSON. + const outputLengthBeforeSliceB = output().length; + const newOutput = () => output().slice(outputLengthBeforeSliceB); + const sliceB = buildSlice(); + await insertSlice(sliceB, { repo, token, host }); + + await expect.poll(newOutput, { timeout: 30_000 }).toContain("Changes detected in slices"); + expect(newOutput()).not.toContain("custom types"); + await expect(project).toContainSlice(sliceB); }, 60_000); });