fix(export): stop comment.renderHTML returning a live jsdom node on the server (#298)

Page/space export (Markdown & HTML, both via jsonToHtml -> generateHTML) crashed with
"Export failed:undefined" on any page carrying a `comment` mark. Root cause:
comment.renderHTML returned a LIVE DOM node (document.createElement + a click listener)
whenever a global `document` existed — and the in-process MCP module injects a jsdom
global.window+global.document into the Node server, defeating the old
`typeof document === "undefined"` guard. The server export runs happy-dom's
DOMSerializer, which crashes appending the foreign jsdom node
(NodeUtility.isInclusiveAncestor -> "Cannot read properties of undefined (reading
'length')"). comment is the only extension returning a live node.

Fix: widen the guard with an isNodeRuntime check (process.versions.node) so on any Node
runtime renderHTML returns the plain, serializable spec array — even when MCP injected
jsdom globals. The browser branch (createElement + click -> ACTIVE_COMMENT_EVENT) is
untouched, so in-editor comment interactivity is preserved (Vite defines only
process.env as a member-expression substitution, no `process` object in the browser
bundle, so isNodeRuntime is false there). The mcp schema mirror already returns a spec
array and is not on the export path (tiptapExtensions imports Comment from
@docmost/editor-ext), so no mirror change is needed.

Also: export-modal now reads the real error text from the response Blob
(responseType:'blob' made err.response.data.message always undefined) so a failed export
shows the server's message instead of "undefined".

Adds a regression test that runs the real jsonToHtml on a comment-marked doc with
jsdom globals injected (reproduces the crash on the unpatched code, passes after).

closes #298

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
agent_coder
2026-07-03 01:34:53 +03:00
parent d89650a45e
commit 3f7e1bdc7b
3 changed files with 114 additions and 2 deletions
@@ -14,6 +14,22 @@ import { notifications } from "@mantine/notifications";
import { exportSpace } from "@/features/space/services/space-service";
import { useTranslation } from "react-i18next";
// The export request uses `responseType: "blob"`, so a server error body arrives
// as a Blob rather than parsed JSON — `err.response?.data.message` is therefore
// always undefined. Read and parse the blob to surface the real error message.
async function extractExportError(err: any): Promise<string> {
const data = err?.response?.data;
if (data instanceof Blob) {
try {
const json = JSON.parse(await data.text());
return json?.message ?? "";
} catch {
return "";
}
}
return data?.message ?? err?.message ?? "";
}
interface ExportModalProps {
id: string;
type: "space" | "page";
@@ -52,8 +68,9 @@ export default function ExportModal({
});
onClose();
} catch (err) {
const message = await extractExportError(err);
notifications.show({
message: "Export failed:" + err.response?.data.message,
message: t("Export failed") + (message ? `: ${message}` : ""),
color: "red",
});
console.error("export error", err);
@@ -0,0 +1,82 @@
import { JSDOM } from 'jsdom';
import { jsonToHtml } from '../../collaboration/collaboration.util';
/**
* Regression test for issue #298: page/space export (Markdown/HTML) crashes on
* pages that contain inline comments.
*
* The in-process MCP module injects a jsdom `global.window` + `global.document`
* into the Node server (see packages/mcp/src/lib/collaboration.ts). Before the
* fix, the comment mark's `renderHTML` guard was only
* `typeof window === "undefined" || typeof document === "undefined"`, so with
* BOTH jsdom globals present it took the interactive browser branch and returned
* a LIVE jsdom <span> node. The export path serializes via happy-dom's
* DOMSerializer, and appending a foreign jsdom node crashed happy-dom
* ("Cannot read properties of undefined (reading 'length')").
*
* We reproduce the MCP-loaded server by injecting jsdom globals, then export a
* doc containing a comment mark and assert the serialization SUCCEEDS and emits
* the expected serializable <span data-comment-id=... class="comment-mark">.
*
* Non-vacuity: this test only exercises the buggy branch because BOTH jsdom
* `window` AND `document` are set below. If the `isNodeRuntime` condition is
* removed from the guard in packages/editor-ext/src/lib/comment/comment.ts,
* `renderHTML` returns a live jsdom node and `jsonToHtml` throws — this test
* then fails. (In a plain node env without the injected globals the guard's
* `typeof window === "undefined"` clause already short-circuits, so it is the
* injected globals that make this assertion meaningful.)
*/
describe('export with inline comments (issue #298)', () => {
const originalWindow = (global as any).window;
const originalDocument = (global as any).document;
beforeAll(() => {
const dom = new JSDOM('<!DOCTYPE html><html><body></body></html>');
(global as any).window = dom.window;
(global as any).document = dom.window.document;
});
afterAll(() => {
(global as any).window = originalWindow;
(global as any).document = originalDocument;
});
const docWithComment = (resolved: boolean) => ({
type: 'doc',
content: [
{
type: 'paragraph',
content: [
{
type: 'text',
marks: [
{
type: 'comment',
attrs: { commentId: 'c-123', resolved },
},
],
text: 'commented text',
},
],
},
],
});
it('exports a page with an unresolved comment mark without crashing', () => {
let html: string;
expect(() => {
html = jsonToHtml(docWithComment(false));
}).not.toThrow();
expect(html).toContain('data-comment-id="c-123"');
expect(html).toContain('class="comment-mark"');
expect(html).toContain('commented text');
});
it('exports a resolved comment mark with the resolved class/attr', () => {
const html = jsonToHtml(docWithComment(true));
expect(html).toContain('data-comment-id="c-123"');
expect(html).toContain('comment-mark resolved');
expect(html).toContain('data-resolved="true"');
});
});
+14 -1
View File
@@ -172,7 +172,20 @@ export const Comment = Mark.create<ICommentOptions, ICommentStorage>({
const commentId = HTMLAttributes?.["data-comment-id"] || null;
const resolved = HTMLAttributes?.["data-resolved"] || false;
if (typeof window === "undefined" || typeof document === "undefined") {
// The in-process MCP module injects a jsdom `global.document` into the Node
// server, so `typeof document === "undefined"` is not enough to detect SSR.
// On any Node runtime always return a plain, serializable spec array; the
// interactive live-DOM branch below is browser-only. This stops server-side
// HTML/Markdown export (happy-dom DOMSerializer) from appending a foreign
// jsdom node into a happy-dom tree.
const isNodeRuntime =
typeof process !== "undefined" && !!process.versions?.node;
if (
typeof window === "undefined" ||
typeof document === "undefined" ||
isNodeRuntime
) {
return [
"span",
mergeAttributes(this.options.HTMLAttributes, HTMLAttributes, {