fix: React custom blocks cause infinite rerender on mobile (BLO-1212)#2830
fix: React custom blocks cause infinite rerender on mobile (BLO-1212)#2830matthewlipski wants to merge 2 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds an ChangesReact node-view mutation filtering
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
@blocknote/ariakit
@blocknote/code-block
@blocknote/core
@blocknote/mantine
@blocknote/react
@blocknote/server-util
@blocknote/shadcn
@blocknote/xl-ai
@blocknote/xl-docx-exporter
@blocknote/xl-email-exporter
@blocknote/xl-multi-column
@blocknote/xl-odt-exporter
@blocknote/xl-pdf-exporter
commit: |
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/react/src/schema/ReactBlockSpec.tsx (1)
318-350: 🏗️ Heavy liftAdd a mobile regression test for mutation-type filtering.
Lines [318-350] implement behavior that is sensitive to mutation type and target; without regression coverage, this is easy to break. Add a test that asserts wrapper
attributesmutations are ignored, while contentchildList/characterDatamutations are not ignored.🤖 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/react/src/schema/ReactBlockSpec.tsx` around lines 318 - 350, Add a regression test for the ReactBlockSpec.ignoreMutation behavior: create a test that constructs a node-view using the ReactBlockSpec (or the same block spec used in tests) and simulates mutations against the wrapper element and the inner editable content; assert that an attributes mutation on the editable wrapper returns true (ignored), while childList and characterData mutations on the content return false (not ignored). Target the ignoreMutation function in ReactBlockSpec.tsx and exercise cases for mutation.type === "selection" (should return false), an attributes mutation with mutation.target === content wrapper (true), and content childList/characterData mutations where mutation.target is inside [data-node-view-content] (false).
🤖 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/react/src/schema/ReactBlockSpec.tsx`:
- Around line 343-347: In ReactBlockSpec.tsx inside the ignoreMutation
implementation (the ignoreMutation function used by the ReactNodeView), stop
returning true for all mutation types when mutation.target === content; instead
only ignore wrapper attribute changes by checking that mutation.type ===
"attributes" && mutation.target === content before returning true so
childList/textInput mutations on the [data-node-view-content] element are not
suppressed.
---
Nitpick comments:
In `@packages/react/src/schema/ReactBlockSpec.tsx`:
- Around line 318-350: Add a regression test for the
ReactBlockSpec.ignoreMutation behavior: create a test that constructs a
node-view using the ReactBlockSpec (or the same block spec used in tests) and
simulates mutations against the wrapper element and the inner editable content;
assert that an attributes mutation on the editable wrapper returns true
(ignored), while childList and characterData mutations on the content return
false (not ignored). Target the ignoreMutation function in ReactBlockSpec.tsx
and exercise cases for mutation.type === "selection" (should return false), an
attributes mutation with mutation.target === content wrapper (true), and content
childList/characterData mutations where mutation.target is inside
[data-node-view-content] (false).
🪄 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
Run ID: 715e9430-fa98-4470-b143-703543153e9f
📒 Files selected for processing (1)
packages/react/src/schema/ReactBlockSpec.tsx
nperez0111
left a comment
There was a problem hiding this comment.
Let's double check with Tiptap's new React Renderer, just to make sure we aren't fixing something that is fixed.
Summary
This PR fixes an issue with React custom blocks sometimes causing infinite re-render loops on mobile. This can be seen most easily with the Alert Block with Full UX example, where attempting to insert an alert block via the slash menu will result in an infinite re-render loop and crash. This can be replicated via the mobile emulator in Chrome dev tools.
The reason this happens is that the way ProseMirror detects when a block's editable content has changed is different on desktop vs mobile. On desktop, it's detected using key events within the editable content wrapper element. On mobile though, key inputs don't use actually use typical key events, but insert characters using IME composition (see here and here). Therefore, ProseMirror instead detects changes in the editable content based on DOM mutations. Yet for some reason, re-renders are triggered by DOM mutations anywhere in the block, not just within its editable content.
And so the problem is that React's own rendering also causes DOM mutations, leading to ProseMirror triggering a re-render, causing React to render the block again, and starting an infinite loop. Therefore, we need to explicitly tell ProseMirror to ignore mutations outside the editable content to avoid this from happening.
Closes #2804
Rationale
This is a severe issue on mobile.
Changes
ignoreMutationtocreateReactBlockSpecto ignore DOM mutations outside of block's editable content.Impact
N/A
Testing
N/A (we don't have testing infrastructure on mobile)
Screenshots/Video
N/A
Checklist
Additional Notes
N/A
Summary by CodeRabbit