fix(core): extendMarkRange match only the current mark of a type#7628
fix(core): extendMarkRange match only the current mark of a type#7628bdbch merged 10 commits intoueberdosis:mainfrom
Conversation
✅ Deploy Preview for tiptap-embed ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
🦋 Changeset detectedLatest commit: 894b745 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 |
There was a problem hiding this comment.
Pull request overview
Fixes extendMarkRange in @tiptap/core so that when attributes is omitted it only extends to the current mark instance (e.g., a single adjacent link), aligning behavior with the underlying getMarkRange change from #5826.
Changes:
- Removed the
attributes = {}default inextendMarkRangesoundefinedcan flow through togetMarkRange’s defaulting logic. - Updated
extendMarkRangeunit tests to cover omitted attributes vs. explicitly passing{}. - Added a changeset to ship the behavior change as a patch release.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/core/src/commands/extendMarkRange.ts | Stops forcing {} attributes, allowing getMarkRange to match only the current mark by default. |
| packages/core/tests/extendMarkRange.spec.ts | Adjusts existing expectation and adds coverage for adjacent marks with differing attributes (omitted vs {}). |
| .changeset/mean-islands-change.md | Declares a patch release note for the user-facing selection behavior fix. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
bdbch
left a comment
There was a problem hiding this comment.
Thanks for chasing this down - the adjacent-link behavior looks right, but I do not think this is safe to merge yet.
Removing the default in extendMarkRange makes omitted attributes inherit getMarkRanges current fallback, and that fallback uses the first mark on the text node rather than the requested mark type. That means extendMarkRange(type) can now regress on text carrying multiple marks depending on mark order.
I would like to see that helper/default behavior tightened up and covered by a regression test before approving. It would also help to make the changeset more explicit that omitted attributes and explicit empty-object now have different behavior.
|
One additional concern here: I think this is effectively a public API behavior change, not just an internal implementation tweak. Today, omitting Because of that, I think we should be careful about how we merge and ship this so we do not create migration surprises:
I do think the new behavior is more intuitive for the link case; I just want us to be explicit about whether we are shipping a bug fix or intentionally changing the command contract for existing users. |
I agree that this is technically a breaking change, since it changes the behavior of existing code. However it would be nice to release it sooner than Tiptap v4 :) I did a quick check for uses of extendMarkRange in the wild, by searching GitHub globally for ".extendMarkRange". Within the first five pages of code results, essentially all used the command for its "standard" purpose: I've also updated the changeset to clarify the change and how to preserve the old behavior. |
Changes Overview
Currently, If two links are adjacent with no white space between, extendMarkRange will select the range containing both the links. This PR changes it to only select the link containing the original selection.
An analogous change was already made for the underlying getMarkRange function (#5826). However, extendMarkRange's default of
attributes = {}was overriding that change.Implementation Approach
Remove the
attributes = {}default, so that omitted/undefined attributes pass through to getMarkRange. getMarkRange then applies its own default, which is to use the attributes of the original mark.Testing Done
See changes to extendMarkRange tests.
Verification Steps
In the Marks/Link preview, create two adjacent links with different hrefs. Then, place your cursor in one of the links, click Set Link, and update it to a 3rd href.
Before this PR: both adjacent links acquire the 3rd href.
After this PR: only the link containing the cursor acquires the 3rd href.
Additional Notes
You can recover the original behavior by passing explicitly passing
{}as the attributes parameter.Checklist
Related Issues
Analogous getMarkRange issue: #3872