fix(markdown): never parse to an empty document (fixes setContent crash) - #8017
Open
codewithsupra wants to merge 10 commits into
Open
fix(markdown): never parse to an empty document (fixes setContent crash)#8017codewithsupra wants to merge 10 commits into
codewithsupra wants to merge 10 commits into
Conversation
Fixes ueberdosis#7914 MarkdownManager.parse() could return { type: 'doc', content: [] } for markdown that yields no renderable blocks — whitespace-only input, or input whose only token has no registered handler. A common trigger: a line of leading whitespace followed by text (e.g. ' / ') is tokenized by marked as an indented code block (4+ leading spaces per CommonMark); with no code-block extension registered, that token produces nothing, leaving empty content. A doc node requires block+, so the empty document made setContent throw 'RangeError: Invalid content for node doc: <>'. parse() now falls back to a single empty paragraph when parseTokens yields no blocks, matching how an empty markdown string is represented — so no input string can crash the editor. Adds regression tests (whitespace-only, whitespace+slash, whitespace+backslash, empty string all yield a valid one-paragraph doc; setContent no longer throws; meaningful single-char content still parses). Verified the parse tests fail with the fix reverted. Full markdown + core suites (713 tests) pass.
🦋 Changeset detectedLatest commit: f8e66e4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 72 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for tiptap-embed ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
arnaugomez
approved these changes
Jul 16, 2026
arnaugomez
requested changes
Jul 16, 2026
Comment on lines
+351
to
+358
| // A `doc` node requires at least one block child (`block+`), so an empty | ||
| // `content` array produces an invalid document that makes `setContent` | ||
| // throw `RangeError: Invalid content for node doc: <>`. This happens for | ||
| // input that yields no renderable blocks — whitespace-only markdown, or | ||
| // markdown whose only token has no registered handler (e.g. an indented | ||
| // code block when no code-block extension is present, as with a line of | ||
| // leading whitespace followed by text). Fall back to a single empty | ||
| // paragraph, matching how an empty markdown string is represented. |
Contributor
There was a problem hiding this comment.
Move this comment to just before "content", and re-write it to make it shorter.
| // Convert tokens to Tiptap JSON | ||
| const content = this.parseTokens(tokens, true) | ||
|
|
||
| // Return a document node containing the parsed content |
Contributor
There was a problem hiding this comment.
Do not delete this comment
Co-authored-by: Arnau Gómez Farell <arnaugomez@users.noreply.github.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough
ChangesMarkdown empty document fallback
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/markdown/__tests__/empty-doc-fallback.spec.ts`:
- Around line 31-40: Update the test covering whitespace-plus-slash markdown to
initialize an empty Editor first, then invoke editor.commands.setContent with
the markdown content and contentType: 'markdown'. Keep the no-throw assertion
and paragraph JSON validation focused on this command path rather than
constructor initialization.
In `@packages/markdown/src/MarkdownManager.ts`:
- Around line 354-355: Update the document content construction in
MarkdownManager to reuse parsed content only when it contains at least one node
recognized by the registered schema as a block type, rather than checking
content.length alone. Otherwise, use the existing paragraph fallback, and add a
regression case covering an unsupported top-level token that produces text
content.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c81410a3-d9d2-4fbd-b1f2-ebfcc5ad4933
📒 Files selected for processing (3)
.changeset/fix-markdown-empty-doc-fallback.mdpackages/markdown/__tests__/empty-doc-fallback.spec.tspackages/markdown/src/MarkdownManager.ts
arnaugomez
previously approved these changes
Jul 16, 2026
…ercise setContent command path in tests Addresses CodeRabbit review on ueberdosis#8017: - The empty-doc fallback checked content.length > 0, which doesn't guarantee the doc's block+ requirement — a custom token handler returning a bare top-level {type:'text'} node would pass the check and still crash setContent. Now checks that at least one top-level node is actually a registered block node type. - Derives block-node names per-extension instead of via getSchema(), which requires the full extension set to form one valid ProseMirror schema; that's stricter than this check needs and would silently discard valid content if unrelated extensions conflict. - Added a setContent-via-commands regression test alongside the existing constructor-init one, and a regression test/fix pair for the bare-text-node case using a custom extension.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/markdown/src/MarkdownManager.ts`:
- Around line 1084-1106: Update the block-node cache construction around
schemaBlockNodeNamesCache to derive names from this.extensions so directly
registered block extensions are included, while preserving the existing node and
inline/topNode filtering. In registerExtension(), invalidate
this.schemaBlockNodeNamesCache after updating this.extensions, and add a
regression test covering directly registered block content without introducing
the empty paragraph fallback.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 460432a7-9665-4c3c-86c5-247daf57234b
📒 Files selected for processing (2)
packages/markdown/__tests__/empty-doc-fallback.spec.tspackages/markdown/src/MarkdownManager.ts
…ension in the block-node cache Addresses CodeRabbit review on ueberdosis#8017. schemaBlockNodeNamesCache only read this.baseExtensions (the constructor's initial list), so a block extension added later via the public registerExtension() method would have its content wrongly replaced by the empty-paragraph fallback. Read this.extensions instead (every extension ever registered), and invalidate the cache whenever registerExtension() runs.
content.some(node => ...) yields node.type as string | undefined (JSONContent.type is optional); Set<string>.has() requires a string argument. CI's dts build (stricter than local tsc --noEmit here) caught what plain vitest runs didn't.
arnaugomez
reviewed
Jul 16, 2026
| * set), and treating that failure as "no block nodes exist" would discard | ||
| * otherwise-correct parsed content. | ||
| */ | ||
| private getSchemaBlockNodeNames(): Set<string> { |
Contributor
There was a problem hiding this comment.
Shouldn't this function check for the group: "block" property of the Node extension?
…text' test
The extension/schema-derived block-node check broke on a case I found
by simulating CI's merge of this branch with the latest main: the
'heading' case in parseToken's switch statement builds a
{ type: 'heading' } node directly, independent of whether a Heading
extension is registered. A manager built with only
[Document, Paragraph, Text] correctly returns a heading node for a
setext heading — but the extension-derived block-name set had no
entry for 'heading' (no such extension was registered), so the
fallback wrongly discarded it and replaced it with an empty paragraph.
The actual invariant only needs to rule out one concrete shape: a bare
top-level `text` node, which is the only inline type parseToken ever
returns unwrapped (the 'text' and 'escape' cases). Checking that
directly removes the whole extension/schema-introspection path
(getSchemaBlockNodeNames, its cache, and the registerExtension
invalidation) along with its failure modes, and is correct for every
case already covered: the original whitespace/slash bug, a custom
handler returning bare text, a directly-registered block extension,
and now a hardcoded-case node type with no matching extension.
arnaugomez
previously approved these changes
Jul 16, 2026
Contributor
|
I added these fixes:
|
arnaugomez
approved these changes
Jul 16, 2026
1 task
This was referenced Jul 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes
Fixes #7914
Changes and Review
parse()could return adocwith no valid block content for certain inputs, which crashessetContent.Checklist
Responsibility