From 655c60a84634343bb243ba052dd4d153cd8627dd Mon Sep 17 00:00:00 2001 From: Kendall Ernst Date: Tue, 21 Apr 2026 18:49:17 -0700 Subject: [PATCH 1/3] fix(paragraph): guard listRendering destructure against null MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ParagraphNodeView destructured `{ suffix, justification }` from `this.node.attrs.listRendering` without a null-check in #updateListStyles, and accessed `listRendering.markerText`/`.suffix` directly in #initList. When a paragraph node carried `listRendering: null` — which can happen after certain editor mutations (e.g. dispatching a transaction that combines `setDocAttribute('bodySectPr', …)` with a paragraph delete) — the post-transaction list-styles re-pass threw: TypeError: Cannot destructure property 'suffix' of 'this.node.attrs.listRendering' as it is null Use `?? {}` and optional chaining so a null value falls back through the existing defaults (`suffix ?? 'tab'` and the `suffix == null` branch in #createSeparator). Adds a regression test. --- .../extensions/paragraph/ParagraphNodeView.js | 6 +++--- .../paragraph/ParagraphNodeView.test.js | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/packages/super-editor/src/editors/v1/extensions/paragraph/ParagraphNodeView.js b/packages/super-editor/src/editors/v1/extensions/paragraph/ParagraphNodeView.js index 94b3717102..e18ef01c9d 100644 --- a/packages/super-editor/src/editors/v1/extensions/paragraph/ParagraphNodeView.js +++ b/packages/super-editor/src/editors/v1/extensions/paragraph/ParagraphNodeView.js @@ -232,7 +232,7 @@ export class ParagraphNodeView { * @returns {boolean} */ #updateListStyles() { - let { suffix, justification } = this.node.attrs.listRendering; + let { suffix, justification } = this.node.attrs.listRendering ?? {}; suffix = suffix ?? 'tab'; this.#calculateMarkerStyle(justification); if (suffix === 'tab') { @@ -283,8 +283,8 @@ export class ParagraphNodeView { * @param {{ markerText: string, suffix?: string }} listRendering */ #initList(listRendering) { - this.#createMarker(listRendering.markerText); - this.#createSeparator(listRendering.suffix); + this.#createMarker(listRendering?.markerText); + this.#createSeparator(listRendering?.suffix); } #checkIsList() { diff --git a/packages/super-editor/src/editors/v1/extensions/paragraph/ParagraphNodeView.test.js b/packages/super-editor/src/editors/v1/extensions/paragraph/ParagraphNodeView.test.js index 4575db07f8..aefd80714d 100644 --- a/packages/super-editor/src/editors/v1/extensions/paragraph/ParagraphNodeView.test.js +++ b/packages/super-editor/src/editors/v1/extensions/paragraph/ParagraphNodeView.test.js @@ -199,6 +199,25 @@ describe('ParagraphNodeView', () => { expect(nodeView.separator.textContent).toBe('\u00A0'); }); + it('does not throw when listRendering is null', () => { + // Regression: #updateListStyles destructured `{ suffix, justification }` + // from `this.node.attrs.listRendering` without a null-check, throwing + // `TypeError: Cannot destructure property 'suffix' of ... as it is null` + // whenever a paragraph node carried `listRendering: null`. + isList.mockReturnValue(true); + const baseAttrs = createNode().attrs; + const { nodeView } = mountNodeView({ attrs: { ...baseAttrs } }); + + const nextNode = createNode({ + attrs: { + ...baseAttrs, + listRendering: null, + }, + }); + + expect(() => nodeView.update(nextNode, [])).not.toThrow(); + }); + it('uses hanging indent width for right-justified tabs and skips tab helper', () => { isList.mockReturnValue(true); const attrs = { From 030734303ad4e355db86daaa1fb7c181cf6923e0 Mon Sep 17 00:00:00 2001 From: Kendall Ernst Date: Tue, 21 Apr 2026 19:05:18 -0700 Subject: [PATCH 2/3] fix(paragraph): no-op list style updates when listRendering is null MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously the null-guarded path fell back to `suffix = 'tab'` and still invoked `#calculateMarkerStyle`/`#calculateTabSeparatorStyle`. Reviewer (codex-connector) flagged that in mixed-suffix updates — where a queued RAF callback runs after a node transitions from `suffix: 'space'` to `listRendering: null` — the separator may still be a Text node. Writing `this.separator.style.cssText` on a Text node throws. Change #updateListStyles and #initList to return early when `listRendering` is null, leaving the existing marker/separator untouched. Future updates (when the node gets a real listRendering or isList() returns false) will clean up as before. Adds a regression test covering the space→null transition. --- .../extensions/paragraph/ParagraphNodeView.js | 21 +++++++++++++--- .../paragraph/ParagraphNodeView.test.js | 24 +++++++++++++++++++ 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/packages/super-editor/src/editors/v1/extensions/paragraph/ParagraphNodeView.js b/packages/super-editor/src/editors/v1/extensions/paragraph/ParagraphNodeView.js index e18ef01c9d..b1c3f4963d 100644 --- a/packages/super-editor/src/editors/v1/extensions/paragraph/ParagraphNodeView.js +++ b/packages/super-editor/src/editors/v1/extensions/paragraph/ParagraphNodeView.js @@ -232,7 +232,16 @@ export class ParagraphNodeView { * @returns {boolean} */ #updateListStyles() { - let { suffix, justification } = this.node.attrs.listRendering ?? {}; + const listRendering = this.node.attrs.listRendering; + // When listRendering is null (can happen transiently during certain + // transactions, e.g. after a setDocAttribute + paragraph delete), leave + // the existing marker/separator untouched. Forcing a default `suffix` here + // would risk writing tab-style CSS onto a text-node separator created by + // a prior 'space'/'nothing' suffix and scheduled RAF pass. + if (!listRendering) { + return true; + } + let { suffix, justification } = listRendering; suffix = suffix ?? 'tab'; this.#calculateMarkerStyle(justification); if (suffix === 'tab') { @@ -283,8 +292,14 @@ export class ParagraphNodeView { * @param {{ markerText: string, suffix?: string }} listRendering */ #initList(listRendering) { - this.#createMarker(listRendering?.markerText); - this.#createSeparator(listRendering?.suffix); + // See #updateListStyles: when listRendering is null the previous marker/ + // separator are left in place; avoid invoking the create helpers with + // undefined values. + if (!listRendering) { + return; + } + this.#createMarker(listRendering.markerText); + this.#createSeparator(listRendering.suffix); } #checkIsList() { diff --git a/packages/super-editor/src/editors/v1/extensions/paragraph/ParagraphNodeView.test.js b/packages/super-editor/src/editors/v1/extensions/paragraph/ParagraphNodeView.test.js index aefd80714d..d80a328191 100644 --- a/packages/super-editor/src/editors/v1/extensions/paragraph/ParagraphNodeView.test.js +++ b/packages/super-editor/src/editors/v1/extensions/paragraph/ParagraphNodeView.test.js @@ -218,6 +218,30 @@ describe('ParagraphNodeView', () => { expect(() => nodeView.update(nextNode, [])).not.toThrow(); }); + it('does not try to style a text-node separator when switching to null listRendering', () => { + // Regression: when transitioning from a 'space'/'nothing' suffix (which + // creates a text-node separator) to `listRendering: null`, the null-guarded + // path must not fall back to the 'tab' branch, since writing + // `this.separator.style.cssText` on a Text node throws. + isList.mockReturnValue(true); + const spaceAttrs = { + ...createNode().attrs, + listRendering: { suffix: 'space', justification: 'left', markerText: '1.' }, + }; + const { nodeView } = mountNodeView({ attrs: spaceAttrs }); + // The separator should be a Text node under the 'space' suffix. + expect(nodeView.separator?.nodeType).toBe(Node.TEXT_NODE); + const textSeparator = nodeView.separator; + + const nullNode = createNode({ + attrs: { ...spaceAttrs, listRendering: null }, + }); + + expect(() => nodeView.update(nullNode, [])).not.toThrow(); + // The text-node separator must be left alone (not replaced, not styled). + expect(nodeView.separator).toBe(textSeparator); + }); + it('uses hanging indent width for right-justified tabs and skips tab helper', () => { isList.mockReturnValue(true); const attrs = { From 86da5c89540c461dcb1f4088146f7eac00a3d2ca Mon Sep 17 00:00:00 2001 From: Kendall Ernst Date: Wed, 22 Apr 2026 08:50:41 -0700 Subject: [PATCH 3/3] test(paragraph): cover constructor mount and null-to-tab recovery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses review feedback on #2896: - Update #initList JSDoc `@param` to include `| null`, matching the no-op-on-null behavior added in the previous commit. - Add a test that mounts a ParagraphNodeView with `listRendering: null` (the constructor path, not just update()), confirming the null guards in #initList and #updateListStyles cover first-render too. - Add a test for the space→null→tab transition that verifies the separator swaps from a text node back to a span when listRendering returns with a different suffix. --- .../extensions/paragraph/ParagraphNodeView.js | 2 +- .../paragraph/ParagraphNodeView.test.js | 33 +++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/packages/super-editor/src/editors/v1/extensions/paragraph/ParagraphNodeView.js b/packages/super-editor/src/editors/v1/extensions/paragraph/ParagraphNodeView.js index b1c3f4963d..b5156a4586 100644 --- a/packages/super-editor/src/editors/v1/extensions/paragraph/ParagraphNodeView.js +++ b/packages/super-editor/src/editors/v1/extensions/paragraph/ParagraphNodeView.js @@ -289,7 +289,7 @@ export class ParagraphNodeView { } /** - * @param {{ markerText: string, suffix?: string }} listRendering + * @param {{ markerText: string, suffix?: string } | null} listRendering */ #initList(listRendering) { // See #updateListStyles: when listRendering is null the previous marker/ diff --git a/packages/super-editor/src/editors/v1/extensions/paragraph/ParagraphNodeView.test.js b/packages/super-editor/src/editors/v1/extensions/paragraph/ParagraphNodeView.test.js index d80a328191..08376ef5d7 100644 --- a/packages/super-editor/src/editors/v1/extensions/paragraph/ParagraphNodeView.test.js +++ b/packages/super-editor/src/editors/v1/extensions/paragraph/ParagraphNodeView.test.js @@ -242,6 +242,39 @@ describe('ParagraphNodeView', () => { expect(nodeView.separator).toBe(textSeparator); }); + it('does not throw when mounted with listRendering null', () => { + // Regression: the null guards in #initList and #updateListStyles must also + // cover the constructor path — mounting a paragraph whose listRendering is + // already null previously threw before update() ever ran. + isList.mockReturnValue(true); + const nullAttrs = { ...createNode().attrs, listRendering: null }; + expect(() => mountNodeView({ attrs: nullAttrs })).not.toThrow(); + }); + + it('recovers marker/separator when listRendering returns from null to tab', () => { + // Regression: the null-guarded path leaves the existing marker/separator in + // place. When listRendering clears and later returns with a different suffix + // (here: space → null → tab), the separator has to swap from a text node + // back to a span element — #createSeparator handles this only if the + // recovery path actually runs, so exercise it end-to-end. + isList.mockReturnValue(true); + const spaceAttrs = { + ...createNode().attrs, + listRendering: { suffix: 'space', justification: 'left', markerText: '1.' }, + }; + const { nodeView } = mountNodeView({ attrs: spaceAttrs }); + + nodeView.update(createNode({ attrs: { ...spaceAttrs, listRendering: null } }), []); + + const tabNode = createNode({ + attrs: { ...spaceAttrs, listRendering: { suffix: 'tab', justification: 'left', markerText: '2.' } }, + }); + nodeView.update(tabNode, []); + + expect(nodeView.marker?.textContent).toBe('2.'); + expect(nodeView.separator?.tagName?.toLowerCase()).toBe('span'); + }); + it('uses hanging indent width for right-justified tabs and skips tab helper', () => { isList.mockReturnValue(true); const attrs = {