Compare commits

...

15 Commits

Author SHA1 Message Date
agent_coder 9d1b033fe8 fix(#344 review F1-F4): test-mock coverage + getSpaces freshness + comment/test fixes
- F1 [blocking]: share-modal.test.tsx + comment-content-view.test.tsx mocked
  page-query without usePageMetaQuery → 3 tests threw (ShareModal uses it
  directly, comment-content-view via MentionContent). Added usePageMetaQuery to
  both mocks (the space-tree mocks were already fixed; these two were missed).
- F2: restored refetchOnMount:true on useGetSpacesQuery — ["spaces"] is
  invalidated only by same-tab mutations (no socket path), so a cross-actor
  change (an admin adding/removing THIS user from a space) left the list stale
  until a hard reload. The other refetchOnMount removals (favorites/watched —
  per-user, same-tab-only gap) stay removed.
- F3: corrected the trash-list + recent-changes KEEP comments — both keys ARE
  invalidated (trash-list by 3 mutations, recent-changes by page CRUD), but
  invalidateQueries only marks an UNMOUNTED query stale without refetching, so the
  mount refetch closes the gap. The old "never invalidated" wording was wrong and
  risked a maintainer deleting a live invalidation as dead code.
- F4: tests for the two load-bearing pure paths — invalidate-on-update-page (the
  undefined-guard: a title-only event keeps the icon; sibling/unrelated subtrees
  untouched) and breadcrumb-path-equal (equal chain → true; any id/slugId/name/
  icon change or length diff → false; both-null → true). Exported
  breadcrumbPathEqual for the test.

Gate: client tsc 0; the 4 affected/new test files 33 passed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-05 01:05:32 +03:00
agent_coder fcbe840c74 perf(client): cut background re-renders + duplicate work (#344)
Outside the editor the UI did background work on every tree event, socket
reconnect, and navigation. Tree infra (virtualization/memo/O(N) utils) was
already good — the cost was in the subscriptions and duplicates around it.
Client-only; behavior 1:1.

- Setter-only atom subscriptions → useSetAtom: space-tree-row, use-tree-mutation,
  use-tree-socket no longer subscribe every visible row to the WHOLE treeDataAtom
  value (a tree event re-rendered all ~20-30 rows, bypassing the DocTreeRow memo).
  space-tree-node-menu / mention-list read the tree imperatively (store.get) in
  their handlers only. breadcrumb.tsx uses a selectAtom slice (ancestor chain +
  field equality) instead of the whole-tree subscription.
- Socket handler cleanup (BUG): use-tree-socket + use-query-subscription now
  socket.off() their named handlers on cleanup (were accumulating listeners on
  every reconnect → duplicated invalidations/tree-walks). Mirrors
  use-notification-socket.
- Field-update tree path: invalidateOnUpdatePage does a pointwise patch of the
  cached embed subtrees instead of a blanket invalidatePageTree() (refetch storm);
  structural events keep the blanket invalidate.
- usePageMetaQuery: a content-less select slice for the 13 peripheral subscribers
  that read only title/permissions/id, so they stop re-rendering every ~3s while
  typing / on every collab page.updated (page.tsx keeps the full query for content).
- page.tsx: skeleton + placeholderData keepPreviousData (no blank flash on nav).
- Removed refetchOnMount:true where socket/mutation invalidation already keeps the
  cache fresh (favorite/space/space-watcher/workspace). KEPT it on the 3 queries
  with NO other freshness path (trash-list, created-by, recent-changes) — the
  global default is refetchOnMount:false, so those overrides are load-bearing.
- Small: resize mousemove/up attached only while dragging; per-row emoji-picker
  keydown gated on `opened`; AiChatWindow queries enabled only when the window is
  open.

Gate: client tsc 0, client vitest page+websocket 200 passed (+editor suites),
build ok.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-05 00:34:51 +03:00
agent_vscode 5336f06d10 Merge pull request 'fix(e2e)+ci: канон callout '> [!info]' в e2e-mcp + параллельная сборка с гейтом на publish' (#356) from fix/e2e-callout-and-gate-build into develop 2026-07-04 22:42:11 +03:00
agent_vscode 4bd579f7f6 ci(develop): build image in parallel with tests, gate only the publish
Two-phase scheme instead of the sequential gate: the build job runs in
parallel with test/e2e jobs and only warms the buildx GHA cache
(push:false, cache-to mode=max); a new publish job (needs: test,
e2e-server, e2e-mcp, build) rebuilds from the warm cache (near-instant
on hit, full rebuild on eviction — same as the old sequential timing)
and pushes :develop. GHCR login moved to publish; build-args blocks are
kept textually identical between the two jobs so the cache hits.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-04 22:41:25 +03:00
agent_vscode 7bf1c91a95 ci(develop): gate the :develop image build on e2e suites
Reverse the previous policy where e2e jobs only turned the run red
without blocking the image publish: build.needs now lists test,
e2e-server and e2e-mcp, so a failing test of any kind stops the
:develop image from being built and pushed. Stale policy comments
updated accordingly.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-04 22:33:06 +03:00
agent_vscode 6c82c54470 test(mcp): expect Obsidian '> [!info]' callout export in e2e (#333 canon)
PR #333 deliberately changed the canonical markdown export of callout
nodes to the Obsidian-native format ('> [!type]' + blockquote body,
pinned by packages/prosemirror-markdown unit tests); the importer still
parses both ':::type' fences and '> [!type]'. The get_page e2e assertion
was missed in that switch and still expected ':::info', failing the
e2e-mcp job on develop since 4369bbc5.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-04 22:33:06 +03:00
agent_vscode 382e5196da Merge pull request 'fix(docker): toolchain python3/make/g++ для нативной сборки re2' (#353) from fix/docker-re2-toolchain into develop 2026-07-04 22:11:49 +03:00
agent_vscode 76e0c08cec fix(docker): install python3/make/g++ toolchain for re2 native build
The develop image build broke at `pnpm install --frozen-lockfile`: the new
native dependency re2@1.25.0 (packages/mcp, search_in_page #330) always
compiles from source under pnpm — its prebuilt-binary downloader
(install-artifact-from-github) cannot identify the GitHub repo because pnpm
does not populate npm_package_repository_*/npm_package_json env vars ("No
github repository was identified. Building locally ..."), and node:22-slim
ships no python3/make/g++ for the node-gyp fallback.

- builder stage: add a cache-friendly apt layer with python3 make g++
  before COPY; the stage is discarded so the toolchain may stay.
- installer stage: install the toolchain, run the prod install as the node
  user via `su node -c`, and purge the toolchain — all in one RUN layer so
  the final image stays slim and node_modules ownership needs no extra
  chown layer; USER node is restored right after.

Fixes the failed run 28715009124 (develop docker build); release.yml uses
the same Dockerfile and is covered too.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-04 22:09:40 +03:00
vvzvlad 8978d69f3e Merge pull request 'fix(converter): стабильность round-trip image/медиа — «» ≡ absent (класс defaults-instability)' (#350) from fix/media-roundtrip-stability into develop
Reviewed-on: #350
2026-07-04 21:30:12 +03:00
agent_coder c192f2a2e1 test(prosemirror-markdown): pin the third state — explicit "" converges once, then idempotent
Reviewer addition to the round-trip stability matrix: besides "attr absent" and
"attr has a real value", a string attr in the empty-string class has a third,
degenerate state — a LITERAL "" (a user types alt/title/name in the editor then
deletes it, and Tiptap persists `attr: ""`, distinct from never-set). The fix's
`getAttribute(...) || null` coercion normalizes such a stored "" to the default
on the FIRST round-trip (a one-time "" -> null diff) and is byte-stable from the
SECOND round-trip on.

Adds a convergence contract to the reusable matrix helper (emptyStringClass flag
+ runConvergenceCase): pass 1 must converge the attr to its schema default (NOT
asserted byte-stable vs the "" input — that is the intended one-time
normalization); pass 2 must deep-equal pass 1 (idempotent thereafter). Driven for
every empty-string-class attr across image + the media family (image/drawio
alt+title, video alt via aria-label, pdf/attachment name, attachment mime).
Documents the one-time normalization so a future sync/QA diff does not flag the
single "" -> null change as converter corruption.

Gate: package suite 33 files / 682 tests passed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-04 21:17:17 +03:00
vvzvlad d78b985062 Merge pull request 'perf(comment): статический рендер + ленивые редакторы + мемоизация панели (#340)' (#349) from fix/340-comment-panel-perf into develop
Reviewed-on: #349
2026-07-04 20:55:11 +03:00
agent_coder 2ce672709a fix(prosemirror-markdown): stabilize image round-trip — "" ≡ absent on parse (empty-string class)
A stored image authored without `alt` gained a phantom `alt: ""` on every
round-trip (`markdownToProseMirror(convertProseMirrorToMarkdown(doc))`): `marked`
renders `![](src)` as `<img alt="">`, and the stock tiptap Image `alt` parseHTML
(`getAttribute("alt")`) materialized the empty string where the original had no
attribute. That false diff is a real GS-EDIT-REVERT churn source — an agent /
git-sync touch of a page with an image mutates the stored JSON (`absent -> ""`),
producing phantom diffs that can overwrite live edits.

Fix is PARSE-SIDE ("" ≡ absent), so the RAW round-trip is idempotent — not only
the canonical form (history / stored JSON diff on the raw shape; masking it only
in canonicalize would leave that noise). `image.alt`/`title` parseHTML now coerce
`getAttribute(...) || null`, plus defense-in-depth `|| null` across the at-risk
empty-string class (video aria-label, drawio/excalidraw title+alt, pdf name,
attachment name+mime) matching the existing `image.caption || null` precedent.

NOTE — image `align` is NOT changed: it round-trips correctly (center via the
schema default "center", left/right via the `<!--img {...}-->` comment). Its
`toBeUndefined()` in the git-sync gate is canonical-form normalization, not a loss.

Intentional divergence from editor-ext: editor-ext's literal `alt` parseHTML
returns "" verbatim, but this coercion CONVERGES on editor-ext's real STORED
shape (an image inserted without alt has no `alt` attribute -> re-parses absent,
never ""), so the round-trip is idempotent and matches real documents.

Adds a reusable, node-agnostic round-trip-stability matrix helper
(test/roundtrip-stability.helper.ts) — given a node + attr spec it enumerates
default/non-default combos and asserts byte-stability of BOTH the raw and the
canonical round-trip (the documented numeric width/height→string coercion encoded
as an explicit allowed normalization) — driven over image + the whole media
family (video/audio/pdf/attachment/embed/drawio/excalidraw). The only raw
empty-string instability it found was image.alt; the family was already stable.

Gate: package suite 33 files / 672 tests passed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-04 20:51:34 +03:00
agent_coder a4fc6c7f64 fix(comment): underline mark + draft-surviving tabs + test coverage (#349 review F1-F4)
- F1: render the `underline` mark statically (StarterKit v3 enables Underline;
  comment-editor does not disable it) — an underlined comment no longer degrades
  the whole comment to the read-only editor fallback. renderMarks gains a
  `case "underline" -> <u>`, mirroring the other marks (+ test).
- F2: keep the Open tab panel mounted (`Tabs.Panel value="open" keepMounted`)
  while the heavy Resolved panel still unmounts (`Tabs keepMounted={false}`). A
  per-panel keepMounted overrides the parent's `false` (Mantine 8 TabsPanel), so
  an in-progress reply draft / edit in the Open panel survives an
  Open->Resolved->Open switch, keeping the micro-opt of not mounting the large
  Resolved list.
- F3: cover edit->save->re-render in comment-list-item.test.tsx — save calls
  mutateAsync with JSON.stringify(editContentRef) and a new comment.content prop
  updates the visible body; cancel restores the static body without mutating;
  clearing editContentRef after cancel.
- F4: extract childrenByParent grouping into an exported pure
  `buildChildrenByParent(items)` (unit-tested: nesting, orphan reply, sibling
  order) + new comment-list-with-tabs.test.tsx covering the lazy reply-editor
  activation (stub -> click/focus/Enter mounts the editor).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-04 20:48:21 +03:00
vvzvlad c252068672 Merge pull request 'feat(ai-chat): отложенная загрузка инструментов (deferred tools + loadTools) (#332)' (#341) from fix/332-deferred-tools into develop
Reviewed-on: #341
2026-07-04 20:47:45 +03:00
agent_coder cb9c5dda59 perf(comment): static comment renderer + lazy editors + memoized list (#340)
The comment panel lagged for seconds on open and stuttered on every resolve/apply
with many comments (real case: 30 open + 326 resolved ≈ 356 threads), because each
comment body mounted a full TipTap/ProseMirror editor, both tabs mounted at once,
and any mutation re-rendered the whole list.

- CommentContentView: static recursive renderer of comment ProseMirror JSON (no
  editor instance) for the read-only body — supports exactly CommentEditor's node
  set (doc/paragraph/text/hardBreak/mention) + marks (bold/italic/strike/code/
  link), reproducing the 3-level DOM nesting for pixel-identical CSS. Unknown
  node/mark or unparseable content degrades that one comment to the read-only
  CommentEditor; legacy non-JSON strings render as plain text.
  SECURITY: link hrefs are protocol-allowlisted (safeHref, mirroring
  @tiptap/extension-link) so a stored comment with a `javascript:`/`data:` href
  cannot XSS — the old TipTap read-only path sanitized this; the static renderer
  must too. Control-char smuggling (java\tscript:) is stripped before the check.
- MentionContent extracted from MentionView, shared by the TipTap NodeView and the
  static renderer (identical user/page-mention behavior).
- keepMounted={false} on the tabs: the inactive tab no longer mounts its editors.
- Lazy reply editor: a stub until click/focus, then the real editor (kept mounted
  so the draft survives thread re-renders).
- React.memo(CommentListItem) + a childrenByParent map (replaces the per-thread
  O(n^2) filter) + localized reply-send pending state: resolve/apply/reply now
  re-render only the touched thread.
- Progressive first paint: useCommentsQuery no longer blocks on hasNextPage.

Gate: client comment+mention suites 22/22 passed, tsc --noEmit 0.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-04 20:20:32 +03:00
44 changed files with 2030 additions and 156 deletions
+42 -11
View File
@@ -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
View File
@@ -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");
+17 -9
View File
@@ -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,
});
}
+2 -2
View File
@@ -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(() => {
+18 -3
View File
@@ -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>
);
}
+1 -1
View File
@@ -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 `![](src)` 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 `![](src)` 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();
});
}
}
});