fix: preserve aliased input occurrence identities
This commit is contained in:
@@ -1,7 +1,7 @@
|
||||
import { cleanup, render, screen } from "@testing-library/react";
|
||||
import userEvent from "@testing-library/user-event";
|
||||
import { useState } from "react";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { normalizeSchema } from "../schema-form/schema-field.js";
|
||||
import type { ExpressionEditorState } from "./input-expression-editor.js";
|
||||
import { InputExpressionControl } from "./InputExpressionControl.js";
|
||||
@@ -265,6 +265,95 @@ describe("InputExpressionControl", () => {
|
||||
expect(screen.getByRole("textbox", { name: "Additional property name for items item 1" })).toHaveValue("");
|
||||
});
|
||||
|
||||
it("gives repeated object references distinct keys across external array changes", async () => {
|
||||
const shared = { kind: "object", fields: [] } satisfies ExpressionEditorState;
|
||||
const user = userEvent.setup();
|
||||
const consoleError = vi.spyOn(console, "error").mockImplementation(() => undefined);
|
||||
const Harness = () => {
|
||||
const [state, setState] = useState<ExpressionEditorState>({ kind: "array", items: [shared, shared] });
|
||||
return (
|
||||
<>
|
||||
<button
|
||||
onClick={() => setState({ kind: "array", items: state.kind === "array" ? [...state.items].reverse() : [] })}
|
||||
type="button"
|
||||
>
|
||||
External reorder aliases
|
||||
</button>
|
||||
<button
|
||||
onClick={() => setState({ kind: "array", items: state.kind === "array" ? state.items.slice(1) : [] })}
|
||||
type="button"
|
||||
>
|
||||
External remove alias
|
||||
</button>
|
||||
<button
|
||||
onClick={() => setState({ kind: "array", items: state.kind === "array" ? [...state.items, shared] : [] })}
|
||||
type="button"
|
||||
>
|
||||
External add alias
|
||||
</button>
|
||||
<button
|
||||
onClick={() => setState({ kind: "array", items: [{ kind: "object", fields: [] }, shared] })}
|
||||
type="button"
|
||||
>
|
||||
External replace alias
|
||||
</button>
|
||||
<InputExpressionControl
|
||||
field={normalizeSchema({ type: "array", items: { type: "object", additionalProperties: true } })}
|
||||
label="items"
|
||||
onChange={setState}
|
||||
state={state}
|
||||
/>
|
||||
</>
|
||||
);
|
||||
};
|
||||
|
||||
render(<Harness />);
|
||||
await user.click(screen.getByRole("button", { name: "External reorder aliases" }));
|
||||
await user.click(screen.getByRole("button", { name: "External remove alias" }));
|
||||
await user.click(screen.getByRole("button", { name: "External add alias" }));
|
||||
await user.click(screen.getByRole("button", { name: "External replace alias" }));
|
||||
|
||||
expect(consoleError.mock.calls.flat().join(" ")).not.toContain("same key");
|
||||
consoleError.mockRestore();
|
||||
});
|
||||
|
||||
it("transfers the edited occurrence identity when aliased children diverge", async () => {
|
||||
const shared = {
|
||||
kind: "object",
|
||||
fields: [{ name: "value", value: { kind: "literal", value: "", touched: false } }],
|
||||
} satisfies ExpressionEditorState;
|
||||
const user = userEvent.setup();
|
||||
const Harness = () => {
|
||||
const [state, setState] = useState<ExpressionEditorState>({ kind: "array", items: [shared, shared] });
|
||||
return (
|
||||
<>
|
||||
<button
|
||||
onClick={() => setState({ kind: "array", items: state.kind === "array" ? [...state.items].reverse() : [] })}
|
||||
type="button"
|
||||
>
|
||||
External reorder after child edit
|
||||
</button>
|
||||
<InputExpressionControl
|
||||
field={normalizeSchema({
|
||||
type: "array",
|
||||
items: { type: "object", properties: { value: { type: "string" } }, additionalProperties: true },
|
||||
})}
|
||||
label="items"
|
||||
onChange={setState}
|
||||
state={state}
|
||||
/>
|
||||
</>
|
||||
);
|
||||
};
|
||||
|
||||
render(<Harness />);
|
||||
await user.type(screen.getByRole("textbox", { name: "Additional property name for items item 2" }), "second-local");
|
||||
await user.type(screen.getAllByRole("textbox", { name: "Value" })[1]!, "changed");
|
||||
await user.click(screen.getByRole("button", { name: "External reorder after child edit" }));
|
||||
|
||||
expect(screen.getByRole("textbox", { name: "Additional property name for items item 1" })).toHaveValue("second-local");
|
||||
});
|
||||
|
||||
it("gives repeated labels unique datalist and typed-leaf control ids", () => {
|
||||
const field = normalizeSchema({ type: "string" });
|
||||
render(
|
||||
|
||||
@@ -80,6 +80,76 @@ const safeIdSuffix = (value: string): string =>
|
||||
|
||||
const EMPTY_FIELD_SOURCES: FieldSources = {};
|
||||
|
||||
type ExpressionItemIdentityCache = {
|
||||
readonly byState: WeakMap<ExpressionEditorState, string[]>;
|
||||
previousItems: ReadonlyArray<ExpressionEditorState>;
|
||||
previousIds: ReadonlyArray<string>;
|
||||
nextId: number;
|
||||
};
|
||||
|
||||
const allocateItemId = (
|
||||
cache: ExpressionItemIdentityCache,
|
||||
item: ExpressionEditorState,
|
||||
controlId: string,
|
||||
): string => {
|
||||
const id = `${controlId}-item-${cache.nextId}`;
|
||||
cache.nextId += 1;
|
||||
const knownIds = cache.byState.get(item) ?? [];
|
||||
knownIds.push(id);
|
||||
cache.byState.set(item, knownIds);
|
||||
return id;
|
||||
};
|
||||
|
||||
/** Reconcile by reference and occurrence, keeping aliased array slots unique. */
|
||||
const reconcileArrayItemIds = (
|
||||
cache: ExpressionItemIdentityCache,
|
||||
items: ReadonlyArray<ExpressionEditorState>,
|
||||
controlId: string,
|
||||
): ReadonlyArray<string> => {
|
||||
const usedPreviousIndices = new Set<number>();
|
||||
const usedIds = new Set<string>();
|
||||
const ids: string[] = [];
|
||||
for (const [index, item] of items.entries()) {
|
||||
let previousIndex = -1;
|
||||
if (cache.previousItems[index] === item && !usedPreviousIndices.has(index)) {
|
||||
previousIndex = index;
|
||||
} else {
|
||||
let nearestDistance = Number.POSITIVE_INFINITY;
|
||||
for (const [candidateIndex, candidate] of cache.previousItems.entries()) {
|
||||
const distance = Math.abs(candidateIndex - index);
|
||||
if (
|
||||
candidate === item &&
|
||||
!usedPreviousIndices.has(candidateIndex) &&
|
||||
distance < nearestDistance
|
||||
) {
|
||||
previousIndex = candidateIndex;
|
||||
nearestDistance = distance;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
let id = previousIndex >= 0 ? cache.previousIds[previousIndex] : undefined;
|
||||
if (id !== undefined && usedIds.has(id)) id = undefined;
|
||||
if (id === undefined) {
|
||||
const knownIds = cache.byState.get(item) ?? [];
|
||||
id = knownIds.find((candidate) => !usedIds.has(candidate));
|
||||
}
|
||||
if (id === undefined) id = allocateItemId(cache, item, controlId);
|
||||
|
||||
if (previousIndex >= 0) usedPreviousIndices.add(previousIndex);
|
||||
usedIds.add(id);
|
||||
ids.push(id);
|
||||
const knownIds = cache.byState.get(item) ?? [];
|
||||
if (!knownIds.includes(id)) {
|
||||
knownIds.push(id);
|
||||
cache.byState.set(item, knownIds);
|
||||
}
|
||||
}
|
||||
cache.previousItems = items;
|
||||
cache.previousIds = ids;
|
||||
return ids;
|
||||
};
|
||||
|
||||
const InputExpressionLeaf = ({
|
||||
field,
|
||||
label,
|
||||
@@ -171,23 +241,22 @@ export const InputExpressionControl = ({
|
||||
}: InputExpressionControlProps) => {
|
||||
const controlId = `input-expression-${safeIdSuffix(useId())}`;
|
||||
const [itemIdentity] = useState(() => ({
|
||||
byState: new WeakMap<ExpressionEditorState, string>(),
|
||||
byState: new WeakMap<ExpressionEditorState, string[]>(),
|
||||
previousItems: [],
|
||||
previousIds: [],
|
||||
nextId: 0,
|
||||
}));
|
||||
}) satisfies ExpressionItemIdentityCache);
|
||||
const [additionalName, setAdditionalName] = useState("");
|
||||
const identityForItem = (item: ExpressionEditorState): string => {
|
||||
const existing = itemIdentity.byState.get(item);
|
||||
if (existing !== undefined) return existing;
|
||||
const id = `${controlId}-item-${itemIdentity.nextId}`;
|
||||
itemIdentity.nextId += 1;
|
||||
itemIdentity.byState.set(item, id);
|
||||
return id;
|
||||
};
|
||||
const arrayItemIds = state.kind === "array"
|
||||
? state.items.map(identityForItem)
|
||||
? reconcileArrayItemIds(itemIdentity, state.items, controlId)
|
||||
: [];
|
||||
const rememberItemIdentity = (item: ExpressionEditorState, id: string | undefined): void => {
|
||||
if (id !== undefined) itemIdentity.byState.set(item, id);
|
||||
if (id === undefined) return;
|
||||
const knownIds = itemIdentity.byState.get(item) ?? [];
|
||||
if (!knownIds.includes(id)) {
|
||||
knownIds.unshift(id);
|
||||
itemIdentity.byState.set(item, knownIds);
|
||||
}
|
||||
};
|
||||
const source = valueSourceFor(state);
|
||||
const selectSource = (next: "path" | "literal" | "construct"): void =>
|
||||
@@ -281,9 +350,7 @@ export const InputExpressionControl = ({
|
||||
className="schema-form__secondary-action"
|
||||
onClick={() => {
|
||||
const item = defaultExpressionEditorState(field?.item ?? null);
|
||||
const itemId = `${controlId}-item-${itemIdentity.nextId}`;
|
||||
itemIdentity.nextId += 1;
|
||||
itemIdentity.byState.set(item, itemId);
|
||||
allocateItemId(itemIdentity, item, controlId);
|
||||
onChange({ kind: "array", items: [...state.items, item] });
|
||||
}}
|
||||
type="button"
|
||||
|
||||
Reference in New Issue
Block a user