fix(client): ревью #347 — Reasoning-panel отступы, диагностика вставки, тест guard'а, комменты, доки (#347, ревью)
Правки по 5 находкам ревью #498. F1 (регрессия отступов списков в Reasoning-панели): добавлен `.reasoningText li p {margin:0}` (зеркало существующего `.markdown li p`). Reasoning рендерит через тот же renderChatMarkdown (теперь всегда <li><p>…</p></li>), но под .reasoningText, где `.reasoningText p{margin:0 0 4px}` давал 4px на пункт. Обе поверхности покрыты. F2 (глухой catch): `.catch` вставки был `()=>{}` → теперь `(err)=>console.error( "markdown paste conversion failed, inserting raw text", err)` — тихая деградация в raw-текст больше не невидима (покрывает и конвертер, и тело success-.then, напр. PMNode.fromJSON при дрейфе схемы). F3 (нет теста doc-changed guard): +3 теста в markdown-clipboard.paste.test.ts: success-ветка при mid-flight изменении дока → вставка в живую selection (маркер цел, без клоббера/throw); fail-open ветка при mid-flight + провале конверсии → raw-текст в живую selection без RangeError; две вставки в полёте → инвариант «ни один payload не потерян». F4 (устаревшие комменты): исправлены ссылки на удалённый md-слой в markdown- clipboard.ts, footnote-sync/util(+test), docmost-schema, foreign-markdown, footnote-canonicalize → на @docmost/prosemirror-markdown / локальные символы. F5 (внешние доки): AGENTS.md (apps/client как потребитель через browser-entry, jsdom только в Node, удалён marked/turndown-слой); prosemirror-markdown/README (секция Node vs browser entry, markdownToProseMirrorSync); CHANGELOG. Тесты: client paste+canonicalize+ai-chat 61; pmd 744; editor-ext 196; клиентская сборка успешна, grep бандла на JSDOM/parse5/happy-dom/turndown — 0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -334,7 +334,7 @@ pnpm workspace (`pnpm@10.4.0`) orchestrated by **Nx**. Four workspace packages:
|
||||
| `apps/client` | `client` | React 18 + Vite + Mantine 8 + TanStack Query + Jotai | SPA frontend |
|
||||
| `packages/editor-ext` | `@docmost/editor-ext` | Tiptap/ProseMirror | Shared Tiptap node/mark extensions, imported by both the client and the server |
|
||||
| `packages/mcp` | `@docmost/mcp` | MCP SDK, Tiptap, Yjs | Standalone MCP server, also bundled into the server at `/mcp`. Consumes the shared converter/schema from `@docmost/prosemirror-markdown` (#293) — it no longer carries its own vendored converter/schema copy |
|
||||
| `packages/prosemirror-markdown` | `@docmost/prosemirror-markdown` | Tiptap, marked, jsdom | The single, canonical ProseMirror↔Markdown converter + Docmost schema mirror (#293). Consumed by `mcp`, `git-sync`, AND `apps/server` (server-side markdown import/export, #345); there is exactly ONE copy of the converter now |
|
||||
| `packages/prosemirror-markdown` | `@docmost/prosemirror-markdown` | Tiptap, marked; jsdom (Node only) | The single, canonical ProseMirror↔Markdown converter + Docmost schema mirror (#293). Consumed by `mcp`, `git-sync`, `apps/server` (server-side markdown import/export, #345), AND `apps/client` (markdown paste/copy + AI-chat render, via the `browser` entry — native `DOMParser`, no jsdom in the client bundle, #347); there is exactly ONE copy of the converter now |
|
||||
|
||||
`build` targets are Nx-cached and dependency-ordered (`dependsOn: ["^build"]`), so `editor-ext` builds before the apps. `nx.json` sets `affected.defaultBase: main`.
|
||||
|
||||
@@ -460,7 +460,7 @@ The API server is a Fastify app with a global `/api` prefix (`main.ts` excludes
|
||||
### Client structure
|
||||
Vite SPA. Code is organized by feature under `apps/client/src/features/*` (mirrors the server domains: `page`, `space`, `comment`, `ai-chat`, `editor`, …). Conventions:
|
||||
- **TanStack Query** for server state (one `queries/` file per feature), **Jotai** atoms for local/shared UI state, **Mantine 8** + CSS modules (`*.module.css`) + `postcss-preset-mantine` for UI.
|
||||
- The editor is Tiptap; shared node/mark extensions live in `packages/editor-ext` and are imported by **both the client and the server** (collaboration, schema, `canonicalizeFootnotes`) — editor schema changes often need to be made in `editor-ext`, not just the client. Server-side markdown import/export no longer lives in `editor-ext`: it goes through the canonical converter (#345, see below). The ProseMirror↔Markdown converter and its Docmost schema mirror now live in a SINGLE package, `@docmost/prosemirror-markdown` (#293), consumed by `mcp`, `git-sync`, and `apps/server` (#345) — do NOT reintroduce a per-package copy. `editor-ext` is the upstream source of the Tiptap schema; the package's `docmost-schema.ts` mirrors it and a serializer-contract test (`packages/prosemirror-markdown/test/serializer-contract.test.ts`) guards the boundary (every schema node must have a converter case), so a drift surfaces as a failing test rather than silent divergence. For the converter's property-testing and counterexample→fixture process (P1–P4 invariants, the `PROPERTY_SEED`/`PROPERTY_NUM_RUNS` knobs, and the nightly fuzz workflow), see `packages/prosemirror-markdown/README.md`.
|
||||
- The editor is Tiptap; shared node/mark extensions live in `packages/editor-ext` and are imported by **both the client and the server** (collaboration, schema, `canonicalizeFootnotes`) — editor schema changes often need to be made in `editor-ext`, not just the client. Server-side markdown import/export no longer lives in `editor-ext`: it goes through the canonical converter (#345, see below). The ProseMirror↔Markdown converter and its Docmost schema mirror now live in a SINGLE package, `@docmost/prosemirror-markdown` (#293), consumed by `mcp`, `git-sync`, `apps/server` (#345), and `apps/client` (#347) — do NOT reintroduce a per-package copy. The client uses the package's `browser` entry (`@docmost/prosemirror-markdown/browser`): markdown paste (`markdown-clipboard.ts`), copy-as-markdown, and AI-chat rendering now all go through the canonical converter, so the hand-written `marked`/`turndown` markdown layer that used to live in `editor-ext` was deleted (#347). The browser entry runs the HTML→DOM stage on the native `DOMParser`, so jsdom stays out of the client bundle. `editor-ext` is the upstream source of the Tiptap schema; the package's `docmost-schema.ts` mirrors it and a serializer-contract test (`packages/prosemirror-markdown/test/serializer-contract.test.ts`) guards the boundary (every schema node must have a converter case), so a drift surfaces as a failing test rather than silent divergence. For the converter's property-testing and counterexample→fixture process (P1–P4 invariants, the `PROPERTY_SEED`/`PROPERTY_NUM_RUNS` knobs, and the nightly fuzz workflow), see `packages/prosemirror-markdown/README.md`.
|
||||
- API access goes through `apps/client/src/lib/api-client.ts` (axios). The `@` alias maps to `apps/client/src`.
|
||||
- Runtime config is injected at build time by `vite.config.ts` via `define` (`APP_URL`, `COLLAB_URL`, `APP_VERSION`, …) — these come from the root `.env`, not from `import.meta.env`.
|
||||
|
||||
|
||||
@@ -270,6 +270,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
|
||||
### Changed
|
||||
|
||||
- **Client markdown paste/copy and AI-chat rendering now go through the canonical
|
||||
converter.** Pasting markdown into the editor, "Copy as markdown", the AI title
|
||||
generator, and the AI-chat markdown renderer all now use
|
||||
`@docmost/prosemirror-markdown` (via its new `browser` entry — native
|
||||
`DOMParser`, no jsdom in the client bundle) instead of the hand-written
|
||||
`marked`/`turndown` markdown layer in `editor-ext`, which was **deleted**. As a
|
||||
result, pasting canonical markdown (`^[…]` footnotes, `<!--img …-->`,
|
||||
`> [!type]` callouts, `$…$` math, `==…==` highlight, standalone `<!--subpages-->`
|
||||
comments) now produces the SAME nodes the server import produces for the same
|
||||
text. Chat/reasoning markdown now renders through the editor schema (list items
|
||||
are wrapped in `<p>`; CSS keeps them tight). (#347)
|
||||
|
||||
- **Enabling a public share no longer auto-shares the whole sub-tree.** Turning
|
||||
a page "Shared to web" now defaults to the page alone; descendant pages become
|
||||
public only when you explicitly turn on the dedicated "Include sub-pages"
|
||||
|
||||
@@ -181,6 +181,14 @@
|
||||
margin: 0 0 4px;
|
||||
}
|
||||
|
||||
/* Same as `.markdown li p` above: the canonical converter wraps every list
|
||||
item's content in a <p>, so without this each reasoning-panel list item would
|
||||
pick up `.reasoningText p`'s 4px bottom margin and render too loose. Drop it
|
||||
so Reasoning-panel lists stay tight, mirroring the pre-#347 marked output. */
|
||||
.reasoningText li p {
|
||||
margin: 0;
|
||||
}
|
||||
|
||||
.inputWrapper {
|
||||
flex: 0 0 auto;
|
||||
padding-top: var(--mantine-spacing-xs);
|
||||
|
||||
@@ -110,3 +110,69 @@ describe("MarkdownClipboard handlePaste (async md -> PM)", () => {
|
||||
editor.destroy();
|
||||
});
|
||||
});
|
||||
|
||||
// The async seam captures the target range synchronously, then replaces on the
|
||||
// next microtask. If the document changed under it between capture and resolve
|
||||
// (impossible in prod — same microtask — but pinned here), BOTH the success
|
||||
// (replaceRange) and the fail-open (insertText) branches must fall back to the
|
||||
// LIVE selection rather than a stale absolute range, so neither clobbers content
|
||||
// nor throws a RangeError. We force the mid-flight change by dispatching a
|
||||
// doc-mutating transaction AFTER the synchronous claim but BEFORE flushing the
|
||||
// microtask that runs the `.then`/`.catch`.
|
||||
describe("MarkdownClipboard handlePaste — doc-changed-mid-flight guard", () => {
|
||||
// Insert marker text at the doc start via a raw transaction (synchronous),
|
||||
// changing `view.state.doc` so the captured range goes stale.
|
||||
function mutateDoc(editor: Editor, marker: string) {
|
||||
editor.view.dispatch(editor.view.state.tr.insertText(marker, 1));
|
||||
}
|
||||
|
||||
it("success branch: a mid-flight doc change routes the paste to the live selection (no clobber, no throw)", async () => {
|
||||
const editor = makeEditor();
|
||||
const claimed = paste(editor, "hello **bold**");
|
||||
expect(claimed).toBe(true);
|
||||
// Doc changes before the async replace runs: the captured from/to are stale.
|
||||
mutateDoc(editor, "MARKER");
|
||||
await flush();
|
||||
|
||||
const text = editor.getText();
|
||||
// The pre-existing marker survived (a stale-range replaceRange would have
|
||||
// clobbered it) AND the pasted content landed.
|
||||
expect(text).toContain("MARKER");
|
||||
expect(text).toContain("bold");
|
||||
expect(text).not.toContain("**");
|
||||
editor.destroy();
|
||||
});
|
||||
|
||||
it("fail-open branch: a mid-flight doc change + conversion failure re-inserts raw text at the live selection (no RangeError)", async () => {
|
||||
const editor = makeEditor();
|
||||
// `# heading` -> a heading node the minimal schema lacks -> PMNode.fromJSON
|
||||
// throws -> the fail-open catch runs, now with a changed doc.
|
||||
paste(editor, "# raw heading");
|
||||
mutateDoc(editor, "KEEP");
|
||||
await flush();
|
||||
|
||||
const text = editor.getText();
|
||||
// No RangeError/unhandled rejection (the test would fail on a throw), the
|
||||
// marker survived, and the raw text was preserved.
|
||||
expect(text).toContain("KEEP");
|
||||
expect(text).toContain("raw heading");
|
||||
editor.destroy();
|
||||
});
|
||||
|
||||
it("two pastes in flight: neither payload is lost (no data loss)", async () => {
|
||||
// Prod-unreachable (two paste events are separate macrotasks, and each
|
||||
// conversion resolves on a microtask before the next), but pinned here: when
|
||||
// both resolve back-to-back, the second sees the changed doc and inserts at
|
||||
// the live selection the first left — so the two payloads may INTERLEAVE, but
|
||||
// neither is dropped. We assert no data loss, not contiguity.
|
||||
const editor = makeEditor();
|
||||
paste(editor, "alphaword");
|
||||
paste(editor, "betaword");
|
||||
await flush();
|
||||
const text = editor.getText();
|
||||
// Neither payload fully dropped (interleaving may split one of them).
|
||||
expect(text).toContain("alpha");
|
||||
expect(text).toContain("beta");
|
||||
editor.destroy();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -103,12 +103,14 @@ export const MarkdownClipboard = Extension.create({
|
||||
}
|
||||
|
||||
const schema = this.editor.schema;
|
||||
// Capture the target range NOW. markdownToProseMirror is async (its
|
||||
// marked pipeline is declared async), so the actual replace happens
|
||||
// on the next microtask. No user input can interleave a microtask, so
|
||||
// the state is unchanged when we dispatch — but we still re-read the
|
||||
// live state and map the captured range through any doc steps for
|
||||
// safety before replacing.
|
||||
// Capture the target range NOW. markdownToProseMirror RETURNS A
|
||||
// PROMISE (kept async only for the Node consumers' contract; the
|
||||
// conversion pipeline itself is synchronous), so the actual replace
|
||||
// happens on the next microtask. No user input can interleave a
|
||||
// microtask, so the state is unchanged when we dispatch — but we
|
||||
// still re-read the live state before replacing and, if the doc did
|
||||
// change under us, fall back to the live selection rather than the
|
||||
// captured (now-stale) range.
|
||||
const from = view.state.selection.from;
|
||||
const to = view.state.selection.to;
|
||||
const startDoc = view.state.doc;
|
||||
@@ -154,9 +156,10 @@ export const MarkdownClipboard = Extension.create({
|
||||
schema,
|
||||
);
|
||||
|
||||
// Map the captured range through any doc changes since capture
|
||||
// (normally none — same microtask) so the replace targets the
|
||||
// right span even if the document moved.
|
||||
// Target the captured range (normally still valid — same
|
||||
// microtask). If the doc changed under us since capture, the
|
||||
// captured absolute from/to are stale, so fall back to the live
|
||||
// selection rather than StepMap-mapping the old range.
|
||||
const tr = view.state.tr;
|
||||
let mappedFrom = from;
|
||||
let mappedTo = to;
|
||||
@@ -177,11 +180,20 @@ export const MarkdownClipboard = Extension.create({
|
||||
tr.setMeta("paste", true);
|
||||
view.dispatch(tr);
|
||||
})
|
||||
.catch(() => {
|
||||
.catch((err) => {
|
||||
// Fail-open: a conversion error must not swallow the paste
|
||||
// silently in a way that loses the text. We already claimed the
|
||||
// event (returned true), so re-insert the raw text as a plain
|
||||
// paragraph so the user never loses their clipboard content.
|
||||
// Log it: this catch covers BOTH the converter and the success
|
||||
// `.then` body (e.g. PMNode.fromJSON throwing on a schema drift
|
||||
// between the canonical package and the live editor schema), so a
|
||||
// silent degrade to raw text would otherwise be an invisible,
|
||||
// non-reproducible regression ("my table pasted as text").
|
||||
console.error(
|
||||
"markdown paste conversion failed, inserting raw text",
|
||||
err,
|
||||
);
|
||||
if (view.isDestroyed) return;
|
||||
const tr = view.state.tr;
|
||||
// Same guard the success path uses: if the doc changed under us
|
||||
|
||||
@@ -238,8 +238,9 @@ function convertReferenceFootnotes(markdown: string): string {
|
||||
*
|
||||
* LINE-ANCHORED (the same shape the canonical parser uses in
|
||||
* prosemirror-markdown/page-file.ts): the block opens only on `---\n` at the
|
||||
* very start and closes only on a `\n---` line. The retired `markdownToHtml`
|
||||
* strip closed on the FIRST `---` ANYWHERE (an unanchored close), so a value
|
||||
* very start and closes only on a `\n---` line. The retired editor-ext
|
||||
* `markdownToHtml` front-matter strip (removed in #347) closed on the FIRST
|
||||
* `---` ANYWHERE (an unanchored close), so a value
|
||||
* containing a triple-dash (e.g. `title: Q1 --- Q2`) truncated the front-matter
|
||||
* and leaked the rest into the body. An optional leading BOM is tolerated.
|
||||
*/
|
||||
|
||||
@@ -14,7 +14,8 @@ import {
|
||||
* ProseMirror JSON directly (never running the editor's plugins), so the
|
||||
* canonical footnote topology was never enforced on those writes. The consumers
|
||||
* of this editor-ext copy are: the server markdown/HTML import
|
||||
* (`markdownToHtml -> htmlToJson` in import.service / file-import-task.service),
|
||||
* (`markdownToProseMirror` from @docmost/prosemirror-markdown in import.service /
|
||||
* file-import-task.service),
|
||||
* `PageService` create/update (`parseProsemirrorContent` for the JSON/markdown/
|
||||
* HTML REST write paths), and the client markdown PASTE path
|
||||
* (`markdown-clipboard.ts`). (The MCP package mirrors this canonicalizer in
|
||||
|
||||
@@ -103,8 +103,9 @@ interface CollisionPlan {
|
||||
* `X__2`, `X__3`, collision-bumped) so it survives as a distinct footnote — which,
|
||||
* having no matching reference, then falls under the normal orphan policy. It is
|
||||
* only ever dropped for lacking a reference, never for colliding. The IMPORT
|
||||
* paths (footnote.marked.ts / MCP extractFootnotes) instead apply first-wins +
|
||||
* drop + warn for duplicate definitions; that divergence is intentional — import
|
||||
* paths (@docmost/prosemirror-markdown / MCP extractFootnotes) instead apply
|
||||
* first-wins + drop + warn for duplicate definitions; that divergence is
|
||||
* intentional — import
|
||||
* is an agent-authored artifact we sanitize, the editor is live user data we must
|
||||
* not lose.
|
||||
*
|
||||
|
||||
@@ -6,8 +6,9 @@ import { deriveFootnoteId } from "./footnote-util";
|
||||
*
|
||||
* `deriveFootnoteId` lives ONLY in editor-ext now — it is used by
|
||||
* `resolveCollisions` (re-id of a duplicate definition) and `footnotePastePlugin`
|
||||
* (re-id of a pasted colliding definition). The MCP/marked import paths no longer
|
||||
* derive ids (duplicate definitions there are first-wins-dropped, #166), so there
|
||||
* (re-id of a pasted colliding definition). The MCP / @docmost/prosemirror-markdown
|
||||
* import paths no longer derive ids (duplicate definitions there are
|
||||
* first-wins-dropped, #166), so there
|
||||
* is no cross-package copy and no parity test to keep in sync. This table pins the
|
||||
* deterministic scheme so a future change to it is a conscious one.
|
||||
*/
|
||||
|
||||
@@ -63,8 +63,9 @@ export function generateFootnoteId(): string {
|
||||
* its own seen-set before requesting the next derived id.
|
||||
*
|
||||
* Used only inside editor-ext now (resolveCollisions for a re-id'd duplicate
|
||||
* DEFINITION, and footnotePastePlugin). The MCP/marked import paths no longer
|
||||
* derive ids — duplicate definitions there are first-wins-dropped (#166) — so
|
||||
* DEFINITION, and footnotePastePlugin). The MCP / @docmost/prosemirror-markdown
|
||||
* import paths no longer derive ids — duplicate definitions there are
|
||||
* first-wins-dropped (#166) — so
|
||||
* there is no cross-package copy to keep in sync. The golden table in
|
||||
* footnote-util.derive-id.test.ts pins the scheme.
|
||||
*/
|
||||
|
||||
@@ -1,12 +1,31 @@
|
||||
# @docmost/prosemirror-markdown
|
||||
|
||||
The single, canonical **ProseMirror ↔ Markdown converter** plus the Docmost
|
||||
schema mirror (#293/#345). Headless and framework-free: no React, no browser
|
||||
runtime. There is exactly ONE copy of this converter in the repo, consumed by:
|
||||
schema mirror (#293/#345/#347). Headless and framework-free: no React. There is
|
||||
exactly ONE copy of this converter in the repo, consumed by:
|
||||
|
||||
- `packages/mcp` (the MCP server),
|
||||
- `packages/git-sync` (two-way Git sync),
|
||||
- `apps/server` (server-side markdown import/export, #345).
|
||||
- `apps/server` (server-side markdown import/export, #345),
|
||||
- `apps/client` (markdown paste/copy + AI-chat render, #347).
|
||||
|
||||
### Node vs browser entry
|
||||
|
||||
The HTML→DOM stage of markdown import runs on `jsdom` in Node and the native
|
||||
`DOMParser` in the browser, injected per environment so **jsdom never enters a
|
||||
client bundle**:
|
||||
|
||||
- default entry (`@docmost/prosemirror-markdown`) — Node: registers jsdom +
|
||||
`@tiptap/html`'s happy-dom `server` `generateJSON`. Used by mcp / git-sync /
|
||||
apps/server.
|
||||
- `browser` entry (`@docmost/prosemirror-markdown/browser`, via the `"browser"`
|
||||
exports condition) — registers the native `DOMParser` + `@tiptap/html`'s
|
||||
browser `generateJSON`. Used by `apps/client`; carries no jsdom/happy-dom.
|
||||
|
||||
Both entries expose the identical converter surface; only the injected
|
||||
DOM/`generateJSON` implementations differ (`src/lib/dom-parser.ts`). A
|
||||
`markdownToProseMirrorSync` variant exists for callers that cannot await (the
|
||||
client's synchronous chat renderer).
|
||||
|
||||
`src/lib/docmost-schema.ts` **mirrors** the upstream Tiptap schema that lives in
|
||||
`packages/editor-ext`. The mirror is not free-floating: `serializer-contract.test.ts`
|
||||
|
||||
@@ -63,10 +63,9 @@ function getStyleProperty(element: HTMLElement, propertyName: string): string |
|
||||
* The editor SCHEMA genuinely only supports these six banner types — there is no
|
||||
* `tip`/`caution`/`important`/`question` callout node. So those are NOT first-
|
||||
* class types we can round-trip literally; they are INPUT ALIASES (GitHub/Obsidian
|
||||
* alert syntax). The editor's own paste/import path maps them onto the supported
|
||||
* set (see `GITHUB_ALERT_TYPE_MAP` in
|
||||
* `@docmost/editor-ext` markdown/utils/github-callout.marked.ts:
|
||||
* tip -> success, caution -> danger, important -> info). We mirror that aliasing
|
||||
* alert syntax). This package's own `> [!type]` import path maps them onto the
|
||||
* supported set (see `CALLOUT_TYPE_ALIASES` below: tip -> success, caution ->
|
||||
* danger, important -> info). We apply that aliasing
|
||||
* here so an ingested `> [!tip]` / `> [!caution]` lands on the closest real banner
|
||||
* (success / danger) instead of flatly collapsing to `info` — matching exactly how
|
||||
* the editor itself would interpret the same alias. A schema type always maps to
|
||||
@@ -75,11 +74,11 @@ function getStyleProperty(element: HTMLElement, propertyName: string): string |
|
||||
*/
|
||||
const CALLOUT_TYPES = ["default", "info", "note", "success", "warning", "danger"];
|
||||
/**
|
||||
* NON-schema callout aliases -> their closest supported banner. Mirrors the
|
||||
* editor's `GITHUB_ALERT_TYPE_MAP` for the names that are NOT already schema
|
||||
* types (a schema type is preserved as-is and never consulted here). Keeping
|
||||
* these in lockstep means git-sync ingest and an editor paste interpret the same
|
||||
* `> [!alias]` identically.
|
||||
* NON-schema callout aliases -> their closest supported banner, for the names
|
||||
* that are NOT already schema types (a schema type is preserved as-is and never
|
||||
* consulted here). This is the single canonical alias map now that the editor's
|
||||
* old marked layer is gone; git-sync ingest and an editor paste both go through
|
||||
* this package, so they interpret the same `> [!alias]` identically.
|
||||
*/
|
||||
const CALLOUT_TYPE_ALIASES: Record<string, string> = {
|
||||
tip: "success",
|
||||
|
||||
Reference in New Issue
Block a user