Compare commits
15 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 9d1b033fe8 | |||
| fcbe840c74 | |||
| 5336f06d10 | |||
| 4bd579f7f6 | |||
| 7bf1c91a95 | |||
| 6c82c54470 | |||
| 382e5196da | |||
| 76e0c08cec | |||
| 8978d69f3e | |||
| c192f2a2e1 | |||
| d78b985062 | |||
| 2ce672709a | |||
| a4fc6c7f64 | |||
| c252068672 | |||
| cb9c5dda59 |
@@ -18,12 +18,48 @@ env:
|
||||
IMAGE: ghcr.io/vvzvlad/gitmost
|
||||
|
||||
jobs:
|
||||
# Run the reusable test suite first so a failing test blocks the image build.
|
||||
# Run the reusable test suite. Together with the e2e jobs below it gates the
|
||||
# publish job (the image push), not the build itself — build runs in parallel.
|
||||
test:
|
||||
uses: ./.github/workflows/test.yml
|
||||
|
||||
# Runs in parallel with the test/e2e jobs and only warms the buildx cache
|
||||
# (GHA cache, scope develop-amd64). No push happens here — the publish job
|
||||
# below is the only one that pushes the image.
|
||||
build:
|
||||
needs: test
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 30
|
||||
steps:
|
||||
- name: Checkout
|
||||
uses: actions/checkout@v4
|
||||
with:
|
||||
fetch-depth: 0
|
||||
|
||||
- name: Set up Docker Buildx
|
||||
uses: docker/setup-buildx-action@v3
|
||||
|
||||
- name: Resolve version
|
||||
id: version
|
||||
run: echo "value=$(git describe --tags --always)" >> "$GITHUB_OUTPUT"
|
||||
|
||||
- name: Build develop image (warm cache, no push)
|
||||
uses: docker/build-push-action@v6
|
||||
with:
|
||||
context: .
|
||||
platforms: linux/amd64
|
||||
build-args: |
|
||||
APP_VERSION=${{ steps.version.outputs.value }}
|
||||
AI_AGENT_ROLES_CATALOG_URL=https://raw.githubusercontent.com/vvzvlad/gitmost/develop/agent-roles-catalog
|
||||
push: false
|
||||
cache-from: type=gha,scope=develop-amd64
|
||||
cache-to: type=gha,scope=develop-amd64,mode=max,ignore-error=true
|
||||
|
||||
# The gate: rebuilds from the cache the build job just wrote (near-instant on
|
||||
# a cache hit; worst case — cache eviction — a full rebuild, which matches the
|
||||
# old sequential timing) and pushes :develop only when unit tests AND both
|
||||
# e2e suites AND the build are green.
|
||||
publish:
|
||||
needs: [test, e2e-server, e2e-mcp, build]
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 30
|
||||
steps:
|
||||
@@ -57,13 +93,10 @@ jobs:
|
||||
push: true
|
||||
tags: ${{ env.IMAGE }}:develop
|
||||
cache-from: type=gha,scope=develop-amd64
|
||||
cache-to: type=gha,scope=develop-amd64,mode=max,ignore-error=true
|
||||
|
||||
# e2e jobs run on every develop push but DO NOT gate the build/publish above:
|
||||
# `build` stays `needs: test` only, so the :develop image still ships even if
|
||||
# e2e fails. A failing e2e job turns the run red and triggers GitHub's email
|
||||
# to the pusher — that red run + email is the intended notification, not a
|
||||
# deploy block.
|
||||
# e2e jobs gate the publish (image push), not the build: the :develop image
|
||||
# is pushed only when unit tests AND both e2e suites pass (publish.needs
|
||||
# lists them all).
|
||||
e2e-server:
|
||||
runs-on: ubuntu-latest
|
||||
# Hard cap: the full-AppModule e2e leaks open handles and hung jest to the 6h max.
|
||||
@@ -124,9 +157,7 @@ jobs:
|
||||
- name: Run server e2e
|
||||
run: pnpm --filter ./apps/server test:e2e
|
||||
|
||||
# Same rationale as e2e-server: this job is intentionally NOT in
|
||||
# `build.needs`. Deploy of the :develop image must not be blocked by e2e;
|
||||
# a red run plus GitHub's email to the pusher is the notification mechanism.
|
||||
# Gates the publish too — see the comment above e2e-server.
|
||||
e2e-mcp:
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 20
|
||||
|
||||
+16
-2
@@ -5,6 +5,13 @@ RUN npm install -g pnpm@10.4.0
|
||||
|
||||
FROM base AS builder
|
||||
|
||||
# re2 (packages/mcp) always compiles from source under pnpm (the prebuilt-binary
|
||||
# download cannot identify the GitHub repo), so node-gyp needs python3/make/g++.
|
||||
# This stage is discarded, so the toolchain can stay installed.
|
||||
RUN apt-get update \
|
||||
&& apt-get install -y --no-install-recommends python3 make g++ \
|
||||
&& rm -rf /var/lib/apt/lists/*
|
||||
|
||||
WORKDIR /app
|
||||
|
||||
COPY . .
|
||||
@@ -57,9 +64,16 @@ COPY --from=builder /app/patches /app/patches
|
||||
|
||||
RUN chown -R node:node /app
|
||||
|
||||
USER node
|
||||
# Toolchain is needed transiently to compile re2 during the prod install; install
|
||||
# and purge it in one layer to keep the final image slim. The install itself runs
|
||||
# as the node user via su to keep node_modules ownership without a costly chown layer.
|
||||
RUN apt-get update \
|
||||
&& apt-get install -y --no-install-recommends python3 make g++ \
|
||||
&& su node -c "pnpm install --frozen-lockfile --prod" \
|
||||
&& apt-get purge -y --auto-remove python3 make g++ \
|
||||
&& rm -rf /var/lib/apt/lists/*
|
||||
|
||||
RUN pnpm install --frozen-lockfile --prod
|
||||
USER node
|
||||
|
||||
RUN mkdir -p /app/data/storage
|
||||
|
||||
|
||||
@@ -67,14 +67,20 @@ export default function GlobalAppShell({
|
||||
);
|
||||
|
||||
useEffect(() => {
|
||||
//https://codesandbox.io/p/sandbox/kz9de
|
||||
// Attach the global mousemove/mouseup only WHILE resizing (started on the
|
||||
// handle's mousedown via startResizing → isResizing=true) and detach on
|
||||
// mouseup (stopResizing → isResizing=false). Previously these listeners were
|
||||
// attached for the whole app lifetime, so every mouse move over the app ran
|
||||
// the resize handler.
|
||||
// https://codesandbox.io/p/sandbox/kz9de
|
||||
if (!isResizing) return;
|
||||
window.addEventListener("mousemove", resize);
|
||||
window.addEventListener("mouseup", stopResizing);
|
||||
return () => {
|
||||
window.removeEventListener("mousemove", resize);
|
||||
window.removeEventListener("mouseup", stopResizing);
|
||||
};
|
||||
}, [resize, stopResizing]);
|
||||
}, [isResizing, resize, stopResizing]);
|
||||
|
||||
const location = useLocation();
|
||||
const isSettingsRoute = location.pathname.startsWith("/settings");
|
||||
|
||||
@@ -5,7 +5,7 @@ import {
|
||||
Button,
|
||||
useMantineColorScheme,
|
||||
} from "@mantine/core";
|
||||
import { useClickOutside, useDisclosure, useWindowEvent } from "@mantine/hooks";
|
||||
import { useClickOutside, useDisclosure } from "@mantine/hooks";
|
||||
import { Suspense } from "react";
|
||||
import { useTranslation } from "react-i18next";
|
||||
|
||||
@@ -57,14 +57,22 @@ function EmojiPicker({
|
||||
[dropdown, target],
|
||||
);
|
||||
|
||||
// We need this because the default Mantine popover closeOnEscape does not work
|
||||
useWindowEvent("keydown", (event) => {
|
||||
if (opened && event.key === "Escape") {
|
||||
event.stopPropagation();
|
||||
event.preventDefault();
|
||||
handlers.close();
|
||||
}
|
||||
});
|
||||
// We need this because the default Mantine popover closeOnEscape does not work.
|
||||
// Attach the global keydown ONLY while the picker is open (every tree row
|
||||
// renders an EmojiPicker, so an always-on window listener meant ~20-30 idle
|
||||
// keydown handlers firing on each keystroke).
|
||||
useEffect(() => {
|
||||
if (!opened) return;
|
||||
const handleKeydown = (event: KeyboardEvent) => {
|
||||
if (event.key === "Escape") {
|
||||
event.stopPropagation();
|
||||
event.preventDefault();
|
||||
handlers.close();
|
||||
}
|
||||
};
|
||||
window.addEventListener("keydown", handleKeydown);
|
||||
return () => window.removeEventListener("keydown", handleKeydown);
|
||||
}, [opened, handlers]);
|
||||
|
||||
// emoji-mart's built-in autoFocus calls .focus() without preventScroll, which
|
||||
// makes the browser scroll every scrollable ancestor of the search input to
|
||||
|
||||
@@ -36,7 +36,7 @@ import {
|
||||
desktopSidebarAtom,
|
||||
mobileSidebarAtom,
|
||||
} from "@/components/layouts/global/hooks/atoms/sidebar-atom.ts";
|
||||
import { usePageQuery } from "@/features/page/queries/page-query.ts";
|
||||
import { usePageMetaQuery } from "@/features/page/queries/page-query.ts";
|
||||
import { extractPageSlugId } from "@/lib";
|
||||
import {
|
||||
AI_CHATS_RQ_KEY,
|
||||
@@ -219,7 +219,9 @@ export default function AiChatWindow() {
|
||||
// left partly off-screen).
|
||||
const [geom, setGeom] = useAtom(aiChatWindowGeomAtom);
|
||||
|
||||
const { data: chats } = useAiChatsQuery();
|
||||
// Gated on windowOpen: the chat list is only needed once the window is open,
|
||||
// so a closed window issues no chat-list request/refetch on navigation.
|
||||
const { data: chats } = useAiChatsQuery(windowOpen);
|
||||
// Roles for the new-chat picker (any member may list them). Only fetched while
|
||||
// the window is open.
|
||||
const { data: roles } = useAiRolesQuery(windowOpen);
|
||||
@@ -231,8 +233,10 @@ export default function AiChatWindow() {
|
||||
[roles],
|
||||
);
|
||||
|
||||
// Gated on windowOpen too: no message history is fetched while the window is
|
||||
// closed (it is loaded when the window opens with an active chat).
|
||||
const { data: messageRows, isLoading: messagesLoading } =
|
||||
useAiChatMessagesQuery(activeChatId ?? undefined);
|
||||
useAiChatMessagesQuery(activeChatId ?? undefined, windowOpen);
|
||||
|
||||
// The page the user is currently viewing. AiChatWindow lives in a pathless
|
||||
// parent layout route, so useParams() can't see :pageSlug. Match the full
|
||||
@@ -244,7 +248,7 @@ export default function AiChatWindow() {
|
||||
// reads/writes via its CASL-enforced page tools using the id.
|
||||
const pageRouteMatch = useMatch("/s/:spaceSlug/p/:pageSlug");
|
||||
const pageSlug = pageRouteMatch?.params?.pageSlug;
|
||||
const { data: openPageData } = usePageQuery({
|
||||
const { data: openPageData } = usePageMetaQuery({
|
||||
pageId: extractPageSlugId(pageSlug),
|
||||
});
|
||||
const openPage = openPageData
|
||||
|
||||
@@ -52,8 +52,12 @@ export const AI_CHAT_MESSAGES_RQ_KEY = (chatId: string) => [
|
||||
chatId,
|
||||
];
|
||||
|
||||
/** Paginated list of the current user's chats (auto-loads further pages). */
|
||||
export function useAiChatsQuery() {
|
||||
/**
|
||||
* Paginated list of the current user's chats (auto-loads further pages).
|
||||
* `enabled` (default true) lets the AI chat window skip fetching while it is
|
||||
* closed — the list is only needed once the window is open.
|
||||
*/
|
||||
export function useAiChatsQuery(enabled: boolean = true) {
|
||||
const query = useInfiniteQuery({
|
||||
queryKey: AI_CHATS_RQ_KEY,
|
||||
queryFn: ({ pageParam }) =>
|
||||
@@ -61,6 +65,7 @@ export function useAiChatsQuery() {
|
||||
initialPageParam: undefined as string | undefined,
|
||||
getNextPageParam: (lastPage) =>
|
||||
lastPage.meta.hasNextPage ? (lastPage.meta.nextCursor ?? undefined) : undefined,
|
||||
enabled,
|
||||
});
|
||||
|
||||
const data = useMemo<IPagination<IAiChat> | undefined>(() => {
|
||||
@@ -83,7 +88,10 @@ export function useAiChatsQuery() {
|
||||
* Load all persisted messages of a chat (oldest first), flattening the
|
||||
* paginated server response. Used to seed `useChat` initial messages.
|
||||
*/
|
||||
export function useAiChatMessagesQuery(chatId: string | undefined) {
|
||||
export function useAiChatMessagesQuery(
|
||||
chatId: string | undefined,
|
||||
enabled: boolean = true,
|
||||
) {
|
||||
const query = useInfiniteQuery({
|
||||
queryKey: AI_CHAT_MESSAGES_RQ_KEY(chatId ?? ""),
|
||||
queryFn: ({ pageParam }) =>
|
||||
@@ -91,7 +99,7 @@ export function useAiChatMessagesQuery(chatId: string | undefined) {
|
||||
initialPageParam: undefined as string | undefined,
|
||||
getNextPageParam: (lastPage) =>
|
||||
lastPage.meta.hasNextPage ? (lastPage.meta.nextCursor ?? undefined) : undefined,
|
||||
enabled: !!chatId,
|
||||
enabled: !!chatId && enabled,
|
||||
});
|
||||
|
||||
// useInfiniteQuery only fetches the first page on its own. The hook's contract
|
||||
|
||||
@@ -0,0 +1,251 @@
|
||||
import { describe, it, expect, vi } from "vitest";
|
||||
import { render, screen } from "@testing-library/react";
|
||||
import { MantineProvider } from "@mantine/core";
|
||||
import { MemoryRouter } from "react-router-dom";
|
||||
|
||||
// matchMedia (read by MantineProvider) is stubbed globally in vitest.setup.ts.
|
||||
|
||||
// The fallback path renders the full TipTap editor; stub it so we can assert the
|
||||
// safety valve fired without pulling in the editor stack.
|
||||
vi.mock("@/features/comment/components/comment-editor", () => ({
|
||||
default: () => <div data-testid="comment-editor-fallback" />,
|
||||
}));
|
||||
|
||||
// Mention rendering hits react-query; stub the page/share queries so the mention
|
||||
// case renders in isolation.
|
||||
vi.mock("@/features/page/queries/page-query.ts", () => ({
|
||||
usePageQuery: () => ({ data: undefined, isLoading: false, isError: false }),
|
||||
usePageMetaQuery: () => ({ data: undefined, isLoading: false, isError: false }),
|
||||
}));
|
||||
vi.mock("@/features/share/queries/share-query.ts", () => ({
|
||||
useSharePageQuery: () => ({ data: undefined }),
|
||||
}));
|
||||
|
||||
import { CommentContentView } from "./comment-content-view";
|
||||
|
||||
function renderView(content: string | object) {
|
||||
return render(
|
||||
<MantineProvider>
|
||||
<MemoryRouter>
|
||||
<CommentContentView content={content} />
|
||||
</MemoryRouter>
|
||||
</MantineProvider>,
|
||||
);
|
||||
}
|
||||
|
||||
const doc = (content: any[]) => JSON.stringify({ type: "doc", content });
|
||||
const para = (content: any[]) => ({ type: "paragraph", content });
|
||||
const text = (t: string, marks?: any[]) => ({ type: "text", text: t, marks });
|
||||
|
||||
describe("CommentContentView", () => {
|
||||
it("renders paragraphs as <p> with text", () => {
|
||||
const { container } = renderView(doc([para([text("Hello world")])]));
|
||||
expect(screen.getByText("Hello world")).toBeDefined();
|
||||
expect(container.querySelector("p")).not.toBeNull();
|
||||
});
|
||||
|
||||
it("reproduces the read-only CommentEditor DOM nesting for CSS parity", () => {
|
||||
const { container } = renderView(doc([para([text("x")])]));
|
||||
// outer .commentEditor > .ProseMirror (module) > .ProseMirror (global) > p
|
||||
const globalPm = container.querySelector("div.ProseMirror > p");
|
||||
expect(globalPm).not.toBeNull();
|
||||
});
|
||||
|
||||
it("renders the bold mark as <strong>", () => {
|
||||
const { container } = renderView(
|
||||
doc([para([text("bold", [{ type: "bold" }])])]),
|
||||
);
|
||||
const el = container.querySelector("strong");
|
||||
expect(el?.textContent).toBe("bold");
|
||||
});
|
||||
|
||||
it("renders the italic mark as <em>", () => {
|
||||
const { container } = renderView(
|
||||
doc([para([text("it", [{ type: "italic" }])])]),
|
||||
);
|
||||
expect(container.querySelector("em")?.textContent).toBe("it");
|
||||
});
|
||||
|
||||
it("renders the strike mark as <s>", () => {
|
||||
const { container } = renderView(
|
||||
doc([para([text("st", [{ type: "strike" }])])]),
|
||||
);
|
||||
expect(container.querySelector("s")?.textContent).toBe("st");
|
||||
});
|
||||
|
||||
it("renders the underline mark as <u> (not the editor fallback)", () => {
|
||||
const { container } = renderView(
|
||||
doc([para([text("un", [{ type: "underline" }])])]),
|
||||
);
|
||||
expect(container.querySelector("u")?.textContent).toBe("un");
|
||||
// Underline is a supported mark, so no degrade to the editor fallback.
|
||||
expect(screen.queryByTestId("comment-editor-fallback")).toBeNull();
|
||||
});
|
||||
|
||||
it("renders the code mark as <code>", () => {
|
||||
const { container } = renderView(
|
||||
doc([para([text("co", [{ type: "code" }])])]),
|
||||
);
|
||||
expect(container.querySelector("code")?.textContent).toBe("co");
|
||||
});
|
||||
|
||||
it("renders the link mark as an anchor with safe rel/target", () => {
|
||||
const { container } = renderView(
|
||||
doc([
|
||||
para([
|
||||
text("click", [
|
||||
{ type: "link", attrs: { href: "https://example.com" } },
|
||||
]),
|
||||
]),
|
||||
]),
|
||||
);
|
||||
const a = container.querySelector("a");
|
||||
expect(a?.getAttribute("href")).toBe("https://example.com");
|
||||
expect(a?.getAttribute("target")).toBe("_blank");
|
||||
expect(a?.getAttribute("rel")).toBe("noopener noreferrer nofollow");
|
||||
expect(a?.textContent).toBe("click");
|
||||
});
|
||||
|
||||
it("neutralizes a javascript: link href (stored XSS) while keeping the text", () => {
|
||||
const { container } = renderView(
|
||||
doc([
|
||||
para([
|
||||
text("click", [
|
||||
{ type: "link", attrs: { href: "javascript:alert(1)" } },
|
||||
]),
|
||||
]),
|
||||
]),
|
||||
);
|
||||
const a = container.querySelector("a");
|
||||
expect(a).not.toBeNull();
|
||||
// No navigable javascript: href — attribute is absent (or empty).
|
||||
expect(a?.getAttribute("href")).toBeFalsy();
|
||||
// The link text is still rendered.
|
||||
expect(a?.textContent).toBe("click");
|
||||
});
|
||||
|
||||
it("neutralizes a control-char-obfuscated javascript: href", () => {
|
||||
const { container } = renderView(
|
||||
doc([
|
||||
para([
|
||||
text("x", [
|
||||
{ type: "link", attrs: { href: "java\tscript:alert(1)" } },
|
||||
]),
|
||||
]),
|
||||
]),
|
||||
);
|
||||
expect(container.querySelector("a")?.getAttribute("href")).toBeFalsy();
|
||||
});
|
||||
|
||||
it("neutralizes a data: link href", () => {
|
||||
const { container } = renderView(
|
||||
doc([
|
||||
para([
|
||||
text("x", [
|
||||
{
|
||||
type: "link",
|
||||
attrs: { href: "data:text/html,<script>alert(1)</script>" },
|
||||
},
|
||||
]),
|
||||
]),
|
||||
]),
|
||||
);
|
||||
expect(container.querySelector("a")?.getAttribute("href")).toBeFalsy();
|
||||
});
|
||||
|
||||
it("preserves a mailto: link href (allowlisted scheme)", () => {
|
||||
const { container } = renderView(
|
||||
doc([
|
||||
para([
|
||||
text("mail", [
|
||||
{ type: "link", attrs: { href: "mailto:a@b.com" } },
|
||||
]),
|
||||
]),
|
||||
]),
|
||||
);
|
||||
expect(container.querySelector("a")?.getAttribute("href")).toBe(
|
||||
"mailto:a@b.com",
|
||||
);
|
||||
});
|
||||
|
||||
it("preserves a relative link href (no scheme, not a script vector)", () => {
|
||||
const { container } = renderView(
|
||||
doc([
|
||||
para([
|
||||
text("rel", [{ type: "link", attrs: { href: "/some/path" } }]),
|
||||
]),
|
||||
]),
|
||||
);
|
||||
expect(container.querySelector("a")?.getAttribute("href")).toBe(
|
||||
"/some/path",
|
||||
);
|
||||
});
|
||||
|
||||
it("nests multiple marks on one text node", () => {
|
||||
const { container } = renderView(
|
||||
doc([para([text("x", [{ type: "bold" }, { type: "italic" }])])]),
|
||||
);
|
||||
// bold wraps italic (or vice versa) — both elements exist around the text.
|
||||
expect(container.querySelector("strong")).not.toBeNull();
|
||||
expect(container.querySelector("em")).not.toBeNull();
|
||||
expect(screen.getByText("x")).toBeDefined();
|
||||
});
|
||||
|
||||
it("renders hardBreak as <br/>", () => {
|
||||
const { container } = renderView(
|
||||
doc([para([text("a"), { type: "hardBreak" }, text("b")])]),
|
||||
);
|
||||
expect(container.querySelector("br")).not.toBeNull();
|
||||
});
|
||||
|
||||
it("renders a user mention as a styled span", () => {
|
||||
const { container } = renderView(
|
||||
doc([
|
||||
para([
|
||||
{
|
||||
type: "mention",
|
||||
attrs: { label: "Alice", entityType: "user", entityId: "u1" },
|
||||
},
|
||||
]),
|
||||
]),
|
||||
);
|
||||
expect(screen.getByText("@Alice")).toBeDefined();
|
||||
// No fallback to the editor.
|
||||
expect(screen.queryByTestId("comment-editor-fallback")).toBeNull();
|
||||
});
|
||||
|
||||
it("renders a page mention as a link", () => {
|
||||
const { container } = renderView(
|
||||
doc([
|
||||
para([
|
||||
{
|
||||
type: "mention",
|
||||
attrs: {
|
||||
label: "Some Page",
|
||||
entityType: "page",
|
||||
slugId: "pg1",
|
||||
},
|
||||
},
|
||||
]),
|
||||
]),
|
||||
);
|
||||
expect(container.querySelector("a")).not.toBeNull();
|
||||
expect(screen.getByText("Some Page")).toBeDefined();
|
||||
});
|
||||
|
||||
it("renders a legacy plain-text (non-JSON) string as plain text", () => {
|
||||
renderView("just a legacy string");
|
||||
expect(screen.getByText("just a legacy string")).toBeDefined();
|
||||
expect(screen.queryByTestId("comment-editor-fallback")).toBeNull();
|
||||
});
|
||||
|
||||
it("falls back to CommentEditor for an unknown node type", () => {
|
||||
renderView(doc([{ type: "codeBlock", content: [text("x")] }]));
|
||||
expect(screen.getByTestId("comment-editor-fallback")).toBeDefined();
|
||||
});
|
||||
|
||||
it("falls back to CommentEditor for malformed JSON", () => {
|
||||
renderView('{"type":"doc","content":[');
|
||||
expect(screen.getByTestId("comment-editor-fallback")).toBeDefined();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,199 @@
|
||||
import React from "react";
|
||||
import classes from "./comment.module.css";
|
||||
import { MentionContent } from "@/features/editor/components/mention/mention-view";
|
||||
import CommentEditor from "@/features/comment/components/comment-editor";
|
||||
|
||||
// Static, editor-free renderer of a comment body (ProseMirror JSON). It walks the
|
||||
// document and emits plain DOM, avoiding the cost of a full TipTap/ProseMirror
|
||||
// instance per comment (the panel used to spin up 400+ editors on mount).
|
||||
//
|
||||
// The supported node/mark set MUST mirror what CommentEditor enables
|
||||
// (StarterKit + Mention + LinkExtension). Anything outside that set makes the
|
||||
// whole comment degrade to the read-only CommentEditor via the fallback below,
|
||||
// so we never show a half-rendered comment.
|
||||
|
||||
// Sentinel thrown when we hit a node/mark we don't know how to render statically.
|
||||
// Caught at the top level to trigger the CommentEditor fallback for the whole comment.
|
||||
class UnknownNodeError extends Error {}
|
||||
|
||||
// Protocol allowlist mirroring @tiptap/extension-link's default (the read-only
|
||||
// CommentEditor path relies on it to blank javascript:/data: hrefs). The static
|
||||
// renderer must apply the SAME sanitization because the backend stores comment
|
||||
// content verbatim and React does not neutralize javascript: in an href.
|
||||
const ALLOWED_URI_SCHEMES = /^(?:https?|ftps?|mailto|tel|callto|sms|cid|xmpp):/i;
|
||||
|
||||
function safeHref(href: unknown): string | undefined {
|
||||
if (typeof href !== "string") return undefined;
|
||||
// Strip control chars/whitespace that could smuggle a scheme past the test
|
||||
// (e.g. "java\tscript:").
|
||||
const cleaned = href.replace(/[\u0000-\u0020]/g, "").trim();
|
||||
// Allow relative/anchor/protocol-relative links (no scheme) — not script vectors.
|
||||
if (!/^[a-z][a-z0-9+.-]*:/i.test(cleaned)) return href;
|
||||
return ALLOWED_URI_SCHEMES.test(cleaned) ? href : undefined;
|
||||
}
|
||||
|
||||
interface PMMark {
|
||||
type: string;
|
||||
attrs?: Record<string, any>;
|
||||
}
|
||||
|
||||
interface PMNode {
|
||||
type: string;
|
||||
attrs?: Record<string, any>;
|
||||
content?: PMNode[];
|
||||
text?: string;
|
||||
marks?: PMMark[];
|
||||
}
|
||||
|
||||
// Wrap a text node's string in its marks (marks nest, e.g. bold + italic).
|
||||
function renderMarks(
|
||||
text: React.ReactNode,
|
||||
marks: PMMark[] | undefined,
|
||||
keyPrefix: string,
|
||||
): React.ReactNode {
|
||||
if (!marks || marks.length === 0) return text;
|
||||
|
||||
return marks.reduce<React.ReactNode>((acc, mark, i) => {
|
||||
const key = `${keyPrefix}-m${i}`;
|
||||
switch (mark.type) {
|
||||
case "bold":
|
||||
return <strong key={key}>{acc}</strong>;
|
||||
case "italic":
|
||||
return <em key={key}>{acc}</em>;
|
||||
case "strike":
|
||||
return <s key={key}>{acc}</s>;
|
||||
case "underline":
|
||||
// StarterKit enables the Underline extension by default (Mod-u) and
|
||||
// CommentEditor does not disable it, so real comments can carry this
|
||||
// mark. Render it here rather than degrading the whole comment.
|
||||
return <u key={key}>{acc}</u>;
|
||||
case "code":
|
||||
return <code key={key}>{acc}</code>;
|
||||
case "link": {
|
||||
// LinkExtension (TiptapLink) opens links in a new tab; keep the same
|
||||
// safe rel semantics the editor produces. Sanitize the href against the
|
||||
// extension's protocol allowlist — a disallowed scheme (javascript:,
|
||||
// data:) yields undefined so the anchor is non-navigable but still shows
|
||||
// its text, matching how extension-link blanks a bad href.
|
||||
const href = safeHref(mark.attrs?.href);
|
||||
return (
|
||||
<a
|
||||
key={key}
|
||||
href={href}
|
||||
target="_blank"
|
||||
rel="noopener noreferrer nofollow"
|
||||
>
|
||||
{acc}
|
||||
</a>
|
||||
);
|
||||
}
|
||||
default:
|
||||
throw new UnknownNodeError(`Unknown mark type: ${mark.type}`);
|
||||
}
|
||||
}, text);
|
||||
}
|
||||
|
||||
function renderNode(node: PMNode, key: string): React.ReactNode {
|
||||
switch (node.type) {
|
||||
case "paragraph":
|
||||
return <p key={key}>{renderChildren(node.content, key)}</p>;
|
||||
case "text":
|
||||
return (
|
||||
<React.Fragment key={key}>
|
||||
{renderMarks(node.text ?? "", node.marks, key)}
|
||||
</React.Fragment>
|
||||
);
|
||||
case "hardBreak":
|
||||
return <br key={key} />;
|
||||
case "mention":
|
||||
return (
|
||||
<span key={key} style={{ display: "inline" }}>
|
||||
<MentionContent attrs={node.attrs as any} />
|
||||
</span>
|
||||
);
|
||||
default:
|
||||
throw new UnknownNodeError(`Unknown node type: ${node.type}`);
|
||||
}
|
||||
}
|
||||
|
||||
function renderChildren(
|
||||
content: PMNode[] | undefined,
|
||||
keyPrefix: string,
|
||||
): React.ReactNode {
|
||||
if (!content) return null;
|
||||
return content.map((child, i) => renderNode(child, `${keyPrefix}-${i}`));
|
||||
}
|
||||
|
||||
// Reproduce the exact DOM nesting the read-only CommentEditor renders so the
|
||||
// scoped CSS in comment.module.css (which targets
|
||||
// `.commentEditor .ProseMirror :global(.ProseMirror)` and `.ProseMirror p`)
|
||||
// applies pixel-for-pixel. Read-only => no data-editable / data-surface attrs.
|
||||
function Shell({ children }: { children: React.ReactNode }) {
|
||||
return (
|
||||
<div className={classes.commentEditor}>
|
||||
<div className={classes.ProseMirror}>
|
||||
<div className="ProseMirror">{children}</div>
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
interface CommentContentViewProps {
|
||||
content: string | object;
|
||||
}
|
||||
|
||||
export function CommentContentView({ content }: CommentContentViewProps) {
|
||||
// Degrade this single comment to the old editor-based render (safety valve).
|
||||
const fallback = () => {
|
||||
if (import.meta.env.DEV) {
|
||||
console.warn(
|
||||
"CommentContentView: unsupported comment content, falling back to editor",
|
||||
);
|
||||
}
|
||||
return <CommentEditor defaultContent={content} editable={false} />;
|
||||
};
|
||||
|
||||
let doc: unknown = content;
|
||||
|
||||
if (typeof content === "string") {
|
||||
try {
|
||||
doc = JSON.parse(content);
|
||||
} catch {
|
||||
const trimmed = content.trim();
|
||||
// Looks like it was meant to be JSON but is malformed -> safety-valve fallback.
|
||||
if (trimmed.startsWith("{") || trimmed.startsWith("[")) {
|
||||
return fallback();
|
||||
}
|
||||
// Otherwise it's a legacy plain-text comment: render as a single paragraph.
|
||||
return (
|
||||
<Shell>
|
||||
<p>{content}</p>
|
||||
</Shell>
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// Double-stringified / legacy plain-text stored as a JSON string.
|
||||
if (typeof doc === "string") {
|
||||
return (
|
||||
<Shell>
|
||||
<p>{doc}</p>
|
||||
</Shell>
|
||||
);
|
||||
}
|
||||
|
||||
try {
|
||||
const pmDoc = doc as PMNode;
|
||||
if (!pmDoc || typeof pmDoc !== "object" || pmDoc.type !== "doc") {
|
||||
throw new UnknownNodeError("Not a ProseMirror doc");
|
||||
}
|
||||
return <Shell>{renderChildren(pmDoc.content, "n")}</Shell>;
|
||||
} catch (err) {
|
||||
if (err instanceof UnknownNodeError) {
|
||||
return fallback();
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
|
||||
export default CommentContentView;
|
||||
@@ -1,5 +1,5 @@
|
||||
import { describe, it, expect, vi } from "vitest";
|
||||
import { render, screen, fireEvent } from "@testing-library/react";
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import { render, screen, fireEvent, waitFor } from "@testing-library/react";
|
||||
import { MantineProvider } from "@mantine/core";
|
||||
import { IComment } from "@/features/comment/types/comment.types";
|
||||
|
||||
@@ -9,10 +9,11 @@ import { IComment } from "@/features/comment/types/comment.types";
|
||||
// component renders in isolation. We only assert the AI-badge rendering branch.
|
||||
const applyMutateAsync = vi.fn();
|
||||
const dismissMutateAsync = vi.fn();
|
||||
const updateMutateAsync = vi.fn();
|
||||
vi.mock("@/features/comment/queries/comment-query", () => ({
|
||||
useDeleteCommentMutation: () => ({ mutateAsync: vi.fn() }),
|
||||
useResolveCommentMutation: () => ({ mutateAsync: vi.fn() }),
|
||||
useUpdateCommentMutation: () => ({ mutateAsync: vi.fn() }),
|
||||
useUpdateCommentMutation: () => ({ mutateAsync: updateMutateAsync }),
|
||||
useApplySuggestionMutation: () => ({
|
||||
mutateAsync: applyMutateAsync,
|
||||
isPending: false,
|
||||
@@ -23,9 +24,51 @@ vi.mock("@/features/comment/queries/comment-query", () => ({
|
||||
}),
|
||||
}));
|
||||
|
||||
// The document the mocked editor emits via onUpdate when the edit form is open.
|
||||
// Duplicated inside the mock factory (below) to keep the factory self-contained.
|
||||
const EDITED_DOC = {
|
||||
type: "doc",
|
||||
content: [
|
||||
{ type: "paragraph", content: [{ type: "text", text: "edited via editor" }] },
|
||||
],
|
||||
};
|
||||
|
||||
// CommentEditor pulls in the full TipTap editor stack; replace it with a stub.
|
||||
vi.mock("@/features/comment/components/comment-editor", () => ({
|
||||
default: () => <div data-testid="comment-editor" />,
|
||||
// In edit mode the stub exposes buttons that fire the real onUpdate/onSave props
|
||||
// so the edit->save/cancel flow can be driven without a live editor.
|
||||
vi.mock("@/features/comment/components/comment-editor", () => {
|
||||
const doc = {
|
||||
type: "doc",
|
||||
content: [
|
||||
{ type: "paragraph", content: [{ type: "text", text: "edited via editor" }] },
|
||||
],
|
||||
};
|
||||
return {
|
||||
default: ({ onUpdate, onSave }: any) => (
|
||||
<div data-testid="comment-editor">
|
||||
<button
|
||||
type="button"
|
||||
data-testid="editor-emit-update"
|
||||
onClick={() => onUpdate?.(doc)}
|
||||
/>
|
||||
<button
|
||||
type="button"
|
||||
data-testid="editor-emit-save"
|
||||
onClick={() => onSave?.()}
|
||||
/>
|
||||
</div>
|
||||
),
|
||||
};
|
||||
});
|
||||
|
||||
// CommentContentView (used for the read-only body) imports the mention view,
|
||||
// which pulls page-query -> main.tsx (createRoot). Stub the queries so the item
|
||||
// renders in isolation without the app entry side-effect.
|
||||
vi.mock("@/features/page/queries/page-query.ts", () => ({
|
||||
usePageQuery: () => ({ data: undefined, isLoading: false, isError: false }),
|
||||
}));
|
||||
vi.mock("@/features/share/queries/share-query.ts", () => ({
|
||||
useSharePageQuery: () => ({ data: undefined }),
|
||||
}));
|
||||
|
||||
import CommentListItem from "./comment-list-item";
|
||||
@@ -286,3 +329,132 @@ describe("canShowDismiss predicate", () => {
|
||||
expect(canShowDismiss(c({ parentCommentId: "p" }), true, true)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("CommentListItem — edit -> save/cancel flow (#340 F3)", () => {
|
||||
const body = (t: string) =>
|
||||
JSON.stringify({
|
||||
type: "doc",
|
||||
content: [{ type: "paragraph", content: [{ type: "text", text: t }] }],
|
||||
});
|
||||
|
||||
// The edit menu item is gated on the viewer owning the comment
|
||||
// (currentUser.id === creatorId). currentUserAtom is atomWithStorage-backed,
|
||||
// so seed localStorage to make the viewer the owner (creatorId "user-1").
|
||||
beforeEach(() => {
|
||||
updateMutateAsync.mockClear();
|
||||
localStorage.setItem(
|
||||
"currentUser",
|
||||
JSON.stringify({ user: { id: "user-1", name: "Owner" } }),
|
||||
);
|
||||
});
|
||||
afterEach(() => {
|
||||
localStorage.clear();
|
||||
});
|
||||
|
||||
async function openEditor() {
|
||||
// Open the comment menu, then click "Edit comment" to toggle into edit mode.
|
||||
fireEvent.click(screen.getByLabelText("Comment menu"));
|
||||
fireEvent.click(await screen.findByText("Edit comment"));
|
||||
// Edit form (mocked editor + actions) is now mounted.
|
||||
await screen.findByTestId("comment-editor");
|
||||
}
|
||||
|
||||
it("saves the edited content and, on cache update, shows the new body", async () => {
|
||||
const { rerender } = renderItem(
|
||||
baseComment({ content: body("original body") }),
|
||||
);
|
||||
// Static body first.
|
||||
expect(screen.getByText("original body")).toBeDefined();
|
||||
|
||||
await openEditor();
|
||||
|
||||
// Editor emits an update (populates editContentRef), then Save is clicked.
|
||||
fireEvent.click(screen.getByTestId("editor-emit-update"));
|
||||
fireEvent.click(screen.getByRole("button", { name: "Save" }));
|
||||
|
||||
// mutateAsync is called with the stringified edited doc.
|
||||
expect(updateMutateAsync).toHaveBeenCalledWith({
|
||||
commentId: "c-1",
|
||||
content: JSON.stringify(EDITED_DOC),
|
||||
});
|
||||
|
||||
// On success the form closes (isEditing -> false); the static body renders
|
||||
// from the comment.content prop again.
|
||||
await waitFor(() =>
|
||||
expect(screen.queryByTestId("comment-editor")).toBeNull(),
|
||||
);
|
||||
|
||||
// Simulate the cache invalidation swapping in a new comment object with the
|
||||
// updated content — the static body reflects it.
|
||||
rerender(
|
||||
<MantineProvider>
|
||||
<CommentListItem
|
||||
comment={baseComment({ content: body("updated body after save") })}
|
||||
pageId="page-1"
|
||||
canComment={true}
|
||||
canEdit={true}
|
||||
/>
|
||||
</MantineProvider>,
|
||||
);
|
||||
expect(screen.getByText("updated body after save")).toBeDefined();
|
||||
expect(screen.queryByText("original body")).toBeNull();
|
||||
});
|
||||
|
||||
it("cancel restores the static body and does not call the update mutation", async () => {
|
||||
renderItem(baseComment({ content: body("original body") }));
|
||||
await openEditor();
|
||||
|
||||
// Type something (editContentRef set), then cancel.
|
||||
fireEvent.click(screen.getByTestId("editor-emit-update"));
|
||||
fireEvent.click(screen.getByRole("button", { name: "Cancel" }));
|
||||
|
||||
// Editor unmounts, static body restored, no save happened.
|
||||
await waitFor(() =>
|
||||
expect(screen.queryByTestId("comment-editor")).toBeNull(),
|
||||
);
|
||||
expect(screen.getByText("original body")).toBeDefined();
|
||||
expect(updateMutateAsync).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("saving without editing sends the existing content (editContentRef cleared after cancel)", async () => {
|
||||
renderItem(baseComment({ content: body("original body") }));
|
||||
|
||||
// Cancel path clears editContentRef...
|
||||
await openEditor();
|
||||
fireEvent.click(screen.getByTestId("editor-emit-update"));
|
||||
fireEvent.click(screen.getByRole("button", { name: "Cancel" }));
|
||||
await waitFor(() =>
|
||||
expect(screen.queryByTestId("comment-editor")).toBeNull(),
|
||||
);
|
||||
|
||||
// ...so re-opening and saving WITHOUT an update falls back to comment.content.
|
||||
await openEditor();
|
||||
fireEvent.click(screen.getByRole("button", { name: "Save" }));
|
||||
expect(updateMutateAsync).toHaveBeenCalledWith({
|
||||
commentId: "c-1",
|
||||
content: JSON.stringify(body("original body")),
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("CommentListItem — read-only body renders statically", () => {
|
||||
it("renders the comment body as static text without a TipTap editor", () => {
|
||||
renderItem(
|
||||
baseComment({
|
||||
content: JSON.stringify({
|
||||
type: "doc",
|
||||
content: [
|
||||
{
|
||||
type: "paragraph",
|
||||
content: [{ type: "text", text: "Hello static world" }],
|
||||
},
|
||||
],
|
||||
}),
|
||||
}),
|
||||
);
|
||||
// Body text is present...
|
||||
expect(screen.getByText("Hello static world")).toBeDefined();
|
||||
// ...and it did NOT go through the (mocked) CommentEditor instance.
|
||||
expect(screen.queryByTestId("comment-editor")).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,10 +1,11 @@
|
||||
import { Group, Text, Box, Badge, Button } from "@mantine/core";
|
||||
import { AgentAvatarStack } from "@/components/ui/agent-avatar-stack.tsx";
|
||||
import React, { useEffect, useMemo, useRef, useState } from "react";
|
||||
import React, { useMemo, useRef, useState } from "react";
|
||||
import classes from "./comment.module.css";
|
||||
import { useAtom, useAtomValue } from "jotai";
|
||||
import { useTimeAgo } from "@/hooks/use-time-ago";
|
||||
import CommentEditor from "@/features/comment/components/comment-editor";
|
||||
import CommentContentView from "@/features/comment/components/comment-content-view";
|
||||
import { pageEditorAtom } from "@/features/editor/atoms/editor-atoms";
|
||||
import CommentActions from "@/features/comment/components/comment-actions";
|
||||
import CommentMenu from "@/features/comment/components/comment-menu";
|
||||
@@ -50,7 +51,6 @@ function CommentListItem({
|
||||
const [isEditing, setIsEditing] = useState(false);
|
||||
const [isLoading, setIsLoading] = useState(false);
|
||||
const editor = useAtomValue(pageEditorAtom);
|
||||
const [content, setContent] = useState<string>(comment.content);
|
||||
const editContentRef = useRef<any>(null);
|
||||
const updateCommentMutation = useUpdateCommentMutation();
|
||||
const deleteCommentMutation = useDeleteCommentMutation(comment.pageId);
|
||||
@@ -78,22 +78,16 @@ function CommentListItem({
|
||||
const isOwnerOrAdmin =
|
||||
currentUser?.user?.id === comment.creatorId || userSpaceRole === "admin";
|
||||
|
||||
useEffect(() => {
|
||||
setContent(comment.content);
|
||||
}, [comment]);
|
||||
|
||||
async function handleUpdateComment() {
|
||||
try {
|
||||
setIsLoading(true);
|
||||
const commentToUpdate = {
|
||||
commentId: comment.id,
|
||||
content: JSON.stringify(editContentRef.current ?? content),
|
||||
content: JSON.stringify(editContentRef.current ?? comment.content),
|
||||
};
|
||||
await updateCommentMutation.mutateAsync(commentToUpdate);
|
||||
if (editContentRef.current) {
|
||||
setContent(editContentRef.current);
|
||||
editContentRef.current = null;
|
||||
}
|
||||
editContentRef.current = null;
|
||||
setIsEditing(false);
|
||||
} catch (error) {
|
||||
console.error("Failed to update comment:", error);
|
||||
@@ -350,11 +344,11 @@ function CommentListItem({
|
||||
)}
|
||||
|
||||
{!isEditing ? (
|
||||
<CommentEditor defaultContent={content} editable={false} />
|
||||
<CommentContentView content={comment.content} />
|
||||
) : (
|
||||
<>
|
||||
<CommentEditor
|
||||
defaultContent={content}
|
||||
defaultContent={comment.content}
|
||||
editable={true}
|
||||
onUpdate={(newContent: any) => { editContentRef.current = newContent; }}
|
||||
onSave={handleUpdateComment}
|
||||
@@ -374,4 +368,6 @@ function CommentListItem({
|
||||
);
|
||||
}
|
||||
|
||||
export default CommentListItem;
|
||||
// Memoized so a resolve/apply/reply cache update (which only replaces the touched
|
||||
// comment's object identity) re-renders that one thread, not all ~356 items.
|
||||
export default React.memo(CommentListItem);
|
||||
|
||||
@@ -0,0 +1,108 @@
|
||||
import { describe, it, expect, vi } from "vitest";
|
||||
import { render, screen, fireEvent } from "@testing-library/react";
|
||||
import { MantineProvider } from "@mantine/core";
|
||||
import { IComment } from "@/features/comment/types/comment.types";
|
||||
|
||||
// matchMedia (read by MantineProvider) is stubbed globally in vitest.setup.ts.
|
||||
|
||||
// CommentEditor pulls in the full TipTap editor stack; replace it with a stub so
|
||||
// the lazy reply editor's mount transition can be observed without the editor.
|
||||
vi.mock("@/features/comment/components/comment-editor", () => ({
|
||||
default: () => <div data-testid="comment-editor" />,
|
||||
}));
|
||||
|
||||
// page-query -> main.tsx (createRoot) is a module side effect; stub the queries
|
||||
// pulled in transitively so importing the module is side-effect free.
|
||||
vi.mock("@/features/page/queries/page-query.ts", () => ({
|
||||
usePageQuery: () => ({ data: undefined, isLoading: false, isError: false }),
|
||||
}));
|
||||
vi.mock("@/features/share/queries/share-query.ts", () => ({
|
||||
useSharePageQuery: () => ({ data: undefined }),
|
||||
}));
|
||||
// space-query -> main.tsx (createRoot) is another module side effect; stub it.
|
||||
vi.mock("@/features/space/queries/space-query.ts", () => ({
|
||||
useGetSpaceBySlugQuery: () => ({ data: undefined }),
|
||||
}));
|
||||
|
||||
import {
|
||||
buildChildrenByParent,
|
||||
CommentEditorWithActions,
|
||||
} from "./comment-list-with-tabs";
|
||||
|
||||
const c = (id: string, parentCommentId: string | null = null): IComment =>
|
||||
({ id, parentCommentId }) as IComment;
|
||||
|
||||
describe("buildChildrenByParent (childrenByParent grouping)", () => {
|
||||
it("returns an empty map for undefined or empty input", () => {
|
||||
expect(buildChildrenByParent(undefined).size).toBe(0);
|
||||
expect(buildChildrenByParent([]).size).toBe(0);
|
||||
});
|
||||
|
||||
it("does not index a top-level comment (parentCommentId null)", () => {
|
||||
const map = buildChildrenByParent([c("p1", null)]);
|
||||
expect(map.size).toBe(0);
|
||||
expect(map.has("p1")).toBe(false);
|
||||
});
|
||||
|
||||
it("groups replies under the correct parent, including reply-to-reply nesting", () => {
|
||||
const p1 = c("p1", null);
|
||||
const r1 = c("r1", "p1");
|
||||
const r2 = c("r2", "r1"); // a reply to a reply
|
||||
const map = buildChildrenByParent([p1, r1, r2]);
|
||||
expect(map.get("p1")).toEqual([r1]);
|
||||
expect(map.get("r1")).toEqual([r2]);
|
||||
// The top-level comment itself is never a key.
|
||||
expect(map.has("p1") && map.get("p1")?.length).toBe(1);
|
||||
});
|
||||
|
||||
it("still groups a reply whose parent is not present in items", () => {
|
||||
const orphan = c("o1", "missing-parent");
|
||||
const map = buildChildrenByParent([orphan]);
|
||||
expect(map.get("missing-parent")).toEqual([orphan]);
|
||||
});
|
||||
|
||||
it("preserves insertion order among sibling replies", () => {
|
||||
const map = buildChildrenByParent([
|
||||
c("a", "p1"),
|
||||
c("b", "p1"),
|
||||
c("d", "p1"),
|
||||
]);
|
||||
expect(map.get("p1")?.map((x) => x.id)).toEqual(["a", "b", "d"]);
|
||||
});
|
||||
});
|
||||
|
||||
function renderReplyEditor() {
|
||||
return render(
|
||||
<MantineProvider>
|
||||
<CommentEditorWithActions commentId="c-1" onSave={vi.fn()} />
|
||||
</MantineProvider>,
|
||||
);
|
||||
}
|
||||
|
||||
describe("CommentEditorWithActions — lazy reply editor activation", () => {
|
||||
it("shows only the stub initially (no editor instance mounted)", () => {
|
||||
renderReplyEditor();
|
||||
expect(screen.getByRole("button")).toBeDefined();
|
||||
expect(screen.queryByTestId("comment-editor")).toBeNull();
|
||||
});
|
||||
|
||||
it("mounts the real editor when the stub is clicked and keeps it mounted", () => {
|
||||
renderReplyEditor();
|
||||
fireEvent.click(screen.getByRole("button"));
|
||||
expect(screen.getByTestId("comment-editor")).toBeDefined();
|
||||
// The stub button is replaced by the editor subtree.
|
||||
expect(screen.queryByRole("button")).toBeNull();
|
||||
});
|
||||
|
||||
it("mounts the editor when the stub receives focus", () => {
|
||||
renderReplyEditor();
|
||||
fireEvent.focus(screen.getByRole("button"));
|
||||
expect(screen.getByTestId("comment-editor")).toBeDefined();
|
||||
});
|
||||
|
||||
it("mounts the editor on Enter keydown of the stub", () => {
|
||||
renderReplyEditor();
|
||||
fireEvent.keyDown(screen.getByRole("button"), { key: "Enter" });
|
||||
expect(screen.getByTestId("comment-editor")).toBeDefined();
|
||||
});
|
||||
});
|
||||
@@ -22,8 +22,7 @@ import CommentEditor from "@/features/comment/components/comment-editor";
|
||||
import CommentActions from "@/features/comment/components/comment-actions";
|
||||
import { useFocusWithin } from "@mantine/hooks";
|
||||
import { IComment } from "@/features/comment/types/comment.types.ts";
|
||||
import { usePageQuery } from "@/features/page/queries/page-query.ts";
|
||||
import { IPagination } from "@/lib/types.ts";
|
||||
import { usePageMetaQuery } from "@/features/page/queries/page-query.ts";
|
||||
import { extractPageSlugId } from "@/lib";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import { useGetSpaceBySlugQuery } from "@/features/space/queries/space-query.ts";
|
||||
@@ -36,17 +35,37 @@ interface CommentListWithTabsProps {
|
||||
onClose?: () => void;
|
||||
}
|
||||
|
||||
// Index replies by their parent id once (O(n)), instead of an O(n^2) filter per
|
||||
// thread. Replies whose parent is not in `items` are still grouped under their
|
||||
// parentCommentId (they simply won't be reached by the top-level walk).
|
||||
// Exported for unit testing.
|
||||
export function buildChildrenByParent(
|
||||
items: IComment[] | undefined,
|
||||
): Map<string, IComment[]> {
|
||||
const m = new Map<string, IComment[]>();
|
||||
for (const c of items ?? []) {
|
||||
if (c.parentCommentId) {
|
||||
const arr = m.get(c.parentCommentId);
|
||||
if (arr) arr.push(c);
|
||||
else m.set(c.parentCommentId, [c]);
|
||||
}
|
||||
}
|
||||
return m;
|
||||
}
|
||||
|
||||
function CommentListWithTabs({ onClose }: CommentListWithTabsProps) {
|
||||
const { t } = useTranslation();
|
||||
const { pageSlug } = useParams();
|
||||
const { data: page } = usePageQuery({ pageId: extractPageSlugId(pageSlug) });
|
||||
const { data: page } = usePageMetaQuery({ pageId: extractPageSlugId(pageSlug) });
|
||||
const {
|
||||
data: comments,
|
||||
isLoading: isCommentsLoading,
|
||||
isError,
|
||||
} = useCommentsQuery({ pageId: page?.id });
|
||||
const createCommentMutation = useCreateCommentMutation();
|
||||
const [isLoading, setIsLoading] = useState(false);
|
||||
// mutateAsync is a stable reference across renders; depend on it (not the
|
||||
// mutation object) so the reply/comment callbacks stay stable.
|
||||
const createCommentAsync = createCommentMutation.mutateAsync;
|
||||
const { data: space } = useGetSpaceBySlugQuery(page?.space?.slug);
|
||||
|
||||
const canEdit = page?.permissions?.canEdit ?? false;
|
||||
@@ -75,13 +94,21 @@ function CommentListWithTabs({ onClose }: CommentListWithTabsProps) {
|
||||
return { activeComments: active, resolvedComments: resolved };
|
||||
}, [comments]);
|
||||
|
||||
// Index replies by their parent once, instead of an O(n^2) filter per thread.
|
||||
// The map ref changes on any comments update, so MemoizedChildComments re-runs
|
||||
// (cheap) and re-looks-up, while memoized CommentListItems skip unchanged items.
|
||||
const childrenByParent = useMemo(
|
||||
() => buildChildrenByParent(comments?.items),
|
||||
[comments?.items],
|
||||
);
|
||||
|
||||
const [isPageCommentLoading, setIsPageCommentLoading] = useState(false);
|
||||
|
||||
const handleAddPageComment = useCallback(
|
||||
async (_commentId: string, content: string) => {
|
||||
try {
|
||||
setIsPageCommentLoading(true);
|
||||
const createdComment = await createCommentMutation.mutateAsync({
|
||||
const createdComment = await createCommentAsync({
|
||||
pageId: page?.id,
|
||||
content: JSON.stringify(content),
|
||||
});
|
||||
@@ -100,27 +127,26 @@ function CommentListWithTabs({ onClose }: CommentListWithTabsProps) {
|
||||
setIsPageCommentLoading(false);
|
||||
}
|
||||
},
|
||||
[createCommentMutation, page?.id],
|
||||
[createCommentAsync, page?.id],
|
||||
);
|
||||
|
||||
const handleAddReply = useCallback(
|
||||
async (commentId: string, content: string) => {
|
||||
// Pending state lives inside CommentEditorWithActions so sending a reply
|
||||
// does not churn renderComments and re-render the whole list.
|
||||
try {
|
||||
setIsLoading(true);
|
||||
const commentData = {
|
||||
pageId: page?.id,
|
||||
parentCommentId: commentId,
|
||||
content: JSON.stringify(content),
|
||||
};
|
||||
|
||||
await createCommentMutation.mutateAsync(commentData);
|
||||
await createCommentAsync(commentData);
|
||||
} catch (error) {
|
||||
console.error("Failed to post comment:", error);
|
||||
} finally {
|
||||
setIsLoading(false);
|
||||
}
|
||||
},
|
||||
[createCommentMutation, page?.id],
|
||||
[createCommentAsync, page?.id],
|
||||
);
|
||||
|
||||
const renderComments = useCallback(
|
||||
@@ -143,7 +169,7 @@ function CommentListWithTabs({ onClose }: CommentListWithTabsProps) {
|
||||
userSpaceRole={space?.membership?.role}
|
||||
/>
|
||||
<MemoizedChildComments
|
||||
comments={comments}
|
||||
childrenByParent={childrenByParent}
|
||||
parentId={comment.id}
|
||||
pageId={page?.id}
|
||||
canComment={canComment}
|
||||
@@ -158,16 +184,15 @@ function CommentListWithTabs({ onClose }: CommentListWithTabsProps) {
|
||||
<CommentEditorWithActions
|
||||
commentId={comment.id}
|
||||
onSave={handleAddReply}
|
||||
isLoading={isLoading}
|
||||
/>
|
||||
</>
|
||||
)}
|
||||
</Paper>
|
||||
),
|
||||
[
|
||||
comments,
|
||||
childrenByParent,
|
||||
handleAddReply,
|
||||
isLoading,
|
||||
page?.id,
|
||||
space?.membership?.role,
|
||||
canComment,
|
||||
canEdit,
|
||||
@@ -203,6 +228,11 @@ function CommentListWithTabs({ onClose }: CommentListWithTabsProps) {
|
||||
<Tabs
|
||||
defaultValue="open"
|
||||
variant="default"
|
||||
// Default to not mounting an inactive tab (the heavy Resolved list stays
|
||||
// unmounted while Open is shown). The Open panel overrides this with its
|
||||
// own keepMounted (below) so an in-progress reply/edit draft survives an
|
||||
// Open -> Resolved -> Open switch.
|
||||
keepMounted={false}
|
||||
style={{
|
||||
flex: "1 1 auto",
|
||||
display: "flex",
|
||||
@@ -261,7 +291,10 @@ function CommentListWithTabs({ onClose }: CommentListWithTabsProps) {
|
||||
type="scroll"
|
||||
>
|
||||
<div style={{ paddingBottom: "8px" }}>
|
||||
<Tabs.Panel value="open" pt="xs">
|
||||
{/* keepMounted keeps the Open panel alive even while Resolved is
|
||||
active, so a lazily-mounted reply editor's draft (and an
|
||||
in-progress edit) is not discarded on tab switch. */}
|
||||
<Tabs.Panel value="open" pt="xs" keepMounted>
|
||||
{activeComments.length === 0 ? (
|
||||
<Center py="xl">
|
||||
<Stack align="center" gap="xs">
|
||||
@@ -307,7 +340,7 @@ function CommentListWithTabs({ onClose }: CommentListWithTabsProps) {
|
||||
}
|
||||
|
||||
interface ChildCommentsProps {
|
||||
comments: IPagination<IComment>;
|
||||
childrenByParent: Map<string, IComment[]>;
|
||||
parentId: string;
|
||||
pageId: string;
|
||||
canComment: boolean;
|
||||
@@ -315,24 +348,18 @@ interface ChildCommentsProps {
|
||||
userSpaceRole?: string;
|
||||
}
|
||||
const ChildComments = ({
|
||||
comments,
|
||||
childrenByParent,
|
||||
parentId,
|
||||
pageId,
|
||||
canComment,
|
||||
canEdit,
|
||||
userSpaceRole,
|
||||
}: ChildCommentsProps) => {
|
||||
const getChildComments = useCallback(
|
||||
(parentId: string) =>
|
||||
comments.items.filter(
|
||||
(comment: IComment) => comment.parentCommentId === parentId,
|
||||
),
|
||||
[comments.items],
|
||||
);
|
||||
const children = childrenByParent.get(parentId) ?? [];
|
||||
|
||||
return (
|
||||
<div>
|
||||
{getChildComments(parentId).map((childComment) => (
|
||||
{children.map((childComment) => (
|
||||
<div key={childComment.id}>
|
||||
<CommentListItem
|
||||
comment={childComment}
|
||||
@@ -342,7 +369,7 @@ const ChildComments = ({
|
||||
userSpaceRole={userSpaceRole}
|
||||
/>
|
||||
<MemoizedChildComments
|
||||
comments={comments}
|
||||
childrenByParent={childrenByParent}
|
||||
parentId={childComment.id}
|
||||
pageId={pageId}
|
||||
canComment={canComment}
|
||||
@@ -357,22 +384,61 @@ const ChildComments = ({
|
||||
|
||||
const MemoizedChildComments = memo(ChildComments);
|
||||
|
||||
const CommentEditorWithActions = ({
|
||||
export const CommentEditorWithActions = ({
|
||||
commentId,
|
||||
onSave,
|
||||
isLoading,
|
||||
placeholder = undefined,
|
||||
}) => {
|
||||
const { t } = useTranslation();
|
||||
// Lazily mount the TipTap reply editor: until the user interacts with the
|
||||
// stub, no editor instance is created for this thread. Once mounted it stays
|
||||
// mounted so the draft is preserved.
|
||||
const [mounted, setMounted] = useState(false);
|
||||
const [content, setContent] = useState("");
|
||||
const [isSending, setIsSending] = useState(false);
|
||||
const { ref, focused } = useFocusWithin();
|
||||
const commentEditorRef = useRef(null);
|
||||
|
||||
const handleSave = useCallback(() => {
|
||||
onSave(commentId, content);
|
||||
setContent("");
|
||||
commentEditorRef.current?.clearContent();
|
||||
const activate = useCallback(() => setMounted(true), []);
|
||||
|
||||
const handleSave = useCallback(async () => {
|
||||
try {
|
||||
setIsSending(true);
|
||||
await onSave(commentId, content);
|
||||
setContent("");
|
||||
commentEditorRef.current?.clearContent();
|
||||
} finally {
|
||||
setIsSending(false);
|
||||
}
|
||||
}, [commentId, content, onSave]);
|
||||
|
||||
if (!mounted) {
|
||||
return (
|
||||
<div
|
||||
role="button"
|
||||
tabIndex={0}
|
||||
onClick={activate}
|
||||
onFocus={activate}
|
||||
onKeyDown={(e) => {
|
||||
if (e.key === "Enter" || e.key === " ") {
|
||||
e.preventDefault();
|
||||
activate();
|
||||
}
|
||||
}}
|
||||
style={{
|
||||
padding: "6px",
|
||||
fontSize: "var(--mantine-font-size-sm)",
|
||||
lineHeight: 1.4,
|
||||
color: "var(--mantine-color-placeholder)",
|
||||
cursor: "text",
|
||||
borderRadius: "var(--mantine-radius-sm)",
|
||||
}}
|
||||
>
|
||||
{placeholder || t("Reply...")}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
return (
|
||||
<div ref={ref}>
|
||||
<CommentEditor
|
||||
@@ -381,8 +447,9 @@ const CommentEditorWithActions = ({
|
||||
onSave={handleSave}
|
||||
editable={true}
|
||||
placeholder={placeholder}
|
||||
autofocus={true}
|
||||
/>
|
||||
{focused && <CommentActions onSave={handleSave} isLoading={isLoading} />}
|
||||
{focused && <CommentActions onSave={handleSave} isLoading={isSending} />}
|
||||
</div>
|
||||
);
|
||||
};
|
||||
|
||||
@@ -53,7 +53,10 @@ export function useCommentsQuery(params: ICommentParams) {
|
||||
|
||||
return {
|
||||
data,
|
||||
isLoading: query.isLoading || query.hasNextPage,
|
||||
// Paint the first page as soon as it arrives instead of blocking until every
|
||||
// page has loaded; the background effect above keeps streaming the rest
|
||||
// (tab counts grow as pages arrive).
|
||||
isLoading: query.isLoading,
|
||||
isError: query.isError,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -24,7 +24,7 @@ import classes from "./link.module.css";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import { INTERNAL_LINK_REGEX } from "@/lib/constants";
|
||||
import { LinkEditorPanel } from "@/features/editor/components/link/link-editor-panel.tsx";
|
||||
import { usePageQuery } from "@/features/page/queries/page-query.ts";
|
||||
import { usePageMetaQuery } from "@/features/page/queries/page-query.ts";
|
||||
import { useSharePageQuery } from "@/features/share/queries/share-query.ts";
|
||||
import { buildSharedPageUrl } from "@/features/page/page.utils.ts";
|
||||
import { extractPageSlugId } from "@/lib";
|
||||
@@ -83,7 +83,7 @@ export default function LinkView(props: MarkViewProps) {
|
||||
const isPopoverVisible = popoverState !== "closed";
|
||||
const activeView = isPopoverVisible ? popoverState : lastOpenState.current;
|
||||
|
||||
const { data: linkedPage } = usePageQuery({
|
||||
const { data: linkedPage } = usePageMetaQuery({
|
||||
pageId: isPopoverVisible && slugId && !isShareRoute ? slugId : null,
|
||||
});
|
||||
|
||||
|
||||
@@ -25,7 +25,7 @@ import { IconFileDescription, IconPlus } from "@tabler/icons-react";
|
||||
import { useSpaceQuery } from "@/features/space/queries/space-query.ts";
|
||||
import { useParams } from "react-router-dom";
|
||||
import { v7 as uuid7 } from "uuid";
|
||||
import { useAtom } from "jotai";
|
||||
import { useAtom, useSetAtom, useStore } from "jotai";
|
||||
import { currentUserAtom } from "@/features/user/atoms/current-user-atom.ts";
|
||||
import {
|
||||
MentionListProps,
|
||||
@@ -34,7 +34,7 @@ import {
|
||||
import { IPage } from "@/features/page/types/page.types";
|
||||
import {
|
||||
useCreatePageMutation,
|
||||
usePageQuery,
|
||||
usePageMetaQuery,
|
||||
} from "@/features/page/queries/page-query";
|
||||
import { treeDataAtom } from "@/features/page/tree/atoms/tree-data-atom";
|
||||
import { treeModel } from "@/features/page/tree/model/tree-model";
|
||||
@@ -50,12 +50,16 @@ const MentionList = forwardRef<any, MentionListProps>((props, ref) => {
|
||||
const [countAnnouncement, setCountAnnouncement] = useState("");
|
||||
const [selectionAnnouncement, setSelectionAnnouncement] = useState("");
|
||||
const { pageSlug, spaceSlug } = useParams();
|
||||
const { data: page } = usePageQuery({ pageId: extractPageSlugId(pageSlug) });
|
||||
const { data: page } = usePageMetaQuery({ pageId: extractPageSlugId(pageSlug) });
|
||||
const { data: space } = useSpaceQuery(spaceSlug);
|
||||
const [currentUser] = useAtom(currentUserAtom);
|
||||
const [renderItems, setRenderItems] = useState<MentionSuggestionItem[]>([]);
|
||||
const { t } = useTranslation();
|
||||
const [data, setData] = useAtom(treeDataAtom);
|
||||
// Setter-only: the tree value is read only imperatively inside createPage
|
||||
// (via `store` below), never at render, so useSetAtom avoids re-rendering the
|
||||
// mention popup on any tree event.
|
||||
const setData = useSetAtom(treeDataAtom);
|
||||
const store = useStore();
|
||||
const createPageMutation = useCreatePageMutation();
|
||||
const emit = useQueryEmit();
|
||||
const isInCommentContext = props.isInCommentContext ?? false;
|
||||
@@ -272,9 +276,11 @@ const MentionList = forwardRef<any, MentionListProps>((props, ref) => {
|
||||
children: [],
|
||||
};
|
||||
|
||||
const lastIndex = data.length;
|
||||
// Read the live tree imperatively at call time.
|
||||
const currentTree = store.get(treeDataAtom);
|
||||
const lastIndex = currentTree.length;
|
||||
|
||||
setData(treeModel.insert(data, parentId, newNode, lastIndex));
|
||||
setData(treeModel.insert(currentTree, parentId, newNode, lastIndex));
|
||||
|
||||
props.command({
|
||||
id: uuid7(),
|
||||
|
||||
@@ -2,7 +2,7 @@ import { NodeViewProps, NodeViewWrapper } from "@tiptap/react";
|
||||
import { ActionIcon, Anchor, Text } from "@mantine/core";
|
||||
import { IconFileDescription } from "@tabler/icons-react";
|
||||
import { Link, useLocation, useNavigate, useParams } from "react-router-dom";
|
||||
import { usePageQuery } from "@/features/page/queries/page-query.ts";
|
||||
import { usePageMetaQuery } from "@/features/page/queries/page-query.ts";
|
||||
import { useSharePageQuery } from "@/features/share/queries/share-query.ts";
|
||||
import {
|
||||
buildPageUrl,
|
||||
@@ -11,9 +11,19 @@ import {
|
||||
import { extractPageSlugId } from "@/lib";
|
||||
import classes from "./mention.module.css";
|
||||
|
||||
export default function MentionView(props: NodeViewProps) {
|
||||
const { node } = props;
|
||||
const { label, entityType, entityId, slugId, anchorId } = node.attrs;
|
||||
interface MentionAttrs {
|
||||
label?: string;
|
||||
entityType?: string;
|
||||
entityId?: string;
|
||||
slugId?: string;
|
||||
anchorId?: string;
|
||||
}
|
||||
|
||||
// Presentational mention renderer (no NodeViewWrapper). Shared by the editor
|
||||
// NodeView (MentionView) and the static comment renderer (CommentContentView)
|
||||
// so mention click/nav/icon behavior stays identical outside of an editor.
|
||||
export function MentionContent({ attrs }: { attrs: MentionAttrs }) {
|
||||
const { label, entityType, slugId, anchorId } = attrs;
|
||||
const isPageMention = entityType === "page";
|
||||
const { spaceSlug, pageSlug } = useParams();
|
||||
const { shareId } = useParams();
|
||||
@@ -26,7 +36,7 @@ export default function MentionView(props: NodeViewProps) {
|
||||
data: page,
|
||||
isLoading,
|
||||
isError,
|
||||
} = usePageQuery({ pageId: isPageMention && !isShareRoute ? slugId : null });
|
||||
} = usePageMetaQuery({ pageId: isPageMention && !isShareRoute ? slugId : null });
|
||||
|
||||
const { data: sharedPage } = useSharePageQuery({
|
||||
pageId: isPageMention && isShareRoute ? slugId : undefined,
|
||||
@@ -56,7 +66,7 @@ export default function MentionView(props: NodeViewProps) {
|
||||
});
|
||||
|
||||
return (
|
||||
<NodeViewWrapper style={{ display: "inline" }} data-drag-handle>
|
||||
<>
|
||||
{entityType === "user" && (
|
||||
<Text className={classes.userMention} component="span">
|
||||
@{label}
|
||||
@@ -139,6 +149,14 @@ export default function MentionView(props: NodeViewProps) {
|
||||
</span>
|
||||
</Anchor>
|
||||
)}
|
||||
</>
|
||||
);
|
||||
}
|
||||
|
||||
export default function MentionView(props: NodeViewProps) {
|
||||
return (
|
||||
<NodeViewWrapper style={{ display: "inline" }} data-drag-handle>
|
||||
<MentionContent attrs={props.node.attrs} />
|
||||
</NodeViewWrapper>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -24,7 +24,6 @@ export function useFavoritesQuery(type?: FavoriteType, spaceId?: string) {
|
||||
initialPageParam: undefined as string | undefined,
|
||||
getNextPageParam: (lastPage) =>
|
||||
lastPage.meta.hasNextPage ? lastPage.meta.nextCursor : undefined,
|
||||
refetchOnMount: true,
|
||||
});
|
||||
}
|
||||
|
||||
@@ -32,7 +31,6 @@ export function useFavoriteIds(type: FavoriteType, spaceId?: string): Set<string
|
||||
const { data } = useQuery({
|
||||
queryKey: ["favorite-ids", type, spaceId],
|
||||
queryFn: () => getFavoriteIds(type, spaceId),
|
||||
refetchOnMount: true,
|
||||
});
|
||||
|
||||
const items = data?.items;
|
||||
|
||||
@@ -12,7 +12,7 @@ import { useAtomValue } from "jotai";
|
||||
import { useParams } from "react-router-dom";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import { extractPageSlugId } from "@/lib";
|
||||
import { usePageQuery } from "@/features/page/queries/page-query.ts";
|
||||
import { usePageMetaQuery } from "@/features/page/queries/page-query.ts";
|
||||
import { pageEditorAtom } from "@/features/editor/atoms/editor-atoms.ts";
|
||||
import { useBacklinksCountQuery } from "@/features/page-details/queries/backlinks-query.ts";
|
||||
import { BacklinksModal } from "./backlinks-modal";
|
||||
@@ -23,7 +23,7 @@ import { LabelsSection } from "@/features/label/components/labels-section.tsx";
|
||||
|
||||
export function PageDetailsAside() {
|
||||
const { pageSlug } = useParams();
|
||||
const { data: page } = usePageQuery({
|
||||
const { data: page } = usePageMetaQuery({
|
||||
pageId: extractPageSlugId(pageSlug),
|
||||
});
|
||||
const pageEditor = useAtomValue(pageEditorAtom);
|
||||
|
||||
@@ -0,0 +1,53 @@
|
||||
import { describe, it, expect, vi } from "vitest";
|
||||
import { SpaceTreeNode } from "@/features/page/tree/types";
|
||||
|
||||
// breadcrumb.tsx transitively imports @/main.tsx (via usePageMetaQuery ->
|
||||
// queryClient), whose module body calls ReactDOM.createRoot on a null root in
|
||||
// jsdom. Stub it so importing the pure helper under test doesn't run that
|
||||
// (breadcrumbPathEqual does not use queryClient, so a dummy is enough).
|
||||
vi.mock("@/main.tsx", () => ({ queryClient: {} }));
|
||||
|
||||
import { breadcrumbPathEqual } from "./breadcrumb";
|
||||
|
||||
// breadcrumbPathEqual is the ONLY point where a false-positive equality would
|
||||
// leave a stale/incorrect breadcrumb trail on screen: it decides whether the
|
||||
// selectAtom hands back the same reference (no re-render) for the ancestor chain.
|
||||
// Pin both directions — a too-loose equality goes stale on a rename; a too-tight
|
||||
// one loses the perf win.
|
||||
const node = (over: Partial<SpaceTreeNode>): SpaceTreeNode =>
|
||||
({ id: "a", slugId: "sa", name: "A", icon: "📄", ...over }) as SpaceTreeNode;
|
||||
|
||||
describe("breadcrumbPathEqual", () => {
|
||||
it("both null → true", () => {
|
||||
expect(breadcrumbPathEqual(null, null)).toBe(true);
|
||||
});
|
||||
|
||||
it("same reference → true", () => {
|
||||
const p = [node({})];
|
||||
expect(breadcrumbPathEqual(p, p)).toBe(true);
|
||||
});
|
||||
|
||||
it("equal by id/slugId/name/icon (different arrays) → true", () => {
|
||||
expect(breadcrumbPathEqual([node({})], [node({})])).toBe(true);
|
||||
});
|
||||
|
||||
it("one side null → false", () => {
|
||||
expect(breadcrumbPathEqual([node({})], null)).toBe(false);
|
||||
expect(breadcrumbPathEqual(null, [node({})])).toBe(false);
|
||||
});
|
||||
|
||||
it("different length → false", () => {
|
||||
expect(
|
||||
breadcrumbPathEqual([node({})], [node({}), node({ id: "b" })]),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it.each(["name", "icon", "slugId", "id"] as const)(
|
||||
"a changed %s → false (breadcrumb must re-render)",
|
||||
(field) => {
|
||||
expect(
|
||||
breadcrumbPathEqual([node({})], [node({ [field]: "CHANGED" })]),
|
||||
).toBe(false);
|
||||
},
|
||||
);
|
||||
});
|
||||
@@ -1,7 +1,9 @@
|
||||
import { useAtomValue } from "jotai";
|
||||
import { selectAtom } from "jotai/utils";
|
||||
import { treeDataAtom } from "@/features/page/tree/atoms/tree-data-atom.ts";
|
||||
import React, { useCallback, useEffect, useState } from "react";
|
||||
import React, { useCallback, useEffect, useMemo, useState } from "react";
|
||||
import { computeBreadcrumbState } from "./breadcrumb.utils";
|
||||
import { findBreadcrumbPath } from "@/features/page/tree/utils";
|
||||
import {
|
||||
Button,
|
||||
Anchor,
|
||||
@@ -18,7 +20,7 @@ import { SpaceTreeNode } from "@/features/page/tree/types.ts";
|
||||
import { IPage } from "@/features/page/types/page.types.ts";
|
||||
import { buildPageUrl } from "@/features/page/page.utils.ts";
|
||||
import {
|
||||
usePageQuery,
|
||||
usePageMetaQuery,
|
||||
usePageBreadcrumbsQuery,
|
||||
} from "@/features/page/queries/page-query.ts";
|
||||
import { extractPageSlugId } from "@/lib";
|
||||
@@ -32,39 +34,84 @@ function getTitle(name: string, icon: string) {
|
||||
return name;
|
||||
}
|
||||
|
||||
/**
|
||||
* Equality over a breadcrumb chain by the only fields the breadcrumb renders
|
||||
* (id, slugId, name, icon). Lets the selectAtom below hand back the SAME
|
||||
* reference when an unrelated tree mutation leaves THIS page's ancestor chain
|
||||
* visually unchanged, so the breadcrumb no longer re-renders on every tree
|
||||
* event (it previously subscribed to the whole treeDataAtom).
|
||||
*/
|
||||
export function breadcrumbPathEqual(
|
||||
a: SpaceTreeNode[] | null,
|
||||
b: SpaceTreeNode[] | null,
|
||||
): boolean {
|
||||
if (a === b) return true;
|
||||
if (!a || !b || a.length !== b.length) return false;
|
||||
for (let i = 0; i < a.length; i++) {
|
||||
if (
|
||||
a[i].id !== b[i].id ||
|
||||
a[i].slugId !== b[i].slugId ||
|
||||
a[i].name !== b[i].name ||
|
||||
a[i].icon !== b[i].icon
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
export default function Breadcrumb() {
|
||||
const { t } = useTranslation();
|
||||
const treeData = useAtomValue(treeDataAtom);
|
||||
const [breadcrumbNodes, setBreadcrumbNodes] = useState<
|
||||
SpaceTreeNode[] | null
|
||||
>(null);
|
||||
const { pageSlug, spaceSlug } = useParams();
|
||||
const { data: currentPage } = usePageQuery({
|
||||
const { data: currentPage } = usePageMetaQuery({
|
||||
pageId: extractPageSlugId(pageSlug),
|
||||
});
|
||||
const currentPageId = currentPage?.id;
|
||||
// The page's own ancestor chain, fetched independently of the lazily-built
|
||||
// sidebar tree so a deep page doesn't render a blank breadcrumb for seconds
|
||||
// while the tree backfills (#218).
|
||||
const { data: ancestors } = usePageBreadcrumbsQuery(currentPage?.id);
|
||||
const { data: ancestors } = usePageBreadcrumbsQuery(currentPageId);
|
||||
const isMobile = useMediaQuery("(max-width: 48em)");
|
||||
|
||||
// Narrowed subscription: instead of subscribing to the whole treeDataAtom and
|
||||
// recomputing on every tree event, derive ONLY the current page's ancestor
|
||||
// chain. The custom equality returns the previous reference when that chain is
|
||||
// visually unchanged, so an unrelated tree mutation no longer re-renders this
|
||||
// component. Mirrors computeBreadcrumbState's tree-hit branch
|
||||
// (findBreadcrumbPath); the tree-miss/ancestors fallback is applied below.
|
||||
const treePathAtom = useMemo(
|
||||
() =>
|
||||
selectAtom(
|
||||
treeDataAtom,
|
||||
(tree): SpaceTreeNode[] | null =>
|
||||
currentPageId ? findBreadcrumbPath(tree, currentPageId) : null,
|
||||
breadcrumbPathEqual,
|
||||
),
|
||||
[currentPageId],
|
||||
);
|
||||
const treePath = useAtomValue(treePathAtom);
|
||||
|
||||
useEffect(() => {
|
||||
if (!currentPage) return;
|
||||
|
||||
// Selection/mapping + stale-clearing live in a pure, unit-tested helper
|
||||
// (#218). It resolves the correct chain when possible and, on a transient
|
||||
// miss, clears a chain left over from a previously-viewed page instead of
|
||||
// showing the wrong trail — while keeping a chain already resolved for THIS
|
||||
// page to avoid a blank flash.
|
||||
// (#218). The tree-hit chain (treePath) always wins when present; otherwise
|
||||
// fall back to the page's own ancestors and the stale-clearing logic — this
|
||||
// reproduces computeBreadcrumbState(fullTree, ancestors, …) exactly, since
|
||||
// its tree-hit branch is precisely findBreadcrumbPath(fullTree, pageId).
|
||||
setBreadcrumbNodes((previous) =>
|
||||
treePath ??
|
||||
computeBreadcrumbState(
|
||||
treeData,
|
||||
null,
|
||||
ancestors as IPage[] | undefined,
|
||||
currentPage.id,
|
||||
previous,
|
||||
),
|
||||
);
|
||||
}, [currentPage?.id, treeData, ancestors]);
|
||||
}, [currentPage?.id, treePath, ancestors]);
|
||||
|
||||
const HiddenNodesTooltipContent = () =>
|
||||
breadcrumbNodes?.slice(1, -1).map((node) => (
|
||||
|
||||
@@ -24,7 +24,7 @@ import { historyAtoms } from "@/features/page-history/atoms/history-atoms.ts";
|
||||
import { useDisclosure, useHotkeys } from "@mantine/hooks";
|
||||
import { useClipboard } from "@/hooks/use-clipboard";
|
||||
import { useParams } from "react-router-dom";
|
||||
import { usePageQuery } from "@/features/page/queries/page-query.ts";
|
||||
import { usePageMetaQuery } from "@/features/page/queries/page-query.ts";
|
||||
import {
|
||||
useToggleTemporaryMutation,
|
||||
syncTemporaryExpiresInCache,
|
||||
@@ -67,7 +67,7 @@ export default function PageHeaderMenu({ readOnly }: PageHeaderMenuProps) {
|
||||
const commentsTriggerProps = useAsideTriggerProps("comments");
|
||||
const tocTriggerProps = useAsideTriggerProps("toc");
|
||||
const { pageSlug } = useParams();
|
||||
const { data: page } = usePageQuery({
|
||||
const { data: page } = usePageMetaQuery({
|
||||
pageId: extractPageSlugId(pageSlug),
|
||||
});
|
||||
const isDeleted = !!page?.deletedAt;
|
||||
@@ -146,7 +146,7 @@ function PageActionMenu({ readOnly }: PageActionMenuProps) {
|
||||
const [, setHistoryModalOpen] = useAtom(historyAtoms);
|
||||
const clipboard = useClipboard({ timeout: 500 });
|
||||
const { pageSlug, spaceSlug } = useParams();
|
||||
const { data: page, isLoading } = usePageQuery({
|
||||
const { data: page, isLoading } = usePageMetaQuery({
|
||||
pageId: extractPageSlugId(pageSlug),
|
||||
});
|
||||
const { handleDelete } = useTreeMutation(page?.spaceId ?? "");
|
||||
|
||||
@@ -10,7 +10,7 @@ import { IconClockHour4, IconTrash } from "@tabler/icons-react";
|
||||
import { useState } from "react";
|
||||
import { Trans, useTranslation } from "react-i18next";
|
||||
import { useTimeAgo } from "@/hooks/use-time-ago.tsx";
|
||||
import { usePageQuery } from "@/features/page/queries/page-query.ts";
|
||||
import { usePageMetaQuery } from "@/features/page/queries/page-query.ts";
|
||||
import { useTreeMutation } from "@/features/page/tree/hooks/use-tree-mutation.ts";
|
||||
import {
|
||||
useToggleTemporaryMutation,
|
||||
@@ -35,7 +35,7 @@ type TemporaryNoteBannerProps = {
|
||||
*/
|
||||
export function TemporaryNoteBanner({ slugId }: TemporaryNoteBannerProps) {
|
||||
const { t } = useTranslation();
|
||||
const { data: page } = usePageQuery({ pageId: slugId });
|
||||
const { data: page } = usePageMetaQuery({ pageId: slugId });
|
||||
const { data: space } = useGetSpaceBySlugQuery(page?.space?.slug);
|
||||
const spaceAbility = useSpaceAbility(space?.membership?.permissions);
|
||||
const expiresTimeAgo = useTimeAgo(page?.temporaryExpiresAt);
|
||||
|
||||
@@ -0,0 +1,90 @@
|
||||
import { describe, it, expect, beforeEach, vi } from "vitest";
|
||||
import type { IPage } from "@/features/page/types/page.types";
|
||||
|
||||
// A fresh QueryClient stands in for the app singleton (importing the real
|
||||
// @/main.tsx would run ReactDOM.createRoot, which has no DOM root in jsdom). The
|
||||
// factory constructs it (QueryClient can't be referenced in vi.hoisted — that
|
||||
// runs before imports resolve); we import the SAME mocked instance back to seed
|
||||
// and assert on it.
|
||||
vi.mock("@/main.tsx", async () => {
|
||||
const { QueryClient } = await import("@tanstack/react-query");
|
||||
return { queryClient: new QueryClient() };
|
||||
});
|
||||
|
||||
import { queryClient as h_qc } from "@/main.tsx";
|
||||
import { invalidateOnUpdatePage } from "./page-query";
|
||||
|
||||
const h = { qc: h_qc };
|
||||
|
||||
// invalidateOnUpdatePage is the field-only (title/icon) tree path: instead of a
|
||||
// blanket invalidate it patches the affected node IN PLACE in every cached embed
|
||||
// subtree. The undefined-guard is LOAD-BEARING: a title-only socket event carries
|
||||
// icon:undefined, and without the guard `{...p, icon: undefined}` would WIPE the
|
||||
// icon in every cached subtree.
|
||||
const page = (over: Partial<IPage>): IPage =>
|
||||
({ id: "p1", title: "Old", icon: "📄", spaceId: "s1" }) as IPage &
|
||||
typeof over as IPage;
|
||||
|
||||
describe("invalidateOnUpdatePage — pointwise embed-cache patch", () => {
|
||||
beforeEach(() => {
|
||||
h.qc.clear();
|
||||
});
|
||||
|
||||
it("title-only event updates title but PRESERVES the icon (undefined-guard)", () => {
|
||||
const key = ["page-tree", "parent-1"];
|
||||
h.qc.setQueryData<IPage[]>(key, [
|
||||
{ id: "p1", title: "Old", icon: "📄", spaceId: "s1" } as IPage,
|
||||
{ id: "p2", title: "Other", icon: "📁", spaceId: "s1" } as IPage,
|
||||
]);
|
||||
|
||||
// icon passed as undefined (a title-only update)
|
||||
invalidateOnUpdatePage(
|
||||
"s1",
|
||||
"parent-1",
|
||||
"p1",
|
||||
"New Title",
|
||||
undefined as unknown as string,
|
||||
);
|
||||
|
||||
const patched = h.qc.getQueryData<IPage[]>(key)!;
|
||||
const p1 = patched.find((p) => p.id === "p1")!;
|
||||
const p2 = patched.find((p) => p.id === "p2")!;
|
||||
expect(p1.title).toBe("New Title");
|
||||
expect(p1.icon).toBe("📄"); // preserved, not wiped
|
||||
// Sibling node untouched.
|
||||
expect(p2.title).toBe("Other");
|
||||
expect(p2.icon).toBe("📁");
|
||||
});
|
||||
|
||||
it("icon-only event updates icon but preserves the title", () => {
|
||||
const key = ["page-tree", "parent-1"];
|
||||
h.qc.setQueryData<IPage[]>(key, [
|
||||
{ id: "p1", title: "Keep", icon: "📄", spaceId: "s1" } as IPage,
|
||||
]);
|
||||
|
||||
invalidateOnUpdatePage(
|
||||
"s1",
|
||||
"parent-1",
|
||||
"p1",
|
||||
undefined as unknown as string,
|
||||
"🚀",
|
||||
);
|
||||
|
||||
const p1 = h.qc.getQueryData<IPage[]>(key)!.find((p) => p.id === "p1")!;
|
||||
expect(p1.icon).toBe("🚀");
|
||||
expect(p1.title).toBe("Keep");
|
||||
});
|
||||
|
||||
it("does not touch a subtree that lacks the updated node", () => {
|
||||
const otherKey = ["page-tree", "unrelated"];
|
||||
const before = [
|
||||
{ id: "x1", title: "X", icon: "❌", spaceId: "s1" } as IPage,
|
||||
];
|
||||
h.qc.setQueryData<IPage[]>(otherKey, before);
|
||||
|
||||
invalidateOnUpdatePage("s1", "parent-1", "p1", "New", "🚀");
|
||||
|
||||
// Same reference back — the subtree without p1 is left as-is.
|
||||
expect(h.qc.getQueryData<IPage[]>(otherKey)).toBe(before);
|
||||
});
|
||||
});
|
||||
@@ -51,6 +51,10 @@ export function usePageQuery(
|
||||
queryFn: () => getPageById(pageInput),
|
||||
enabled: !!pageInput.pageId,
|
||||
staleTime: 5 * 60 * 1000,
|
||||
// Keep the previously-loaded page visible while navigating to a new one
|
||||
// instead of flashing a blank/skeleton frame (the new page's content
|
||||
// streams in when ready). isLoading stays true only for the very first load.
|
||||
placeholderData: keepPreviousData,
|
||||
});
|
||||
|
||||
useEffect(() => {
|
||||
@@ -66,6 +70,61 @@ export function usePageQuery(
|
||||
return query;
|
||||
}
|
||||
|
||||
/**
|
||||
* A page view that omits the large, frequently-changing `content` field. Every
|
||||
* other field is preserved, so consumers that read only metadata (title, icon,
|
||||
* permissions, id, creator, timestamps, …) keep working unchanged.
|
||||
*/
|
||||
export type IPageMeta = Omit<IPage, "content">;
|
||||
|
||||
function selectPageMeta(page: IPage): IPageMeta {
|
||||
// Drop `content`; react-query's structural sharing (replaceEqualDeep) then
|
||||
// returns the SAME reference whenever the remaining fields are unchanged, so a
|
||||
// pure content churn (typing / debouncedUpdateContent, collab `page.updated`)
|
||||
// no longer changes this slice's identity and its ~13 subscribers don't
|
||||
// re-render on every keystroke wave.
|
||||
const { content: _content, ...meta } = page;
|
||||
return meta as IPageMeta;
|
||||
}
|
||||
|
||||
/**
|
||||
* Metadata-only variant of {@link usePageQuery}. Shares the SAME query cache
|
||||
* entry (`["pages", pageId]`, full object incl. content), but this hook returns
|
||||
* a stable content-less slice so peripheral subscribers stop re-rendering on
|
||||
* every content update. Use it anywhere the full `content` is not read.
|
||||
*/
|
||||
export function usePageMetaQuery(
|
||||
pageInput: Partial<IPageInput>,
|
||||
): UseQueryResult<IPageMeta, Error> {
|
||||
const query = useQuery({
|
||||
queryKey: ["pages", pageInput.pageId],
|
||||
queryFn: () => getPageById(pageInput),
|
||||
enabled: !!pageInput.pageId,
|
||||
staleTime: 5 * 60 * 1000,
|
||||
select: selectPageMeta,
|
||||
// Match usePageQuery: keep the previous page's metadata visible while
|
||||
// navigating so the periphery (header, breadcrumb, …) doesn't flash blank.
|
||||
placeholderData: keepPreviousData,
|
||||
});
|
||||
|
||||
// Mirror usePageQuery's cross-key alias write so a page fetched by one
|
||||
// identifier is also cached under the other. The cache stores the FULL page
|
||||
// (select only narrows what THIS hook returns), so read the full object back
|
||||
// from the cache and alias THAT — never the content-less slice.
|
||||
useEffect(() => {
|
||||
if (!query.data) return;
|
||||
const full = queryClient.getQueryData<IPage>(["pages", pageInput.pageId]);
|
||||
if (!full) return;
|
||||
if (isValidUuid(pageInput.pageId)) {
|
||||
queryClient.setQueryData(["pages", full.slugId], full);
|
||||
} else {
|
||||
queryClient.setQueryData(["pages", full.id], full);
|
||||
}
|
||||
}, [query.data]);
|
||||
|
||||
return query;
|
||||
}
|
||||
|
||||
export function useCreatePageMutation() {
|
||||
const { t } = useTranslation();
|
||||
return useMutation<IPage, Error, Partial<IPageInput>>({
|
||||
@@ -351,6 +410,12 @@ export function useRecentChangesQuery(spaceId?: string) {
|
||||
initialPageParam: undefined as string | undefined,
|
||||
getNextPageParam: (lastPage) =>
|
||||
lastPage.meta.hasNextPage ? lastPage.meta.nextCursor : undefined,
|
||||
// KEEP refetchOnMount:true (against the global default false): recent-changes
|
||||
// IS invalidated on page create/update/move/delete, but invalidateQueries only
|
||||
// marks an UNMOUNTED query stale — it doesn't refetch it. The widget isn't
|
||||
// always mounted, so an event that lands while it's unmounted leaves it stale,
|
||||
// and the global refetchOnMount:false would not re-fetch on remount. The mount
|
||||
// refetch closes that gap.
|
||||
refetchOnMount: true,
|
||||
});
|
||||
}
|
||||
@@ -367,6 +432,9 @@ export function useCreatedByQuery(params?: {
|
||||
initialPageParam: undefined as string | undefined,
|
||||
getNextPageParam: (lastPage) =>
|
||||
lastPage.meta.hasNextPage ? lastPage.meta.nextCursor : undefined,
|
||||
// KEEP refetchOnMount:true: the "created-by" key is never invalidated (no
|
||||
// socket/mutation path), so the mount refetch is its ONLY freshness mechanism
|
||||
// — without it the list shows stale cache on navigation.
|
||||
refetchOnMount: true,
|
||||
});
|
||||
}
|
||||
@@ -380,8 +448,14 @@ export function useDeletedPagesQuery(
|
||||
queryFn: () => getDeletedPages(spaceId, params),
|
||||
enabled: !!spaceId,
|
||||
placeholderData: keepPreviousData,
|
||||
refetchOnMount: true,
|
||||
staleTime: 0,
|
||||
// KEEP refetchOnMount:true: ["trash-list"] IS invalidated by the
|
||||
// move-to-trash / delete / restore mutations, but invalidateQueries only marks
|
||||
// an unmounted query stale — it doesn't refetch it. The trash panel isn't
|
||||
// usually mounted when a page is trashed, so on opening it the global
|
||||
// refetchOnMount:false would show a stale list; the mount refetch closes that.
|
||||
// (Do NOT remove the three trash-list invalidations — they are not dead code.)
|
||||
refetchOnMount: true,
|
||||
});
|
||||
}
|
||||
|
||||
@@ -516,7 +590,35 @@ export function invalidateOnUpdatePage(
|
||||
title: string,
|
||||
icon: string,
|
||||
) {
|
||||
invalidatePageTree();
|
||||
// Scoped page-tree refresh (was a blanket `invalidatePageTree()`): this is the
|
||||
// FIELD-only update path (title/icon — no structural change), and the sidebar
|
||||
// tree is already updated pointwise (applyUpdateOne / optimistic setData) plus
|
||||
// via the sidebar-pages cache below. Invalidating ALL ["page-tree"] queries
|
||||
// here refetched every open recursive subpages-embed block on each
|
||||
// rename/icon-change — pure duplicate work. Instead patch just the affected
|
||||
// node IN PLACE in every cached embed subtree: same visible result, no network
|
||||
// churn, no full embed-tree rebuild. Structural events (create/move/delete)
|
||||
// keep the blanket invalidate in their own helpers.
|
||||
const pageTreeMatches = queryClient.getQueriesData<IPage[]>({
|
||||
queryKey: ["page-tree"],
|
||||
});
|
||||
pageTreeMatches.forEach(([key, items]) => {
|
||||
if (!items || !items.some((p) => p.id === id)) return;
|
||||
queryClient.setQueryData<IPage[]>(key, (old) =>
|
||||
old?.map((p) =>
|
||||
p.id === id
|
||||
? {
|
||||
...p,
|
||||
// Guard undefined so a title-only event can't wipe the icon (and
|
||||
// vice versa) in the embed cache.
|
||||
...(title !== undefined ? { title } : {}),
|
||||
...(icon !== undefined ? { icon } : {}),
|
||||
}
|
||||
: p,
|
||||
),
|
||||
);
|
||||
});
|
||||
|
||||
let queryKey: QueryKey = null;
|
||||
if (parentPageId === null) {
|
||||
queryKey = ["root-sidebar-pages", spaceId];
|
||||
|
||||
@@ -7,7 +7,7 @@ import { useRestorePageModal } from "@/features/page/hooks/use-restore-page-moda
|
||||
import { useDeletePageModal } from "@/features/page/hooks/use-delete-page-modal.tsx";
|
||||
import {
|
||||
useDeletePageMutation,
|
||||
usePageQuery,
|
||||
usePageMetaQuery,
|
||||
useRestorePageMutation,
|
||||
} from "@/features/page/queries/page-query.ts";
|
||||
import { getSpaceUrl } from "@/lib/config.ts";
|
||||
@@ -25,7 +25,7 @@ type DeletedPageBannerProps = {
|
||||
export function DeletedPageBanner({ slugId }: DeletedPageBannerProps) {
|
||||
const { t } = useTranslation();
|
||||
const navigate = useNavigate();
|
||||
const { data: page } = usePageQuery({ pageId: slugId });
|
||||
const { data: page } = usePageMetaQuery({ pageId: slugId });
|
||||
const { data: space } = useGetSpaceBySlugQuery(page?.space?.slug);
|
||||
const spaceAbility = useSpaceAbility(space?.membership?.permissions);
|
||||
const deletedTimeAgo = useTimeAgo(page?.deletedAt);
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { useAtom } from "jotai";
|
||||
import { useSetAtom, useStore } from "jotai";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import { useParams } from "react-router-dom";
|
||||
import { ActionIcon, Menu, rem } from "@mantine/core";
|
||||
@@ -52,7 +52,11 @@ export function NodeMenu({ node, canEdit }: NodeMenuProps) {
|
||||
const clipboard = useClipboard({ timeout: 500 });
|
||||
const { spaceSlug } = useParams();
|
||||
const { handleDelete } = useTreeMutation(node.spaceId);
|
||||
const [data, setData] = useAtom(treeDataAtom);
|
||||
// Setter-only: the tree value is read only imperatively inside the duplicate
|
||||
// handler (via `store` below), never at render, so useSetAtom avoids
|
||||
// re-rendering every row's NodeMenu on any tree event.
|
||||
const setData = useSetAtom(treeDataAtom);
|
||||
const store = useStore();
|
||||
const emit = useQueryEmit();
|
||||
const [exportOpened, { open: openExportModal, close: closeExportModal }] =
|
||||
useDisclosure(false);
|
||||
@@ -125,8 +129,8 @@ export function NodeMenu({ node, canEdit }: NodeMenuProps) {
|
||||
try {
|
||||
const duplicatedPage = await duplicatePage({ pageId: node.id });
|
||||
|
||||
// figure out parent + insertion index
|
||||
const siblings = treeModel.siblingsOf(data, node.id);
|
||||
// figure out parent + insertion index (read the live tree imperatively)
|
||||
const siblings = treeModel.siblingsOf(store.get(treeDataAtom), node.id);
|
||||
const parentId = siblings?.parentId ?? null;
|
||||
const currentIndex = siblings?.index ?? 0;
|
||||
const newIndex = currentIndex + 1;
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { useRef } from "react";
|
||||
import { Link, useParams } from "react-router-dom";
|
||||
import { useAtom } from "jotai";
|
||||
import { useAtom, useSetAtom } from "jotai";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import { ActionIcon, rem, Tooltip } from "@mantine/core";
|
||||
import {
|
||||
@@ -51,7 +51,11 @@ export function SpaceTreeRow({
|
||||
const { t } = useTranslation();
|
||||
const { spaceSlug } = useParams();
|
||||
const updatePageMutation = useUpdatePageMutation();
|
||||
const [, setTreeData] = useAtom(treeDataAtom);
|
||||
// Setter-only: subscribing to the whole treeDataAtom (via useAtom) re-rendered
|
||||
// every virtualized row on any tree event, bypassing the DocTreeRow memo. This
|
||||
// row never reads the tree value, only writes it, so useSetAtom avoids the
|
||||
// value subscription.
|
||||
const setTreeData = useSetAtom(treeDataAtom);
|
||||
const emit = useQueryEmit();
|
||||
const timerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
|
||||
const [mobileSidebarOpened] = useAtom(mobileSidebarAtom);
|
||||
|
||||
@@ -35,6 +35,7 @@ vi.mock("@/features/page/queries/page-query.ts", () => ({
|
||||
isFetching: false,
|
||||
}),
|
||||
usePageQuery: () => ({ data: undefined }),
|
||||
usePageMetaQuery: () => ({ data: undefined }),
|
||||
fetchAllAncestorChildren: (...args: unknown[]) =>
|
||||
fetchAllAncestorChildrenMock(...args),
|
||||
}));
|
||||
|
||||
@@ -26,6 +26,7 @@ vi.mock("@/features/page/queries/page-query.ts", () => ({
|
||||
isFetching: false,
|
||||
}),
|
||||
usePageQuery: () => ({ data: undefined }),
|
||||
usePageMetaQuery: () => ({ data: undefined }),
|
||||
fetchAllAncestorChildren: vi.fn(),
|
||||
}));
|
||||
|
||||
|
||||
@@ -15,7 +15,7 @@ import { notifications } from "@mantine/notifications";
|
||||
import {
|
||||
fetchAllAncestorChildren,
|
||||
useGetRootSidebarPagesQuery,
|
||||
usePageQuery,
|
||||
usePageMetaQuery,
|
||||
} from "@/features/page/queries/page-query.ts";
|
||||
import classes from "@/features/page/tree/styles/tree.module.css";
|
||||
import { treeDataAtom } from "@/features/page/tree/atoms/tree-data-atom.ts";
|
||||
@@ -76,7 +76,7 @@ const SpaceTree = forwardRef<SpaceTreeApi, SpaceTreeProps>(function SpaceTree(
|
||||
const [isDataLoaded, setIsDataLoaded] = useState(false);
|
||||
const spaceIdRef = useRef(spaceId);
|
||||
spaceIdRef.current = spaceId;
|
||||
const { data: currentPage } = usePageQuery({
|
||||
const { data: currentPage } = usePageMetaQuery({
|
||||
pageId: extractPageSlugId(pageSlug),
|
||||
});
|
||||
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { useCallback } from "react";
|
||||
import { useAtom, useSetAtom, useStore } from "jotai";
|
||||
import { useSetAtom, useStore } from "jotai";
|
||||
import { notifications } from "@mantine/notifications";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import { useNavigate, useParams } from "react-router-dom";
|
||||
@@ -34,7 +34,10 @@ export type UseTreeMutation = {
|
||||
|
||||
export function useTreeMutation(spaceId: string): UseTreeMutation {
|
||||
const { t } = useTranslation();
|
||||
const [, setData] = useAtom(treeDataAtom);
|
||||
// Setter-only: this hook never reads the tree reactively (handlers read the
|
||||
// live value imperatively via `store` below), so useSetAtom avoids
|
||||
// re-rendering SpaceSidebar on every tree event.
|
||||
const setData = useSetAtom(treeDataAtom);
|
||||
// `store` reads the *current* treeDataAtom imperatively in handlers — avoids
|
||||
// stale-closure issues when the caller updates the tree (e.g. lazy-load
|
||||
// children) and then immediately invokes a handler.
|
||||
|
||||
@@ -28,6 +28,7 @@ vi.mock("@/features/share/queries/share-query.ts", () => ({
|
||||
|
||||
vi.mock("@/features/page/queries/page-query.ts", () => ({
|
||||
usePageQuery: () => ({ data: { id: "page-1", title: "Doc" } }),
|
||||
usePageMetaQuery: () => ({ data: { id: "page-1", title: "Doc" } }),
|
||||
}));
|
||||
|
||||
vi.mock("@/features/space/queries/space-query.ts", () => ({
|
||||
|
||||
@@ -20,7 +20,7 @@ import {
|
||||
import { Link, useParams } from "react-router-dom";
|
||||
import { extractPageSlugId, getPageIcon } from "@/lib";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import { usePageQuery } from "@/features/page/queries/page-query.ts";
|
||||
import { usePageMetaQuery } from "@/features/page/queries/page-query.ts";
|
||||
import CopyTextButton from "@/components/common/copy.tsx";
|
||||
import { getAppUrl } from "@/lib/config.ts";
|
||||
import { buildPageUrl } from "@/features/page/page.utils.ts";
|
||||
@@ -37,7 +37,7 @@ export default function ShareModal({ readOnly }: ShareModalProps) {
|
||||
const { t } = useTranslation();
|
||||
const { pageSlug } = useParams();
|
||||
const pageSlugId = extractPageSlugId(pageSlug);
|
||||
const { data: page } = usePageQuery({ pageId: pageSlugId });
|
||||
const { data: page } = usePageMetaQuery({ pageId: pageSlugId });
|
||||
const pageId = page?.id;
|
||||
const { data: share } = useShareForPageQuery(pageId);
|
||||
const { spaceSlug } = useParams();
|
||||
|
||||
@@ -38,6 +38,11 @@ export function useGetSpacesQuery(
|
||||
queryKey: ["spaces", params],
|
||||
queryFn: () => getSpaces(params),
|
||||
placeholderData: keepPreviousData,
|
||||
// KEEP refetchOnMount:true (against the global default false): the ["spaces"]
|
||||
// key is invalidated only by same-tab mutations (no socket path), so a
|
||||
// cross-actor change — an admin adding/removing THIS user from a space — has
|
||||
// no local mutation or socket event and would leave the space list stale until
|
||||
// a hard reload. The mount refetch is its only cross-actor freshness path.
|
||||
refetchOnMount: true,
|
||||
});
|
||||
}
|
||||
|
||||
@@ -16,7 +16,6 @@ export function useWatchedSpaceIds(): Set<string> {
|
||||
const { data } = useQuery({
|
||||
queryKey: [WATCHED_SPACE_IDS_KEY],
|
||||
queryFn: () => getWatchedSpaceIds(),
|
||||
refetchOnMount: true,
|
||||
});
|
||||
|
||||
const items = data?.items;
|
||||
|
||||
@@ -19,7 +19,11 @@ export const useQuerySubscription = () => {
|
||||
const [socket] = useAtom(socketAtom);
|
||||
|
||||
React.useEffect(() => {
|
||||
socket?.on("message", (event) => {
|
||||
if (!socket) return;
|
||||
// Named handler + off() cleanup (mirrors use-notification-socket). Without
|
||||
// cleanup, every socket recreation / effect re-run stacked another listener,
|
||||
// so a single broadcast fired duplicated invalidateQueries / setQueryData.
|
||||
const handleMessage = (event) => {
|
||||
const data: WebSocketEvent = event;
|
||||
|
||||
let entity = null;
|
||||
@@ -163,6 +167,11 @@ export const useQuerySubscription = () => {
|
||||
});
|
||||
break;
|
||||
}
|
||||
});
|
||||
};
|
||||
|
||||
socket.on("message", handleMessage);
|
||||
return () => {
|
||||
socket.off("message", handleMessage);
|
||||
};
|
||||
}, [queryClient, socket]);
|
||||
};
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { useEffect } from "react";
|
||||
import { socketAtom } from "@/features/websocket/atoms/socket-atom.ts";
|
||||
import { useAtom } from "jotai";
|
||||
import { useAtom, useSetAtom } from "jotai";
|
||||
import { treeDataAtom } from "@/features/page/tree/atoms/tree-data-atom.ts";
|
||||
import { WebSocketEvent } from "@/features/websocket/types";
|
||||
import { SpaceTreeNode } from "@/features/page/tree/types.ts";
|
||||
@@ -16,7 +16,10 @@ import localEmitter from "@/lib/local-emitter.ts";
|
||||
|
||||
export const useTreeSocket = () => {
|
||||
const [socket] = useAtom(socketAtom);
|
||||
const [, setTreeData] = useAtom(treeDataAtom);
|
||||
// Setter-only: this hook writes the tree from socket events but never reads it
|
||||
// reactively, so useSetAtom avoids re-rendering UserProvider (its host) on
|
||||
// every tree event.
|
||||
const setTreeData = useSetAtom(treeDataAtom);
|
||||
const queryClient = useQueryClient();
|
||||
|
||||
useEffect(() => {
|
||||
@@ -37,7 +40,11 @@ export const useTreeSocket = () => {
|
||||
}, []);
|
||||
|
||||
useEffect(() => {
|
||||
socket?.on("message", (event: WebSocketEvent) => {
|
||||
if (!socket) return;
|
||||
// Named handler + off() cleanup (mirrors use-notification-socket). Without
|
||||
// cleanup, every socket recreation / effect re-run stacked another listener,
|
||||
// so a single broadcast fired duplicated tree walks after each reconnect.
|
||||
const handleMessage = (event: WebSocketEvent) => {
|
||||
switch (event.operation) {
|
||||
case "updateOne":
|
||||
if (event.entity[0] === "pages") {
|
||||
@@ -64,6 +71,11 @@ export const useTreeSocket = () => {
|
||||
});
|
||||
break;
|
||||
}
|
||||
});
|
||||
}, [socket]);
|
||||
};
|
||||
|
||||
socket.on("message", handleMessage);
|
||||
return () => {
|
||||
socket.off("message", handleMessage);
|
||||
};
|
||||
}, [socket, queryClient, setTreeData]);
|
||||
};
|
||||
|
||||
@@ -243,6 +243,5 @@ export function useAppVersion(
|
||||
queryFn: () => getAppVersion(),
|
||||
staleTime: 60 * 60 * 1000, // 1 hr
|
||||
enabled: isEnabled,
|
||||
refetchOnMount: true,
|
||||
});
|
||||
}
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { useNavigate, useParams } from "react-router-dom";
|
||||
import { useEffect } from "react";
|
||||
import { usePageQuery } from "@/features/page/queries/page-query";
|
||||
import { usePageMetaQuery } from "@/features/page/queries/page-query";
|
||||
import { buildPageUrl } from "@/features/page/page.utils.ts";
|
||||
import { extractPageSlugId } from "@/lib";
|
||||
import { Error404 } from "@/components/ui/error-404.tsx";
|
||||
@@ -11,7 +11,7 @@ export default function PageRedirect() {
|
||||
data: page,
|
||||
isLoading: pageIsLoading,
|
||||
isError,
|
||||
} = usePageQuery({ pageId: extractPageSlugId(pageSlug) });
|
||||
} = usePageMetaQuery({ pageId: extractPageSlugId(pageSlug) });
|
||||
const navigate = useNavigate();
|
||||
|
||||
useEffect(() => {
|
||||
|
||||
@@ -10,7 +10,7 @@ import { useTranslation } from "react-i18next";
|
||||
import React from "react";
|
||||
import { EmptyState } from "@/components/ui/empty-state.tsx";
|
||||
import { IconAlertTriangle, IconFileOff } from "@tabler/icons-react";
|
||||
import { Button } from "@mantine/core";
|
||||
import { Button, Skeleton } from "@mantine/core";
|
||||
import { Link } from "react-router-dom";
|
||||
import { ErrorBoundary } from "react-error-boundary";
|
||||
const MemoizedFullEditor = React.memo(FullEditor);
|
||||
@@ -58,7 +58,7 @@ function PageContent({ pageSlug }: { pageSlug: string | undefined }) {
|
||||
(space?.settings?.comments?.allowViewerComments === true);
|
||||
|
||||
if (isLoading) {
|
||||
return <></>;
|
||||
return <PageSkeleton />;
|
||||
}
|
||||
|
||||
if (isError || !page) {
|
||||
@@ -87,7 +87,7 @@ function PageContent({ pageSlug }: { pageSlug: string | undefined }) {
|
||||
}
|
||||
|
||||
if (!space) {
|
||||
return <></>;
|
||||
return <PageSkeleton />;
|
||||
}
|
||||
|
||||
return (
|
||||
@@ -116,3 +116,18 @@ function PageContent({ pageSlug }: { pageSlug: string | undefined }) {
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
// Lightweight loading placeholder shown instead of a blank fragment while the
|
||||
// page (or its space) is loading, so navigation into a not-yet-cached page no
|
||||
// longer flashes empty. Approximates the title + first content lines.
|
||||
function PageSkeleton() {
|
||||
return (
|
||||
<div>
|
||||
<Skeleton height={34} width="45%" mt="xl" radius="sm" />
|
||||
<Skeleton height={16} mt="xl" radius="sm" />
|
||||
<Skeleton height={16} mt="sm" radius="sm" />
|
||||
<Skeleton height={16} mt="sm" width="85%" radius="sm" />
|
||||
<Skeleton height={16} mt="sm" width="70%" radius="sm" />
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -450,7 +450,7 @@ async function main() {
|
||||
// 8. get_page markdown round-trip sanity (table separator present)
|
||||
const md = await client.getPage(pageId);
|
||||
check("get_page md: table separator emitted", md.data.content.includes("| --- |"), "");
|
||||
check("get_page md: callout exported as :::", md.data.content.includes(":::info"));
|
||||
check("get_page md: callout exported as Obsidian '> [!info]'", md.data.content.includes("> [!info]"));
|
||||
|
||||
// 9. comments: create / list / reply / update / check_new / delete
|
||||
const beforeComments = new Date(Date.now() - 1000).toISOString();
|
||||
|
||||
@@ -635,13 +635,17 @@ const Attachment = Node.create({
|
||||
},
|
||||
name: {
|
||||
default: null,
|
||||
parseHTML: (el: HTMLElement) => el.getAttribute("data-attachment-name"),
|
||||
// Empty-string-vs-absent idempotency (GS-EDIT-REVERT class): "" -> default.
|
||||
parseHTML: (el: HTMLElement) =>
|
||||
el.getAttribute("data-attachment-name") || null,
|
||||
renderHTML: (attrs: Record<string, any>) =>
|
||||
attrs.name ? { "data-attachment-name": attrs.name } : {},
|
||||
},
|
||||
mime: {
|
||||
default: null,
|
||||
parseHTML: (el: HTMLElement) => el.getAttribute("data-attachment-mime"),
|
||||
// Empty-string-vs-absent idempotency (GS-EDIT-REVERT class): "" -> default.
|
||||
parseHTML: (el: HTMLElement) =>
|
||||
el.getAttribute("data-attachment-mime") || null,
|
||||
renderHTML: (attrs: Record<string, any>) =>
|
||||
attrs.mime ? { "data-attachment-mime": attrs.mime } : {},
|
||||
},
|
||||
@@ -689,7 +693,10 @@ const Video = Node.create({
|
||||
},
|
||||
alt: {
|
||||
default: null,
|
||||
parseHTML: (el: HTMLElement) => el.getAttribute("aria-label"),
|
||||
// Empty-string-vs-absent idempotency: coerce "" back to the default so a
|
||||
// stray empty `aria-label` never materializes `alt: ""` on a video stored
|
||||
// with no alt (same GS-EDIT-REVERT class as the image `alt` fix).
|
||||
parseHTML: (el: HTMLElement) => el.getAttribute("aria-label") || null,
|
||||
renderHTML: (attrs: Record<string, any>) =>
|
||||
attrs.alt ? { "aria-label": attrs.alt } : {},
|
||||
},
|
||||
@@ -864,13 +871,15 @@ const diagramAttributes = () => ({
|
||||
},
|
||||
title: {
|
||||
default: null,
|
||||
parseHTML: (el: HTMLElement) => el.getAttribute("data-title"),
|
||||
// Empty-string-vs-absent idempotency (GS-EDIT-REVERT class): "" -> default.
|
||||
parseHTML: (el: HTMLElement) => el.getAttribute("data-title") || null,
|
||||
renderHTML: (attrs: Record<string, any>) =>
|
||||
attrs.title ? { "data-title": attrs.title } : {},
|
||||
},
|
||||
alt: {
|
||||
default: null,
|
||||
parseHTML: (el: HTMLElement) => el.getAttribute("data-alt"),
|
||||
// Empty-string-vs-absent idempotency (GS-EDIT-REVERT class): "" -> default.
|
||||
parseHTML: (el: HTMLElement) => el.getAttribute("data-alt") || null,
|
||||
renderHTML: (attrs: Record<string, any>) =>
|
||||
attrs.alt ? { "data-alt": attrs.alt } : {},
|
||||
},
|
||||
@@ -1106,7 +1115,8 @@ const Pdf = Node.create({
|
||||
},
|
||||
name: {
|
||||
default: null,
|
||||
parseHTML: (el: HTMLElement) => el.getAttribute("data-name"),
|
||||
// Empty-string-vs-absent idempotency (GS-EDIT-REVERT class): "" -> default.
|
||||
parseHTML: (el: HTMLElement) => el.getAttribute("data-name") || null,
|
||||
renderHTML: (attrs: Record<string, any>) =>
|
||||
attrs.name ? { "data-name": attrs.name } : {},
|
||||
},
|
||||
@@ -1491,6 +1501,29 @@ export const docmostExtensions = [
|
||||
...parent.height,
|
||||
parseHTML: (el: HTMLElement) => el.getAttribute("height"),
|
||||
},
|
||||
// Empty-string-vs-absent idempotency (GS-EDIT-REVERT class). `marked`
|
||||
// renders `` as `<img alt="">`, so the stock Image `alt`
|
||||
// parseHTML (`getAttribute("alt")`) materializes `alt: ""` on an image
|
||||
// that was stored with NO alt (attr absent). That is a false diff against
|
||||
// the editor-stored form (a no-alt image has alt ABSENT, not ""), so a
|
||||
// git-sync / ai-chat touch of a page with a plain image produced phantom
|
||||
// churn. Coerce an empty string back to the attr's default (null) so the
|
||||
// import is idempotent. A real alt survives verbatim (`|| undefined` keeps
|
||||
// the truthy value; the default fills the empty case). `title` is coerced
|
||||
// the same way for the whole class, even though `marked` does not
|
||||
// currently emit `title=""` — defence in depth against any path that does.
|
||||
// NOTE: this DIVERGES from editor-ext's literal image `alt` parseHTML
|
||||
// (`getAttribute("alt")`, which returns "" verbatim), but CONVERGES on
|
||||
// editor-ext's real STORED shape: an editor image inserted without alt
|
||||
// renders with no `alt` attribute and re-parses as absent, never "".
|
||||
alt: {
|
||||
...parent.alt,
|
||||
parseHTML: (el: HTMLElement) => el.getAttribute("alt") || null,
|
||||
},
|
||||
title: {
|
||||
...parent.title,
|
||||
parseHTML: (el: HTMLElement) => el.getAttribute("title") || null,
|
||||
},
|
||||
};
|
||||
},
|
||||
}).configure({ inline: false }),
|
||||
|
||||
@@ -0,0 +1,443 @@
|
||||
/**
|
||||
* Reusable round-trip-STABILITY matrix helper (fixtures-first).
|
||||
*
|
||||
* A single stored node authored WITHOUT a given string attribute (attr
|
||||
* absent / undefined) must not gain a phantom EMPTY-STRING value after a
|
||||
* markdown round-trip — the "empty-string-vs-absent" churn class. This helper,
|
||||
* given a node spec, drives a matrix of attribute combinations through the REAL
|
||||
* converter (`convertProseMirrorToMarkdown` -> `markdownToProseMirror`) and
|
||||
* asserts byte-stability on two contours:
|
||||
*
|
||||
* 1. RAW round-trip: for the node under test, every attribute the round-trip
|
||||
* materializes must equal what the INPUT authored — an authored attr keeps
|
||||
* its value, an ABSENT attr may only reappear at its SCHEMA DEFAULT. If an
|
||||
* absent attr comes back as a NON-default value (e.g. `alt: ""` where the
|
||||
* default is `null`), that is an instability and is reported precisely as
|
||||
* `type.attr: absent -> "<got>"`. This is the contour git-sync / stored
|
||||
* JSON diffs on, so masking it only in `canonicalize` would leave the noise.
|
||||
*
|
||||
* 2. CANONICAL round-trip: `canonicalizeContent(original)` must deep-equal
|
||||
* `canonicalizeContent(roundtrip)` (a second, semantic contour).
|
||||
*
|
||||
* The ONLY normalization the helper treats as allowed (not an instability) is
|
||||
* the DOCUMENTED numeric width/height/size/aspectRatio -> string coercion the
|
||||
* converter performs on purpose (a stored numeric `640` re-parses via
|
||||
* `getAttribute` as the string `"640"`). It is encoded here as an explicit
|
||||
* per-spec `numericStringAttrs` set applied to BOTH contours, NOT a silent skip.
|
||||
*
|
||||
* The helper is node-type agnostic: image and the whole media family share the
|
||||
* `align !== "center"` predicate + `<!--name {…}-->` comment machinery, so one
|
||||
* matrix guards the shared class.
|
||||
*/
|
||||
import { getSchema } from "@tiptap/core";
|
||||
import {
|
||||
convertProseMirrorToMarkdown,
|
||||
markdownToProseMirror,
|
||||
canonicalizeContent,
|
||||
docmostExtensions,
|
||||
} from "../src/lib/index.js";
|
||||
import { firstDivergence } from "./roundtrip-helpers.js";
|
||||
|
||||
/** One attribute's two probe values. */
|
||||
export interface AttrMatrixEntry {
|
||||
/** Attribute name on the node. */
|
||||
attr: string;
|
||||
/**
|
||||
* The "default" pick. `undefined` means the attribute is OMITTED entirely
|
||||
* (the absent case — the one that can materialize an empty string on import).
|
||||
* A concrete value is authored verbatim.
|
||||
*/
|
||||
default: unknown;
|
||||
/** A representative NON-default value to exercise (must survive verbatim). */
|
||||
nonDefault: unknown;
|
||||
/**
|
||||
* Marks the attr as a member of the EMPTY-STRING class the fix targets: a
|
||||
* string attr whose schema default is `null`/absent and whose parseHTML
|
||||
* coerces `"" -> default` (image/drawio `alt`+`title`, video `alt` via
|
||||
* aria-label, pdf/attachment `name`, attachment `mime`). Set true to also
|
||||
* drive the THIRD-STATE convergence case (see runConvergenceCase) for this
|
||||
* attr. Attrs whose default is NOT null (e.g. embed `provider`, default "")
|
||||
* or that are not `""`-coerced (control attrs) are left unset.
|
||||
*/
|
||||
emptyStringClass?: boolean;
|
||||
}
|
||||
|
||||
/** A node type + the attribute matrix to sweep for it. */
|
||||
export interface NodeStabilitySpec {
|
||||
/** Node type (e.g. "image", "video"). */
|
||||
type: string;
|
||||
/** Attributes always present on the node (e.g. `{ src: "/i.png" }`). */
|
||||
baseAttrs?: Record<string, unknown>;
|
||||
/** Attributes to sweep at default and non-default. */
|
||||
attrMatrix: AttrMatrixEntry[];
|
||||
/**
|
||||
* Attributes whose numeric -> string coercion on round-trip is DOCUMENTED and
|
||||
* intentional; compared modulo `String(x)` on both sides. Defaults to the
|
||||
* converter's known sizing set.
|
||||
*/
|
||||
numericStringAttrs?: string[];
|
||||
}
|
||||
|
||||
/** A single unstable finding, legible enough to tie a gate-lock to. */
|
||||
export interface Instability {
|
||||
type: string;
|
||||
attr: string;
|
||||
/** What the input authored: the literal value, or the ABSENT sentinel. */
|
||||
authored: unknown | typeof ABSENT;
|
||||
/** What the round-trip produced. */
|
||||
got: unknown;
|
||||
/** What a stable round-trip should have produced (authored value or default). */
|
||||
expected: unknown;
|
||||
}
|
||||
|
||||
/** One matrix cell's result. */
|
||||
export interface ComboResult {
|
||||
label: string;
|
||||
authored: Record<string, unknown>;
|
||||
/** RAW-contour instabilities on the node under test. */
|
||||
raw: Instability[];
|
||||
/** CANONICAL-contour divergence (path + values) or null when equal. */
|
||||
canonical: { path: string; a: unknown; b: unknown } | null;
|
||||
/** True when the node type failed to round-trip at all (structural loss). */
|
||||
missing: boolean;
|
||||
md: string;
|
||||
}
|
||||
|
||||
/** Whole-matrix report for one node spec. */
|
||||
export interface MatrixReport {
|
||||
type: string;
|
||||
combos: ComboResult[];
|
||||
}
|
||||
|
||||
/** Sentinel marking an attribute the input did NOT author. */
|
||||
export const ABSENT = Symbol("ABSENT");
|
||||
|
||||
const DEFAULT_NUMERIC_STRING_ATTRS = [
|
||||
"width",
|
||||
"height",
|
||||
"size",
|
||||
"aspectRatio",
|
||||
];
|
||||
|
||||
// The ProseMirror schema the converter targets — its attribute `default`s are
|
||||
// the authoritative "what an absent attr should re-materialize as" oracle.
|
||||
const schema = getSchema(docmostExtensions);
|
||||
|
||||
/** Read the schema default for every attribute of a node type. */
|
||||
function schemaDefaults(type: string): Record<string, unknown> {
|
||||
const specAttrs = (schema.nodes[type]?.spec?.attrs ?? {}) as Record<
|
||||
string,
|
||||
{ default: unknown }
|
||||
>;
|
||||
const out: Record<string, unknown> = {};
|
||||
for (const [k, v] of Object.entries(specAttrs)) out[k] = v.default;
|
||||
return out;
|
||||
}
|
||||
|
||||
/** Find the first node of a given type anywhere in a PM doc tree. */
|
||||
function findFirst(node: any, type: string): any {
|
||||
if (node && node.type === type) return node;
|
||||
for (const child of node?.content ?? []) {
|
||||
const hit = findFirst(child, type);
|
||||
if (hit) return hit;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
/** Coerce a scalar for the documented numeric->string comparison. */
|
||||
const numStr = (x: unknown): unknown => (x == null ? x : String(x));
|
||||
|
||||
/**
|
||||
* Enumerate the cartesian product of the matrix: every attribute independently
|
||||
* at its default (index 0) or non-default (index 1) pick. The all-default
|
||||
* corner is included (the baseline). Small by construction (2^N over a handful
|
||||
* of at-risk string attrs).
|
||||
*/
|
||||
function enumerateCombos(matrix: AttrMatrixEntry[]): number[][] {
|
||||
let combos: number[][] = [[]];
|
||||
for (let i = 0; i < matrix.length; i++) {
|
||||
const next: number[][] = [];
|
||||
for (const c of combos) {
|
||||
next.push([...c, 0]);
|
||||
next.push([...c, 1]);
|
||||
}
|
||||
combos = next;
|
||||
}
|
||||
return combos;
|
||||
}
|
||||
|
||||
/** Build the authored attrs for one combo pick vector. */
|
||||
function authoredAttrs(
|
||||
spec: NodeStabilitySpec,
|
||||
picks: number[],
|
||||
): Record<string, unknown> {
|
||||
const attrs: Record<string, unknown> = { ...(spec.baseAttrs ?? {}) };
|
||||
spec.attrMatrix.forEach((entry, i) => {
|
||||
if (picks[i] === 1) {
|
||||
attrs[entry.attr] = entry.nonDefault;
|
||||
} else if (entry.default !== undefined) {
|
||||
attrs[entry.attr] = entry.default;
|
||||
}
|
||||
// default === undefined -> OMIT the attr entirely (the absent case).
|
||||
});
|
||||
return attrs;
|
||||
}
|
||||
|
||||
/** Human-readable label for a combo (which attrs are at non-default). */
|
||||
function comboLabel(spec: NodeStabilitySpec, picks: number[]): string {
|
||||
const on = spec.attrMatrix
|
||||
.filter((_, i) => picks[i] === 1)
|
||||
.map((e) => e.attr);
|
||||
return on.length === 0 ? "<all-default>" : on.join("+");
|
||||
}
|
||||
|
||||
/**
|
||||
* Run the full stability matrix for one node spec and return a structured
|
||||
* report (does NOT throw — the caller asserts, so a failure can print the whole
|
||||
* report). Every combo runs the real export->import pipeline once.
|
||||
*/
|
||||
export async function runStabilityMatrix(
|
||||
spec: NodeStabilitySpec,
|
||||
): Promise<MatrixReport> {
|
||||
const numericStringAttrs = new Set(
|
||||
spec.numericStringAttrs ?? DEFAULT_NUMERIC_STRING_ATTRS,
|
||||
);
|
||||
const defaults = schemaDefaults(spec.type);
|
||||
const combos: ComboResult[] = [];
|
||||
|
||||
for (const picks of enumerateCombos(spec.attrMatrix)) {
|
||||
const authored = authoredAttrs(spec, picks);
|
||||
const doc = { type: "doc", content: [{ type: spec.type, attrs: authored }] };
|
||||
const md = convertProseMirrorToMarkdown(doc);
|
||||
const rt = await markdownToProseMirror(md);
|
||||
const node = findFirst(rt, spec.type);
|
||||
|
||||
const result: ComboResult = {
|
||||
label: comboLabel(spec, picks),
|
||||
authored,
|
||||
raw: [],
|
||||
canonical: null,
|
||||
missing: node == null,
|
||||
md,
|
||||
};
|
||||
|
||||
if (node != null) {
|
||||
// RAW contour: every materialized attr must equal the authored value, or
|
||||
// (for an absent attr) the schema default — modulo the documented numeric
|
||||
// string coercion.
|
||||
const rtAttrs = (node.attrs ?? {}) as Record<string, unknown>;
|
||||
for (const key of Object.keys(rtAttrs)) {
|
||||
const authoredHas = Object.prototype.hasOwnProperty.call(authored, key);
|
||||
const expected = authoredHas ? authored[key] : defaults[key];
|
||||
let got = rtAttrs[key];
|
||||
let exp = expected;
|
||||
if (numericStringAttrs.has(key)) {
|
||||
got = numStr(got);
|
||||
exp = numStr(exp);
|
||||
}
|
||||
if (firstDivergence(got, exp) !== null) {
|
||||
result.raw.push({
|
||||
type: spec.type,
|
||||
attr: key,
|
||||
authored: authoredHas ? authored[key] : ABSENT,
|
||||
got: rtAttrs[key],
|
||||
expected,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
// CANONICAL contour: canonical forms deep-equal, modulo the same numeric
|
||||
// string coercion (applied to both trees so a documented coercion is not
|
||||
// counted as a divergence).
|
||||
const ca = normalizeNumeric(canonicalizeContent(doc), numericStringAttrs);
|
||||
const cb = normalizeNumeric(canonicalizeContent(rt), numericStringAttrs);
|
||||
result.canonical = firstDivergence(ca, cb);
|
||||
}
|
||||
|
||||
combos.push(result);
|
||||
}
|
||||
|
||||
return { type: spec.type, combos };
|
||||
}
|
||||
|
||||
/**
|
||||
* Deep-copy a canonical tree, coercing the documented numeric->string attrs to
|
||||
* their string form so an intentional `640 -> "640"` coercion is not reported
|
||||
* as a canonical divergence. Only touches the listed attribute keys.
|
||||
*/
|
||||
function normalizeNumeric(node: any, attrs: Set<string>): any {
|
||||
if (Array.isArray(node)) return node.map((n) => normalizeNumeric(n, attrs));
|
||||
if (node === null || typeof node !== "object") return node;
|
||||
const out: Record<string, unknown> = {};
|
||||
for (const key of Object.keys(node)) {
|
||||
if (key === "attrs" && node.attrs && typeof node.attrs === "object") {
|
||||
const a: Record<string, unknown> = {};
|
||||
for (const [k, v] of Object.entries(node.attrs)) {
|
||||
a[k] = attrs.has(k) ? numStr(v) : v;
|
||||
}
|
||||
out.attrs = a;
|
||||
} else {
|
||||
out[key] = normalizeNumeric(node[key], attrs);
|
||||
}
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
/** Flatten a report to just its unstable combos (for a terse assertion). */
|
||||
export function unstableCombos(report: MatrixReport): ComboResult[] {
|
||||
return report.combos.filter(
|
||||
(c) => c.missing || c.raw.length > 0 || c.canonical !== null,
|
||||
);
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// THIRD STATE: an EXPLICITLY-STORED empty string on a string attr.
|
||||
//
|
||||
// The matrix above sweeps TWO states per string attr: absent/default and a
|
||||
// non-default value — and asserts FIRST-pass byte-stability for both. There is
|
||||
// a third, degenerate state the matrix does NOT cover: the attr stored as a
|
||||
// LITERAL `""`. This is DISTINCT from "the node never had the attr": a user
|
||||
// types an alt in the editor, then deletes it, and Tiptap's
|
||||
// `updateAttributes({ alt: "" })` persists a literal `alt: ""` in the stored
|
||||
// JSON. There is no absent-vs-"" distinction in the DOM once serialized, so the
|
||||
// fix's `getAttribute("alt") || null` coercion canonicalizes BOTH to the
|
||||
// default (`null`).
|
||||
//
|
||||
// Consequence — and this is CORRECT, not a bug: a doc carrying an explicit `""`
|
||||
// converges to the default on the FIRST round-trip (a ONE-TIME diff: `"" ->
|
||||
// null`), then is byte-stable from the SECOND round-trip on (idempotent). So
|
||||
// this state must be pinned with a DIFFERENT contract than the matrix's:
|
||||
// - do NOT assert first-pass byte-stability (the first pass legitimately
|
||||
// changes `""` -> default), and
|
||||
// - DO assert the first pass converges to the default AND the second pass is
|
||||
// idempotent (rt2 deep-equals rt1).
|
||||
//
|
||||
// A future sync/QA pass diffing stored pages will see this one-time `"" -> null`
|
||||
// normalization exactly once per affected node; it is the converter canon, not
|
||||
// corruption, and must not be flagged as data loss.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/** Result of the third-state ("explicit empty string") convergence probe. */
|
||||
export interface ConvergenceResult {
|
||||
type: string;
|
||||
attr: string;
|
||||
/** The schema default the attr must converge to on pass 1 (null / absent). */
|
||||
expectedDefault: unknown;
|
||||
/** rt1's materialized value for the attr — must equal `expectedDefault`. */
|
||||
firstPassValue: unknown;
|
||||
/** True when the node round-tripped AND rt1 converged the attr to default. */
|
||||
convergedToDefault: boolean;
|
||||
/** rt1-vs-rt2 divergence; MUST be null (idempotent from pass 2 on). */
|
||||
secondPassDivergence: { path: string; a: unknown; b: unknown } | null;
|
||||
/** True when the node type failed to round-trip at all (structural loss). */
|
||||
missing: boolean;
|
||||
}
|
||||
|
||||
/** Round-trip a full PM doc through the real converter once. */
|
||||
async function roundtripDoc(doc: any): Promise<any> {
|
||||
return markdownToProseMirror(convertProseMirrorToMarkdown(doc));
|
||||
}
|
||||
|
||||
/**
|
||||
* Third-state convergence probe for one string attr of the empty-string class.
|
||||
*
|
||||
* (a) builds a doc with the attr EXPLICITLY set to `""` (baseAttrs + `""`),
|
||||
* (b) rt1 = roundtrip(doc); asserts rt1's attr equals the schema default — the
|
||||
* documented ONE-TIME `"" -> default` normalization (NOT byte-stable vs the
|
||||
* `""` input, so first-pass stability is deliberately NOT asserted here),
|
||||
* (c) rt2 = roundtrip(rt1); asserts rt2 deep-equals rt1 — idempotent from the
|
||||
* second round-trip on.
|
||||
*
|
||||
* Returns a structured result (does NOT throw) so the caller can assert and
|
||||
* print. Reusable across the whole node family: drive it for every attr flagged
|
||||
* `emptyStringClass` on every spec (see convergenceCasesFor / the test driver).
|
||||
*/
|
||||
export async function runConvergenceCase(
|
||||
spec: NodeStabilitySpec,
|
||||
attr: string,
|
||||
): Promise<ConvergenceResult> {
|
||||
const expectedDefault = schemaDefaults(spec.type)[attr];
|
||||
|
||||
// (a) The degenerate third state: attr persisted as a LITERAL "".
|
||||
const authored = { ...(spec.baseAttrs ?? {}), [attr]: "" };
|
||||
const doc = { type: "doc", content: [{ type: spec.type, attrs: authored }] };
|
||||
|
||||
// (b) First round-trip: "" must normalize to the default (a one-time diff).
|
||||
const rt1 = await roundtripDoc(doc);
|
||||
const node1 = findFirst(rt1, spec.type);
|
||||
const firstPassValue = node1?.attrs?.[attr];
|
||||
const convergedToDefault =
|
||||
node1 != null && firstDivergence(firstPassValue, expectedDefault) === null;
|
||||
|
||||
// (c) Second round-trip: must be byte-stable (rt2 deep-equals rt1). We compare
|
||||
// the WHOLE docs — both are converter OUTPUTS already in the same materialized
|
||||
// form (numeric attrs are strings on both sides), so no numeric normalization
|
||||
// is needed here, unlike the raw/canonical contours above.
|
||||
const rt2 = node1 != null ? await roundtripDoc(rt1) : rt1;
|
||||
const secondPassDivergence =
|
||||
node1 != null ? firstDivergence(rt1, rt2) : null;
|
||||
|
||||
return {
|
||||
type: spec.type,
|
||||
attr,
|
||||
expectedDefault,
|
||||
firstPassValue,
|
||||
convergedToDefault,
|
||||
secondPassDivergence,
|
||||
missing: node1 == null,
|
||||
};
|
||||
}
|
||||
|
||||
/** The attrs of a spec flagged as members of the empty-string class. */
|
||||
export function convergenceCasesFor(spec: NodeStabilitySpec): string[] {
|
||||
return spec.attrMatrix
|
||||
.filter((e) => e.emptyStringClass)
|
||||
.map((e) => e.attr);
|
||||
}
|
||||
|
||||
/** True when a convergence result honours the "converges once, then stable" contract. */
|
||||
export function convergenceOk(r: ConvergenceResult): boolean {
|
||||
return !r.missing && r.convergedToDefault && r.secondPassDivergence === null;
|
||||
}
|
||||
|
||||
/** Render a convergence result as a legible one-liner for a failed assertion. */
|
||||
export function formatConvergence(r: ConvergenceResult): string {
|
||||
if (r.missing) return `${r.type}.${r.attr}: DID-NOT-ROUND-TRIP`;
|
||||
const parts: string[] = [];
|
||||
if (!r.convergedToDefault) {
|
||||
parts.push(
|
||||
`pass1 did NOT converge: got ${JSON.stringify(r.firstPassValue)} (expected default ${JSON.stringify(r.expectedDefault)})`,
|
||||
);
|
||||
}
|
||||
if (r.secondPassDivergence) {
|
||||
parts.push(
|
||||
`pass2 NOT idempotent @ ${r.secondPassDivergence.path}: ${JSON.stringify(r.secondPassDivergence.a)} vs ${JSON.stringify(r.secondPassDivergence.b)}`,
|
||||
);
|
||||
}
|
||||
const status = parts.length === 0 ? "converges-once-then-stable" : parts.join("; ");
|
||||
return `${r.type}.${r.attr}: ${status}`;
|
||||
}
|
||||
|
||||
/** Render a report as a legible multi-line string for a failed assertion. */
|
||||
export function formatReport(report: MatrixReport): string {
|
||||
const lines: string[] = [`node "${report.type}":`];
|
||||
for (const c of report.combos) {
|
||||
const flags: string[] = [];
|
||||
if (c.missing) flags.push("DID-NOT-ROUND-TRIP");
|
||||
for (const i of c.raw) {
|
||||
const authored =
|
||||
i.authored === ABSENT ? "absent" : JSON.stringify(i.authored);
|
||||
flags.push(
|
||||
`RAW ${i.type}.${i.attr}: ${authored} -> ${JSON.stringify(i.got)} (expected ${JSON.stringify(i.expected)})`,
|
||||
);
|
||||
}
|
||||
if (c.canonical) {
|
||||
flags.push(
|
||||
`CANON @ ${c.canonical.path}: ${JSON.stringify(c.canonical.a)} vs ${JSON.stringify(c.canonical.b)}`,
|
||||
);
|
||||
}
|
||||
const status = flags.length === 0 ? "stable" : flags.join("; ");
|
||||
lines.push(` [${c.label}] ${status}`);
|
||||
}
|
||||
return lines.join("\n");
|
||||
}
|
||||
@@ -0,0 +1,164 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
runStabilityMatrix,
|
||||
unstableCombos,
|
||||
formatReport,
|
||||
runConvergenceCase,
|
||||
convergenceCasesFor,
|
||||
convergenceOk,
|
||||
formatConvergence,
|
||||
type NodeStabilitySpec,
|
||||
} from "./roundtrip-stability.helper.js";
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Round-trip STABILITY matrix for image + the media family.
|
||||
//
|
||||
// Guards the "empty-string-vs-absent" churn class (GS-EDIT-REVERT family): a
|
||||
// stored node authored WITHOUT a string attr (alt/title/caption/aria-label/...)
|
||||
// must not gain a phantom `attr: ""` after `markdownToProseMirror(convert…)`.
|
||||
// Each spec sweeps the at-risk string attrs at DEFAULT (absent) and at a real
|
||||
// NON-default value; the helper asserts both the RAW round-trip (attrs equal the
|
||||
// input's, modulo the documented numeric width/height/size/aspectRatio -> string
|
||||
// coercion) and the CANONICAL round-trip (canonical forms deep-equal).
|
||||
//
|
||||
// The image + media family share the `align !== "center"` predicate and the
|
||||
// `<!--name {…}-->` comment machinery, so one matrix guards the shared class.
|
||||
// align is NOT part of this class (it round-trips correctly) and is not swept.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
const SPECS: NodeStabilitySpec[] = [
|
||||
{
|
||||
// Image carries the most at-risk string attrs. `alt` is the one marked
|
||||
// materializes as `<img alt="">` on `` import (the real bug); title
|
||||
// and caption are covered as the same class. attachmentId is a string attr
|
||||
// that must stay absent when unset (control).
|
||||
type: "image",
|
||||
baseAttrs: { src: "/i.png" },
|
||||
attrMatrix: [
|
||||
{ attr: "alt", default: undefined, nonDefault: "a real alt text", emptyStringClass: true },
|
||||
{ attr: "title", default: undefined, nonDefault: "a real title", emptyStringClass: true },
|
||||
{ attr: "caption", default: undefined, nonDefault: "a real caption" },
|
||||
{ attr: "attachmentId", default: undefined, nonDefault: "att-42" },
|
||||
],
|
||||
},
|
||||
{
|
||||
// Video's `alt` rides the `aria-label` attribute (media aria-label at risk).
|
||||
type: "video",
|
||||
baseAttrs: { src: "/v.mp4" },
|
||||
attrMatrix: [
|
||||
{ attr: "alt", default: undefined, nonDefault: "a clip", emptyStringClass: true },
|
||||
{ attr: "attachmentId", default: undefined, nonDefault: "att-1" },
|
||||
],
|
||||
},
|
||||
{
|
||||
// Audio carries no alt/title; attachmentId is its only optional string attr.
|
||||
type: "audio",
|
||||
baseAttrs: { src: "/a.mp3" },
|
||||
attrMatrix: [
|
||||
{ attr: "attachmentId", default: undefined, nonDefault: "att-2" },
|
||||
],
|
||||
},
|
||||
{
|
||||
// pdf: link-form media. `name` (filename) is its at-risk string attr.
|
||||
type: "pdf",
|
||||
baseAttrs: { src: "/d.pdf" },
|
||||
attrMatrix: [
|
||||
{ attr: "name", default: undefined, nonDefault: "report.pdf", emptyStringClass: true },
|
||||
{ attr: "attachmentId", default: undefined, nonDefault: "att-3" },
|
||||
],
|
||||
},
|
||||
{
|
||||
// attachment: link-form media (file card). `name` + `mime` string attrs.
|
||||
type: "attachment",
|
||||
baseAttrs: { url: "/f.zip" },
|
||||
attrMatrix: [
|
||||
{ attr: "name", default: undefined, nonDefault: "bundle.zip", emptyStringClass: true },
|
||||
{ attr: "mime", default: undefined, nonDefault: "application/zip", emptyStringClass: true },
|
||||
{ attr: "attachmentId", default: undefined, nonDefault: "att-4" },
|
||||
],
|
||||
},
|
||||
{
|
||||
// embed: link-form media. `provider` is its at-risk string attr (schema
|
||||
// default ""). embed's numeric width/height defaults (800/600) are a SEPARATE,
|
||||
// documented limitation OUTSIDE the empty-string class: they are not in
|
||||
// canonicalize's KNOWN_DEFAULTS, so an ABSENT width/height re-imports as the
|
||||
// 800/600 default and diverges canonically (see the note in canonicalize.ts).
|
||||
// That is canonicalize-owned and out of scope here, so we author the
|
||||
// dimensions at their defaults (as real editor embeds carry them) to keep this
|
||||
// guard focused on the empty-string/provider class.
|
||||
// provider's schema default is "" (NOT null), so a re-imported "" is the
|
||||
// correct value, not a phantom — it is outside the null-default empty-string
|
||||
// class. We author it at its "" default (the default pick) so the sweep still
|
||||
// asserts a non-default provider ("youtube") round-trips, without tripping the
|
||||
// canonicalize KNOWN_DEFAULTS gap for embed's non-null defaults.
|
||||
type: "embed",
|
||||
baseAttrs: { src: "https://example.com/x", width: 800, height: 600 },
|
||||
attrMatrix: [
|
||||
{ attr: "provider", default: "", nonDefault: "youtube" },
|
||||
],
|
||||
},
|
||||
{
|
||||
// drawio: image-form diagram. `title` + `alt` string attrs (data-title/-alt).
|
||||
type: "drawio",
|
||||
baseAttrs: { src: "blob:drawio" },
|
||||
attrMatrix: [
|
||||
{ attr: "title", default: undefined, nonDefault: "flow chart", emptyStringClass: true },
|
||||
{ attr: "alt", default: undefined, nonDefault: "an alt", emptyStringClass: true },
|
||||
{ attr: "attachmentId", default: undefined, nonDefault: "att-5" },
|
||||
],
|
||||
},
|
||||
{
|
||||
// excalidraw: image-form diagram, same shared diagramAttributes set.
|
||||
type: "excalidraw",
|
||||
baseAttrs: { src: "blob:excalidraw" },
|
||||
attrMatrix: [
|
||||
{ attr: "title", default: undefined, nonDefault: "sketch", emptyStringClass: true },
|
||||
{ attr: "alt", default: undefined, nonDefault: "an alt", emptyStringClass: true },
|
||||
{ attr: "attachmentId", default: undefined, nonDefault: "att-6" },
|
||||
],
|
||||
},
|
||||
];
|
||||
|
||||
describe("round-trip stability matrix (image + media family)", () => {
|
||||
for (const spec of SPECS) {
|
||||
it(`${spec.type}: no attr materializes an empty-string / phantom value`, async () => {
|
||||
const report = await runStabilityMatrix(spec);
|
||||
const unstable = unstableCombos(report);
|
||||
// On failure, print the WHOLE matrix so which (attr, value) combos are
|
||||
// unstable is legible.
|
||||
expect(unstable, `\n${formatReport(report)}\n`).toEqual([]);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// THIRD STATE: an attr EXPLICITLY stored as a literal "" (GS-EDIT-REVERT: a user
|
||||
// typed alt/title/name/... then deleted it, so Tiptap persisted `attr: ""` — a
|
||||
// value DISTINCT from "attr was never set"). Unlike the absent case above, this
|
||||
// state is NOT first-pass byte-stable: the fix's `"" -> default` coercion is a
|
||||
// deliberate ONE-TIME normalization on the FIRST sync round-trip, stable
|
||||
// thereafter. We therefore assert a DIFFERENT contract — "converges to default
|
||||
// on pass 1, then idempotent from pass 2 on" — for every empty-string-class attr
|
||||
// across the whole node family (image/video/pdf/attachment/drawio/excalidraw).
|
||||
//
|
||||
// IMPORTANT for a future sync/QA pass: the pass-1 `"" -> null` diff is the
|
||||
// converter canon, not corruption. It appears at most once per affected node and
|
||||
// must NOT be flagged as "the converter is losing/corrupting page data".
|
||||
// ---------------------------------------------------------------------------
|
||||
describe("round-trip third state: explicit empty string converges once, then idempotent", () => {
|
||||
for (const spec of SPECS) {
|
||||
for (const attr of convergenceCasesFor(spec)) {
|
||||
it(`${spec.type}.${attr}: "" normalizes to default on pass 1, byte-stable from pass 2`, async () => {
|
||||
const r = await runConvergenceCase(spec, attr);
|
||||
// Pass 1 must converge "" -> the schema default (the one-time diff) and
|
||||
// pass 2 (roundtrip of pass-1 output) must be byte-stable. formatConvergence
|
||||
// prints exactly which half failed.
|
||||
expect(convergenceOk(r), `\n${formatConvergence(r)}\n`).toBe(true);
|
||||
// Spell the contract out explicitly so the intent is legible in the test:
|
||||
expect(r.convergedToDefault, `\n${formatConvergence(r)}\n`).toBe(true);
|
||||
expect(r.firstPassValue).toEqual(r.expectedDefault);
|
||||
expect(r.secondPassDivergence, `\n${formatConvergence(r)}\n`).toBeNull();
|
||||
});
|
||||
}
|
||||
}
|
||||
});
|
||||
Reference in New Issue
Block a user