fix(core): repair invalid content before mounting - #8144
Open
bdbch wants to merge 3 commits into
Open
Conversation
✅ Deploy Preview for tiptap-embed ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
🦋 Changeset detectedLatest commit: ee8de71 The changes in this PR will be included in the next version bump. This PR includes changesets to release 74 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 |
Contributor
📝 WalkthroughSummary
WalkthroughAdds schema-aware repair for invalid ProseMirror content, applies it before editor initialization, exports the helper, and adds tests for malformed nodes, marks, nested documents, and content-error handling. ChangesInvalid content repair
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Editor
participant ProseMirrorNode
participant repairNode
participant EditorState
Editor->>ProseMirrorNode: Create fallback document
Editor->>ProseMirrorNode: Check schema validity
Editor->>repairNode: Repair invalid content
repairNode-->>Editor: Return schema-valid document
Editor->>EditorState: Initialize with repaired document
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
coderabbitai
Bot
added
area: core
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/core/__tests__/repairNode.spec.ts (1)
1-207: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNo test coverage for the "repair fails entirely" fallback path. Both files test only the repairable cases; neither exercises
repairNodereturningnullor the resulting Editor empty-document fallback, despite this being an explicit PR objective.
packages/core/__tests__/repairNode.spec.ts#L1-L207: add a unit test with a node type whose content can never satisfyfillBefore(e.g. a required child type with no valid default fill), assertingrepairNode(...)returnsnull.packages/core/__tests__/onContentError.spec.ts#L286-L341: add an integration test where content repair cannot succeed, asserting the Editor falls back to an empty document (viathis.schema.topNodeType.createAndFill()) instead of crashing.🤖 Prompt for 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. In `@packages/core/__tests__/repairNode.spec.ts` around lines 1 - 207, Add unit coverage in packages/core/__tests__/repairNode.spec.ts (lines 1-207) using a node type whose required content cannot be satisfied by fillBefore, and assert repairNode returns null. Add integration coverage in packages/core/__tests__/onContentError.spec.ts (lines 286-341) with unrecoverable content, asserting the Editor falls back to an empty document via schema.topNodeType.createAndFill() instead of throwing.Source: Coding guidelines
packages/core/src/Editor.ts (1)
543-571: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winSkip the second full tree check when
enableContentCheckis already enabled.
enableContentCheck: truepasseserrorOnInvalidContent: truetocreateDocument, and JSON content already callsnode.check()there. For that path,repairInvalidDoc(doc)adds another full-tree traversal before editing starts. Avoid the duplicate check for JSON content when the content option is enabled; for invalid JSON, this path is bypassed by the existing catch branch.🤖 Prompt for 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. In `@packages/core/src/Editor.ts` around lines 543 - 571, Avoid calling repairInvalidDoc for JSON content when enableContentCheck is enabled, since createDocument already validates it with node.check(). Update the document initialization flow around repairInvalidDoc to bypass this redundant traversal while preserving the existing catch behavior for invalid JSON and repair logic for other content paths.
🤖 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.
Nitpick comments:
In `@packages/core/__tests__/repairNode.spec.ts`:
- Around line 1-207: Add unit coverage in
packages/core/__tests__/repairNode.spec.ts (lines 1-207) using a node type whose
required content cannot be satisfied by fillBefore, and assert repairNode
returns null. Add integration coverage in
packages/core/__tests__/onContentError.spec.ts (lines 286-341) with
unrecoverable content, asserting the Editor falls back to an empty document via
schema.topNodeType.createAndFill() instead of throwing.
In `@packages/core/src/Editor.ts`:
- Around line 543-571: Avoid calling repairInvalidDoc for JSON content when
enableContentCheck is enabled, since createDocument already validates it with
node.check(). Update the document initialization flow around repairInvalidDoc to
bypass this redundant traversal while preserving the existing catch behavior for
invalid JSON and repair logic for other content paths.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: ea26df1d-016f-4bb4-9092-57efb89ae200
📒 Files selected for processing (6)
.changeset/2026-07-30-repair-invalid-initial-content.mdpackages/core/__tests__/onContentError.spec.tspackages/core/__tests__/repairNode.spec.tspackages/core/src/Editor.tspackages/core/src/helpers/index.tspackages/core/src/helpers/repairNode.ts
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
1 participant
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
Changes and Review
This PR fixes a bug where content passed into the editor content could lock up clients when the content is not valid, even when the content was checked via
enableContentCheck.Now the content will be repaired when content is invalid. This is happening by going through node children and wrapping/unwrapping invalid childs + repairing and deduping marks. This happens only on the editor constructor - we may extend this logic to other situations if needed.
Unknown content will still be dropped. Just salvaging unknown content and turning it back into the default node type is itself lossy and could destroy important nodes and attributes.
Checklist
Responsibility
(copy of #8143 which was accidentially closed)