fix(editor): use Selection.atStart when clearing selection on blur#3568
fix(editor): use Selection.atStart when clearing selection on blur#3568yashs33244 wants to merge 2 commits into
Conversation
The focus-scopes extension cleared the selection on blur with TextSelection.create(doc, 0). Position 0 resolves to the doc node, which has no inline content, so ProseMirror logged "TextSelection endpoint not pointing into a node with inline content (doc)" on every editor blur (e.g. focusing another form field on the page). Switch to Selection.atStart(doc), which resolves to the first valid cursor position inside an inline-content node, removing the warning while keeping the same intent of moving the cursor to the start. Closes resend#3564
|
@yashs33244 is attempting to deploy a commit to the resend Team on Vercel. A member of the Team first needs to authorize it. |
🦋 Changeset detectedLatest commit: 48998c8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Fixes a ProseMirror warning emitted when clearing selection on blur by using a valid “start of document” selection resolver instead of a TextSelection at position 0.
Changes:
- Replace
TextSelection.create(doc, 0)withSelection.atStart(doc)when clearing selection on blur. - Update existing focus-scope test assertion to validate selection resolves into inline content.
- Add a regression test to ensure no “invalid TextSelection endpoint” warning is logged on blur.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/editor/src/extensions/focus-scopes.ts | Switches blur selection reset to Selection.atStart(doc) to avoid invalid endpoint warnings. |
| packages/editor/src/extensions/focus-scopes.spec.ts | Adjusts assertions around selection start and adds a console.warn regression test. |
| .changeset/blur-selection-atstart.md | Documents the patch-level fix and rationale for the selection change. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 3 files
Confidence score: 4/5
- In
packages/editor/src/extensions/focus-scopes.spec.ts, thenot.toHaveBeenCalledWith(expect.stringContaining(...))check can miss warnings whenconsole.warnis called with multiple arguments, so the test may pass even when the warning behavior regresses—update the assertion to inspect all warn call arguments before merging. - In
packages/editor/src/extensions/focus-scopes.spec.ts,warn.mockRestore()is not guaranteed if an earlier assertion throws, which can leak the spy into later tests and create flaky or misleading failures—move restoration intotry/finallyorafterEachbefore merging.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Address review feedback on the focus-scopes regression test: - Wrap the test body in try/finally so the console.warn spy (and the editor/element it creates) are always cleaned up, even if an earlier assertion throws. Previously a failed assertion would skip warn.mockRestore() and leak the spy into later tests. - Replace toHaveBeenCalledWith(expect.stringContaining(...)) with a scan over warn.mock.calls. The matcher required the full argument list to match, so any extra ProseMirror argument would have produced a false negative. The scan checks every argument of every call for the warning substring instead.
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Auto-approved: This PR fixes a ProseMirror warning by using Selection.atStart instead of TextSelection.create(doc, 0). The change is isolated to one file, has existing and new tests, and only affects non-functional selection logic.
Re-trigger cubic
Summary
@react-email/editor'sfocusScopeextension cleared the selection on blur withTextSelection.create(transaction.doc, 0). Position0resolves to the top-leveldocnode, which has no inline content, so ProseMirror'scheckTextSelectionlogsTextSelection endpoint not pointing into a node with inline content (doc)to the console on every editor blur (e.g. focusing any other field on the page).This swaps it for
Selection.atStart(transaction.doc)— same intent (cursor at document start) but a valid inline position, so no warning. This is the fix suggested in the issue.Changes
Selection.atStart(transaction.doc)infocus-scopes.tsfrom === 0) and added a regression test that asserts no console warning on blurTesting
pnpm vitest run --project unit src/extensions/focus-scopes.spec.ts→ 3 passedtsc --noEmitclean;biome checkcleanCloses #3564
Summary by cubic
Fixes the ProseMirror warning on editor blur by moving the cursor to the first valid position.
@react-email/editornow usesSelection.atStart(doc)instead ofTextSelection.create(doc, 0).Selection.atStart(doc)in focus scopes to avoid invalid selection at position 0.console.warnspy and scanwarn.mock.callsto avoid false negatives.Written for commit 48998c8. Summary will update on new commits.