mirror of
https://github.com/webstudio-is/webstudio.git
synced 2026-10-23 03:27:27 +09:00
fix: Save MDX component edits without revision conflicts
This commit is contained in:
1 parent
a8f9a2af14
commit
50093ed2b7
9 files changed
+223
-91
No files matched your search
@@ -1,8 +1,17 @@
|
||||
import { afterEach, beforeEach, expect, test, vi } from "vitest";
|
||||
import { updateProjectAssetContent } from "@webstudio-is/http-client";
|
||||
import { $authToken } from "~/shared/nano-states";
|
||||
import { $project } from "~/shared/sync/data-stores";
|
||||
import { createUpdateAssetContent } from "./update-asset-content";
|
||||
import { $assets, $pages, $project } from "~/shared/sync/data-stores";
|
||||
import { createDefaultPages } from "@webstudio-is/project-build";
|
||||
import {
|
||||
commitAssetContentUpdate,
|
||||
createUpdateAssetContent,
|
||||
} from "./update-asset-content";
|
||||
import {
|
||||
externalContentSyncStore,
|
||||
registerContainers,
|
||||
serverSyncStore,
|
||||
} from "~/shared/sync/sync-stores";
|
||||
|
||||
const requestContentUpdate = vi.fn<typeof updateProjectAssetContent>();
|
||||
const commitUpdatedAsset = vi.fn();
|
||||
@@ -63,6 +72,28 @@ test("commits a content revision without changing the asset id", async () => {
|
||||
expect(commitUpdatedAsset).toHaveBeenCalledWith(revision);
|
||||
});
|
||||
|
||||
test("shares an already saved revision without writing it to the server again", () => {
|
||||
registerContainers();
|
||||
$pages.set(createDefaultPages({ rootInstanceId: "body" }));
|
||||
$assets.set(new Map([[asset.id, asset]]));
|
||||
const serverWrite = vi.fn();
|
||||
const canvasUpdate = vi.fn();
|
||||
const unsubscribeServer = serverSyncStore.subscribe(serverWrite);
|
||||
const unsubscribeCanvas = externalContentSyncStore.subscribe(canvasUpdate);
|
||||
try {
|
||||
const revision = { ...asset, name: "settings_new.json" };
|
||||
commitAssetContentUpdate(revision);
|
||||
expect($assets.get().get(asset.id)).toEqual(revision);
|
||||
expect(canvasUpdate).toHaveBeenCalledTimes(1);
|
||||
expect(serverWrite).not.toHaveBeenCalled();
|
||||
} finally {
|
||||
unsubscribeServer();
|
||||
unsubscribeCanvas();
|
||||
$assets.set(new Map());
|
||||
$pages.set(undefined);
|
||||
}
|
||||
});
|
||||
|
||||
test("does not commit a conflicting revision", async () => {
|
||||
requestContentUpdate.mockRejectedValue(new Error("File changed"));
|
||||
|
||||
|
||||
@@ -3,10 +3,10 @@ import { updateProjectAssetContent } from "@webstudio-is/http-client";
|
||||
import { fetch } from "~/shared/fetch.client";
|
||||
import { $authToken } from "~/shared/nano-states";
|
||||
import { $project } from "~/shared/sync/data-stores";
|
||||
import { createTransactionFromBuilderPatchPayload } from "~/shared/sync/builder-patch";
|
||||
import { createSyncChangesFromBuilderPatchPayload } from "~/shared/sync/builder-patch";
|
||||
import { getWebstudioData } from "~/shared/instance-utils/data";
|
||||
import { invalidateAssets } from "~/shared/resources";
|
||||
import { onNextTransactionComplete } from "~/shared/sync/project-queue";
|
||||
import { externalContentSyncStore } from "~/shared/sync/sync-stores";
|
||||
|
||||
type UpdateAssetContentDependencies = {
|
||||
requestContentUpdate: typeof updateProjectAssetContent;
|
||||
@@ -54,10 +54,11 @@ export const createUpdateAssetContent =
|
||||
return updatedAsset;
|
||||
};
|
||||
|
||||
export const updateAssetContent = createUpdateAssetContent({
|
||||
requestContentUpdate: updateProjectAssetContent,
|
||||
commitUpdatedAsset: (updatedAsset) => {
|
||||
createTransactionFromBuilderPatchPayload({
|
||||
export const commitAssetContentUpdate = (updatedAsset: Asset) => {
|
||||
// The content endpoint already saved this revision. Share it with the canvas
|
||||
// without a second server write that could restore an older revision.
|
||||
externalContentSyncStore.createTransactionFromChanges(
|
||||
createSyncChangesFromBuilderPatchPayload({
|
||||
data: getWebstudioData(),
|
||||
payload: [
|
||||
{
|
||||
@@ -71,7 +72,12 @@ export const updateAssetContent = createUpdateAssetContent({
|
||||
],
|
||||
},
|
||||
],
|
||||
});
|
||||
onNextTransactionComplete(invalidateAssets);
|
||||
},
|
||||
})
|
||||
);
|
||||
invalidateAssets();
|
||||
};
|
||||
|
||||
export const updateAssetContent = createUpdateAssetContent({
|
||||
requestContentUpdate: updateProjectAssetContent,
|
||||
commitUpdatedAsset: commitAssetContentUpdate,
|
||||
});
|
||||
@@ -2,6 +2,7 @@ import { act } from "react-dom/test-utils";
|
||||
import { createRoot, type Root } from "react-dom/client";
|
||||
import { afterEach, describe, expect, test, vi } from "vitest";
|
||||
import { ContentEditable } from "@lexical/react/LexicalContentEditable";
|
||||
import { userEvent } from "@vitest/browser/context";
|
||||
import {
|
||||
blockComponent,
|
||||
blockTemplateComponent,
|
||||
@@ -122,57 +123,70 @@ describe("TextEditor", () => {
|
||||
);
|
||||
});
|
||||
|
||||
test("does not save the active slash-menu trigger on blur", async () => {
|
||||
const instances = createContentBlockInstances();
|
||||
$instances.set(instances);
|
||||
selectInstance(["current", "block"]);
|
||||
$textEditingInstanceSelector.set({
|
||||
selector: ["current", "block"],
|
||||
reason: "left",
|
||||
});
|
||||
const onChange = vi.fn();
|
||||
const container = document.createElement("div");
|
||||
document.body.appendChild(container);
|
||||
root = createRoot(container);
|
||||
test.each(["element", "window"])(
|
||||
"saves text without the slash trigger on %s blur",
|
||||
async (target) => {
|
||||
const instances = createContentBlockInstances();
|
||||
$instances.set(instances);
|
||||
selectInstance(["current", "block"]);
|
||||
$textEditingInstanceSelector.set({
|
||||
selector: ["current", "block"],
|
||||
reason: "left",
|
||||
});
|
||||
const onChange = vi.fn();
|
||||
const container = document.createElement("div");
|
||||
document.body.appendChild(container);
|
||||
root = createRoot(container);
|
||||
|
||||
await act(async () => {
|
||||
root?.render(
|
||||
<TextEditor
|
||||
rootInstanceSelector={["current", "block"]}
|
||||
instances={instances}
|
||||
props={new Map()}
|
||||
contentEditable={<ContentEditable />}
|
||||
onChange={onChange}
|
||||
onSelectInstance={() => {}}
|
||||
/>
|
||||
await act(async () => {
|
||||
root?.render(
|
||||
<TextEditor
|
||||
rootInstanceSelector={["current", "block"]}
|
||||
instances={instances}
|
||||
props={new Map()}
|
||||
contentEditable={<ContentEditable />}
|
||||
onChange={onChange}
|
||||
onSelectInstance={() => {}}
|
||||
/>
|
||||
);
|
||||
});
|
||||
|
||||
const editable = container.querySelector<HTMLElement>(
|
||||
"[data-lexical-editor]"
|
||||
);
|
||||
});
|
||||
|
||||
const editable = container.querySelector<HTMLElement>(
|
||||
"[data-lexical-editor]"
|
||||
);
|
||||
await act(async () => {
|
||||
editable?.dispatchEvent(
|
||||
new KeyboardEvent("keydown", { key: "/", bubbles: true })
|
||||
await act(async () => {
|
||||
if (target === "window") {
|
||||
await userEvent.type(editable!, " edited");
|
||||
} else {
|
||||
editable?.dispatchEvent(
|
||||
new KeyboardEvent("keydown", { key: "/", bubbles: true })
|
||||
);
|
||||
}
|
||||
});
|
||||
expect(editable?.textContent).toBe(
|
||||
target === "window" ? "Before edited" : "Before/"
|
||||
);
|
||||
});
|
||||
expect(editable?.textContent).toBe("Before/");
|
||||
expect($textEditorContextMenu.get()).toMatchObject({
|
||||
replaceAnchor: false,
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
editable?.dispatchEvent(new FocusEvent("blur"));
|
||||
});
|
||||
await act(async () => {
|
||||
(target === "window" ? window : editable)?.dispatchEvent(
|
||||
new FocusEvent("blur")
|
||||
);
|
||||
});
|
||||
|
||||
expect(onChange).toHaveBeenCalledTimes(1);
|
||||
expect(onChange.mock.calls[0]?.[0]).toContainEqual(
|
||||
expect.objectContaining({
|
||||
id: "current",
|
||||
children: [{ type: "text", value: "Before" }],
|
||||
})
|
||||
);
|
||||
});
|
||||
expect(onChange).toHaveBeenCalledTimes(1);
|
||||
expect(onChange.mock.calls[0]?.[0]).toContainEqual(
|
||||
expect.objectContaining({
|
||||
id: "current",
|
||||
children: [
|
||||
{
|
||||
type: "text",
|
||||
value: target === "window" ? "Before edited" : "Before",
|
||||
},
|
||||
],
|
||||
})
|
||||
);
|
||||
}
|
||||
);
|
||||
|
||||
test("does not save an editor after its instance was replaced", async () => {
|
||||
const instances = createContentBlockInstances();
|
||||
|
||||
@@ -217,7 +217,10 @@ const isChrome = () =>
|
||||
const OnChangeOnBlurPlugin = ({
|
||||
onChange,
|
||||
}: {
|
||||
onChange: (editorState: EditorState, reason: "blur" | "unmount") => void;
|
||||
onChange: (
|
||||
editorState: EditorState,
|
||||
reason: "blur" | "window-blur" | "unmount"
|
||||
) => void;
|
||||
}) => {
|
||||
const [editor] = useLexicalComposerContext();
|
||||
const handleChange = useEffectEvent(onChange);
|
||||
@@ -255,6 +258,21 @@ const OnChangeOnBlurPlugin = ({
|
||||
[editor]
|
||||
);
|
||||
|
||||
useEffect(() => {
|
||||
// Leaving the canvas iframe does not blur its active editable element.
|
||||
// Save without ending editing so toolbar actions can keep the selection.
|
||||
const handleWindowBlur = () => {
|
||||
if ($textEditorContextMenu.get() !== undefined) {
|
||||
return;
|
||||
}
|
||||
editor.read(() => {
|
||||
handleChange(editor.getEditorState(), "window-blur");
|
||||
});
|
||||
};
|
||||
window.addEventListener("blur", handleWindowBlur);
|
||||
return () => window.removeEventListener("blur", handleWindowBlur);
|
||||
}, [editor]);
|
||||
|
||||
useEffect(() => {
|
||||
const handleBlur = () => {
|
||||
// force read to get the latest state
|
||||
@@ -1626,7 +1644,10 @@ export const TextEditor = ({
|
||||
const [newLinkKeyToInstanceId] = useState(() => new Map());
|
||||
|
||||
const handleChange = useEffectEvent(
|
||||
(editorState: EditorState, reason: "blur" | "unmount" | "next") => {
|
||||
(
|
||||
editorState: EditorState,
|
||||
reason: "blur" | "window-blur" | "unmount" | "next"
|
||||
) => {
|
||||
const currentInstances = $instances.get();
|
||||
const treeRootInstance = currentInstances.get(rootInstanceSelector[0]);
|
||||
// Replacing a content block child unmounts its editor after the instance
|
||||
|
||||
@@ -19,11 +19,8 @@ import { requireBuilderReload } from "./sync/reload-required";
|
||||
import { createAssetContentSession } from "@webstudio-is/content-engine/asset-content-session";
|
||||
import { createHttpAssetContentRepository } from "~/builder/shared/assets/mdx-content-repository";
|
||||
import { $authToken } from "./nano-states";
|
||||
import type { Asset } from "@webstudio-is/sdk";
|
||||
import { createTransactionFromBuilderPatchPayload } from "./sync/builder-patch";
|
||||
import { getWebstudioData } from "./instance-utils/data";
|
||||
import { invalidateAssets } from "./resources";
|
||||
import { onNextTransactionComplete } from "./sync/project-queue";
|
||||
import { asset, type Asset } from "@webstudio-is/sdk";
|
||||
import { commitAssetContentUpdate } from "~/builder/shared/assets/update-asset-content";
|
||||
import { disposeExternalContentProject } from "./external-content-roots";
|
||||
|
||||
const apiWindowNamespace = "__webstudio__$__builderApi";
|
||||
@@ -339,22 +336,9 @@ export const initBuilderApi = () => {
|
||||
) {
|
||||
return;
|
||||
}
|
||||
createTransactionFromBuilderPatchPayload({
|
||||
data: getWebstudioData(),
|
||||
payload: [
|
||||
{
|
||||
namespace: "assets",
|
||||
patches: [
|
||||
{
|
||||
op: "replace",
|
||||
path: [current.id],
|
||||
value: { ...current, ...state.asset },
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
});
|
||||
onNextTransactionComplete(invalidateAssets);
|
||||
commitAssetContentUpdate(
|
||||
asset.parse({ ...current, ...state.asset })
|
||||
);
|
||||
});
|
||||
activeContentSession = { projectId, session, active: true };
|
||||
contentSessions.set(projectId, activeContentSession);
|
||||
|
||||
@@ -1256,9 +1256,22 @@ test.each([
|
||||
expect(writes.at(-1)).toBe(
|
||||
'# Existing\n\n<Card title="Edited">Edited text</Card>\n'
|
||||
);
|
||||
expect(
|
||||
getExternalContentRoots().values().next().value?.insertedTemplates
|
||||
).toBeUndefined();
|
||||
executeRuntimeMutation({
|
||||
id: "instances.setTextContent",
|
||||
input: {
|
||||
operation: "set",
|
||||
instanceId: inserted.value,
|
||||
mode: "text",
|
||||
text: "Edited again",
|
||||
},
|
||||
});
|
||||
await flushExternalContentAsset({
|
||||
projectId: "project",
|
||||
assetId: sourceAsset.id,
|
||||
});
|
||||
expect(writes.at(-1)).toBe(
|
||||
'# Existing\n\n<Card title="Edited">Edited again</Card>\n'
|
||||
);
|
||||
|
||||
release();
|
||||
releases.splice(releases.indexOf(release), 1);
|
||||
|
||||
@@ -566,7 +566,6 @@ test("Empty MDX content supports slash menu keyboard and mouse insertion", async
|
||||
const paragraphWrite = waitForAssetWrite(page, (source) =>
|
||||
source.includes("Focused paragraph")
|
||||
);
|
||||
const paragraphMetadataWrite = waitForChangeToBeSaved({ page });
|
||||
await page.keyboard.press("ControlOrMeta+A");
|
||||
await page.keyboard.type("Focused paragraph");
|
||||
await page.keyboard.press("Enter");
|
||||
@@ -575,7 +574,6 @@ test("Empty MDX content supports slash menu keyboard and mouse insertion", async
|
||||
);
|
||||
await nextEmptyParagraphEditor.waitFor({ state: "visible" });
|
||||
await paragraphWrite;
|
||||
await paragraphMetadataWrite;
|
||||
const emptyParagraphId =
|
||||
await nextEmptyParagraphEditor.getAttribute("data-ws-id");
|
||||
if (emptyParagraphId === null) {
|
||||
|
||||
@@ -79,6 +79,34 @@ const createCodeTextFragment = (theme = "github-light"): WebstudioFragment => ({
|
||||
});
|
||||
|
||||
describe("MDX authored content", () => {
|
||||
test("adopts a live fragment without unused resolved assets", async () => {
|
||||
const root = materializeMdxAuthoredContent({
|
||||
identity,
|
||||
document: await parseMdxDocument({ source: "Before" }),
|
||||
templateMaterialization: emptyTemplates,
|
||||
});
|
||||
const live = structuredClone(root.fragment);
|
||||
root.fragment.assets.push({
|
||||
id: "unused",
|
||||
projectId: "project",
|
||||
name: "author.md",
|
||||
type: "file",
|
||||
format: "md",
|
||||
size: 7,
|
||||
createdAt: "2026-01-01T00:00:00.000Z",
|
||||
description: null,
|
||||
meta: {},
|
||||
});
|
||||
const adopted = adoptMdxAuthoredContentFragment({ root, fragment: live });
|
||||
const edited = structuredClone(live);
|
||||
edited.instances[0].children = [{ type: "text", value: "After" }];
|
||||
expect(
|
||||
serializeMdxDocument(
|
||||
reconcileMdxAuthoredContent({ root: adopted, fragment: edited })
|
||||
)
|
||||
).toBe("After\n");
|
||||
});
|
||||
|
||||
test("does not materialize registered JSX without a resolved template", async () => {
|
||||
const source =
|
||||
"<PromotionCard><Heading>Launch offer</Heading></PromotionCard>\n";
|
||||
@@ -168,6 +196,10 @@ describe("MDX authored content", () => {
|
||||
jsxPropContext: htmlJsxPropContext,
|
||||
propNameMappings: [],
|
||||
ignoredJsxPropNames: [],
|
||||
htmlTags: [
|
||||
{ instanceId: "card", tag: "section" },
|
||||
{ instanceId: "replaced-child", tag: "p" },
|
||||
],
|
||||
},
|
||||
],
|
||||
diagnostics: [],
|
||||
@@ -178,8 +210,12 @@ describe("MDX authored content", () => {
|
||||
expect(root.fragment.instances[0]?.children).toEqual([
|
||||
{ type: "text", value: "Template default" },
|
||||
]);
|
||||
const adopted = adoptMdxAuthoredContentFragment({
|
||||
root,
|
||||
fragment: root.fragment,
|
||||
});
|
||||
await expect(
|
||||
serializeMdxAuthoredContent({ root, fragment: root.fragment })
|
||||
serializeMdxAuthoredContent({ root: adopted, fragment: adopted.fragment })
|
||||
).resolves.toBe("<Card />\n");
|
||||
});
|
||||
|
||||
|
||||
@@ -1245,13 +1245,16 @@ export const materializeMdxAuthoredContent = ({
|
||||
preservedJsxPropNames: template.preservedJsxPropNames,
|
||||
ignoredJsxPropNames: template.ignoredJsxPropNames,
|
||||
expandedInstanceIds: resolvedFragment.instances.map(({ id }) => id),
|
||||
htmlTags:
|
||||
htmlTags: (
|
||||
template.htmlTags ??
|
||||
resolvedFragment.instances.flatMap((instance) =>
|
||||
instance.tag === undefined
|
||||
? []
|
||||
: [{ instanceId: instance.id, tag: instance.tag }]
|
||||
),
|
||||
)
|
||||
).filter(({ instanceId }) =>
|
||||
resolvedFragment.instances.some(({ id }) => id === instanceId)
|
||||
),
|
||||
overlaidDescendants,
|
||||
assetProps,
|
||||
namespaceKeys: unsupportedNamespaces.flatMap((namespace) =>
|
||||
@@ -1669,7 +1672,7 @@ export const adoptMdxAuthoredContentFragment = ({
|
||||
const propIds = new Map<string, string>();
|
||||
const inheritedPropIds = new Set<string>();
|
||||
for (const node of root.provenance.nodes) {
|
||||
if (node.type !== "template" || node.overridesTemplateChildren) {
|
||||
if (node.type !== "template") {
|
||||
continue;
|
||||
}
|
||||
const authoredNames = new Set(
|
||||
@@ -1682,8 +1685,9 @@ export const adoptMdxAuthoredContentFragment = ({
|
||||
for (const prop of root.fragment.props) {
|
||||
if (
|
||||
node.expandedInstanceIds.includes(prop.instanceId) &&
|
||||
(prop.instanceId !== node.instanceId ||
|
||||
authoredNames.has(prop.name) === false)
|
||||
(prop.instanceId === node.instanceId
|
||||
? authoredNames.has(prop.name) === false
|
||||
: node.overridesTemplateChildren === false)
|
||||
) {
|
||||
inheritedPropIds.add(prop.id);
|
||||
}
|
||||
@@ -1716,6 +1720,31 @@ export const adoptMdxAuthoredContentFragment = ({
|
||||
|
||||
const namespaceKeys = new Map<string, string>();
|
||||
for (const namespace of unsupportedNamespaces) {
|
||||
if (namespace === "assets") {
|
||||
// Resolved frontmatter and template defaults can load assets that the
|
||||
// rendered body never uses. Assets retain their project IDs, unlike
|
||||
// cloned instances and styles.
|
||||
const sourceAssets = new Map(
|
||||
root.fragment.assets.map((asset) => [asset.id, asset])
|
||||
);
|
||||
const targetAssets = new Map(
|
||||
fragment.assets.map((asset) => [asset.id, asset])
|
||||
);
|
||||
if (
|
||||
fragment.assets.some(
|
||||
(asset) => !equal(sourceAssets.get(asset.id), asset)
|
||||
) ||
|
||||
fragment.props.some(
|
||||
(prop) => prop.type === "asset" && !targetAssets.has(prop.value)
|
||||
)
|
||||
) {
|
||||
throw new Error("Live MDX fragment assets do not match its document");
|
||||
}
|
||||
for (const asset of root.fragment.assets) {
|
||||
namespaceKeys.set(`assets:${asset.id}`, asset.id);
|
||||
}
|
||||
continue;
|
||||
}
|
||||
const sourceRecords = root.fragment[namespace];
|
||||
const targetRecords = fragment[namespace];
|
||||
if (sourceRecords.length !== targetRecords.length) {
|
||||
|
||||
Reference in new issue
Block a user