refactor: simplify @tiptap/extension-link by inlining it to BlockNote#2623
Open
nperez0111 wants to merge 6 commits intomainfrom
Open
refactor: simplify @tiptap/extension-link by inlining it to BlockNote#2623nperez0111 wants to merge 6 commits intomainfrom
nperez0111 wants to merge 6 commits intomainfrom
Conversation
…ptions Strip out carried-over options (openOnClick, enableClickSelection, linkOnPaste, protocols, validate), deprecated types (LinkProtocolOptions, LinkOptions), and verbose JSDoc comments. Inline configuration defaults directly, pre-compile the URI validation regex, and simplify the extension registration in ExtensionManager. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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:
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 ignored due to path filters (1)
📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughReplaces external Changes
Sequence DiagramssequenceDiagram
participant Editor as Editor
participant ProseMirror as ProseMirror
participant Autolink as Autolink Plugin
participant LinkDetector as Link Detector
participant Schema as Schema/Marks
Note over Editor,ProseMirror: Autolink flow
Editor->>ProseMirror: commit transactions (text change)
ProseMirror->>Autolink: appendTransaction(transactions, oldState, newState)
Autolink->>Autolink: check preventAutolink meta & doc diff
Autolink->>LinkDetector: tokenizeLink(textSegment, defaultProtocol)
LinkDetector-->>Autolink: LinkMatch tokens
Autolink->>Schema: verify no code mark / existing link mark
Autolink->>Autolink: run validate() and shouldAutoLink()
Autolink->>ProseMirror: tr.addMark(from, to, linkMark)
Autolink-->>ProseMirror: return transaction (if modified)
sequenceDiagram
participant User as User
participant EditorDOM as Editor (DOM)
participant PastePlugin as Paste Handler
participant LinkDetector as Link Detector
participant Commands as Editor Commands
Note over User,Commands: Paste handling
User->>EditorDOM: paste event (selection non-empty)
EditorDOM->>PastePlugin: handlePaste(event, slice)
PastePlugin->>PastePlugin: extract slice textContent
PastePlugin->>LinkDetector: findLinks(text, defaultProtocol)
LinkDetector-->>PastePlugin: LinkMatch[]
PastePlugin->>PastePlugin: verify full-match && shouldAutoLink
PastePlugin->>Commands: setMark(type, { href })
Commands-->>EditorDOM: apply link mark
sequenceDiagram
participant User as User
participant EditorDOM as Editor (DOM)
participant ClickPlugin as Click Handler
participant Schema as Marks
participant External as External URL
Note over User,External: Click handling
User->>EditorDOM: left-click on link element
EditorDOM->>ClickPlugin: handleClick(view, pos, event)
ClickPlugin->>ClickPlugin: find closest <a> within editor root
ClickPlugin->>Schema: get mark attrs at pos
ClickPlugin->>ClickPlugin: merge attrs, resolve href & target
alt href present
ClickPlugin->>External: window.open(href, target)
ClickPlugin-->>EditorDOM: return true
else
ClickPlugin-->>EditorDOM: return false
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
packages/core/src/extensions/tiptap-extensions/Link/helpers/clickHandler.ts (1)
49-55: Validate the clicked URI in clickHandler for defense-in-depth.While
parseHTML()validates URIs before creating marks andlinkDetectorfilters for safe protocols, thewindow.open(href, target)call at line 54 still receives untrusted input from the DOM element'shrefproperty without explicit validation. Add a check usingisAllowedUri()before opening to prevent any edge case where an invalid URI could reach this handler. Also, addnoopener,noreferrerwhen opening a new tab, as the link mark'srelattribute doesn't apply towindow.open()calls.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/extensions/tiptap-extensions/Link/helpers/clickHandler.ts` around lines 49 - 55, The click handler currently calls window.open(href, target) with unvalidated href; update clickHandler to first run isAllowedUri(href) (use the same isAllowedUri helper used elsewhere) and return false if it fails, then call window.open only for allowed URIs; also ensure you pass the noopener,noreferrer feature string (e.g. window.open(href, target, 'noopener,noreferrer')) when opening a new tab so the rel attribute semantics are preserved for window.open calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/extensions/tiptap-extensions/Link/helpers/autolink.ts`:
- Around line 22-29: isValidLinkStructure currently only accepts a single link
token or a 3-token wrapper like () or [], but tokenizeLink/linkDetector can also
emit a 2-token form like [link, "."] (e.g., "example.com."). Update
isValidLinkStructure to also return true when tokens.length === 2 and the first
token is a link and the second token is a trailing punctuation dot
(tokens[0].isLink && tokens[1].value === ".") while keeping the existing
single-token and 3-token ("()", "[]") logic so the validator stays in sync with
tokenizeLink()/linkDetector behavior.
In `@packages/core/src/extensions/tiptap-extensions/Link/helpers/linkDetector.ts`:
- Around line 33-35: The autolink and paste paths are inconsistent because only
isSingleUrl() knows about SPECIAL_HOSTS and SCHEMELESS_RE enforces a dotted host
so hosts like "localhost" (and with port/path like "localhost:3000" or
"localhost/foo") are never recognized; update the link detection logic so both
tokenizeLink() and findLinks() accept SPECIAL_HOSTS by: expanding SCHEMELESS_RE
(or adding a parallel regex) to allow hostname-only hosts and optional
:port/path, and centralizing the special-host check used by isSingleUrl() into
the shared linkDetector helpers (e.g., reference SPECIAL_HOSTS, isSingleUrl,
SCHEMELESS_RE, tokenizeLink, findLinks) so pasted and typed links consistently
detect localhost variants.
- Around line 56-57: The current SCHEMELESS_RE (and the similar regex used later
in tokenizeLink/findLinks) only allows an optional suffix that begins with "/"
so URLs like example.com?x=1 or example.com#frag are rejected or truncated;
update the regexes (SCHEMELESS_RE and the other regex referenced around
tokenizeLink/findLinks) to allow the suffix to start with "/", "?" or "#" (e.g.,
change the suffix-start character class from "/" to "[\/?#]" and keep the rest
of the suffix pattern [^\s]* and existing port handling) so schemeless URLs with
query or fragment parts are matched correctly.
- Around line 381-386: linkToken() currently classifies any string containing
"@" and not "://" as an email, which causes buildHref() to prepend mailto: to
already-prefixed values (e.g. mailto:user@example.com → mailto:mailto:...), so
update the classification logic in linkToken() (or the snippet that sets const
type) to detect existing mailto prefixes (use a case-insensitive check like
value.toLowerCase().startsWith("mailto:") or /^mailto:/i) and treat those as
already-hrefed (do not reclassify or re-prefix); alternatively, ensure
buildHref() checks for an existing mailto: prefix before adding one—refer to
isSingleUrl(), linkToken(), buildHref(), and defaultProtocol when making the
change.
In `@packages/core/src/extensions/tiptap-extensions/Link/link.test.ts`:
- Around line 822-855: The test currently guards the paste plugin call with an
if so assertions may be skipped; instead assert the plugin and its handler exist
before invoking: locate the test "does not apply link when pasting URL with
empty selection" and replace the conditional block that finds pastePlugin with
explicit expectations (e.g., expect(pastePlugin).toBeDefined() and
expect(pastePlugin.props.handlePaste).toBeDefined()), then call
pastePlugin.props.handlePaste(...) and assert the returned value is false; keep
references to createEditor, editor.replaceBlocks, editor.setTextCursorPosition,
and pastePlugin.props.handlePaste to find and update the code.
- Around line 710-766: The test is unreliable because it conditionally skips
assertions when pastePlugin, pastePlugin.props.handlePaste, or result are falsy;
change the test to assert those values exist instead of guarding: import
TextSelection, Slice, Fragment at top (replace the two require calls) and then
assert pastePlugin is defined and pastePlugin.props.handlePaste is a function
(e.g., expect(pastePlugin).toBeDefined(); expect(typeof
pastePlugin.props.handlePaste).toBe("function")), call the handler and assert
result is truthy (expect(result).toBeTruthy()), and finally assert
getLinksInDocument(editor) yields the expected link (href and text) so the test
fails if the plugin or handler is missing or returns falsy.
- Around line 769-820: The test currently guards the assertions behind "if
(pastePlugin && pastePlugin.props.handlePaste)" which lets the test silently
pass when the plugin or handler is missing; update the test to assert the plugin
and handler exist before invoking them: replace the conditional with explicit
expectations like expect(pastePlugin).toBeDefined() and
expect(pastePlugin.props.handlePaste).toBeDefined(), then call
(pastePlugin.props.handlePaste as any)(...) and run the existing
expect(result).toBe(false) and
expect(getLinksInDocument(editor)).toHaveLength(0); reference the pastePlugin
variable and its props.handlePaste, the ClipboardEvent invocation, and
getLinksInDocument to locate the changes.
---
Nitpick comments:
In `@packages/core/src/extensions/tiptap-extensions/Link/helpers/clickHandler.ts`:
- Around line 49-55: The click handler currently calls window.open(href, target)
with unvalidated href; update clickHandler to first run isAllowedUri(href) (use
the same isAllowedUri helper used elsewhere) and return false if it fails, then
call window.open only for allowed URIs; also ensure you pass the
noopener,noreferrer feature string (e.g. window.open(href, target,
'noopener,noreferrer')) when opening a new tab so the rel attribute semantics
are preserved for window.open calls.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: a5978724-8a30-44e2-8e0b-36d56c1322e5
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
.github/dependabot.ymlpackages/core/package.jsonpackages/core/src/editor/managers/ExtensionManager/extensions.tspackages/core/src/extensions/tiptap-extensions/Link/helpers/autolink.tspackages/core/src/extensions/tiptap-extensions/Link/helpers/clickHandler.tspackages/core/src/extensions/tiptap-extensions/Link/helpers/linkDetector.tspackages/core/src/extensions/tiptap-extensions/Link/helpers/pasteHandler.tspackages/core/src/extensions/tiptap-extensions/Link/helpers/whitespace.tspackages/core/src/extensions/tiptap-extensions/Link/index.tspackages/core/src/extensions/tiptap-extensions/Link/link.test.tspackages/core/src/extensions/tiptap-extensions/Link/link.ts
💤 Files with no reviewable changes (1)
- packages/core/package.json
Comment on lines
+22
to
+29
| function isValidLinkStructure(tokens: LinkMatch[]) { | ||
| if (tokens.length === 1) { | ||
| return tokens[0].isLink; | ||
| } | ||
|
|
||
| if (tokens.length === 3 && tokens[1].isLink) { | ||
| return ["()", "[]"].includes(tokens[0].value + tokens[2].value); | ||
| } |
There was a problem hiding this comment.
Keep isValidLinkStructure() in sync with tokenizeLink().
packages/core/src/extensions/tiptap-extensions/Link/helpers/linkDetector.ts:302-329 can emit {link} and [link, "."], but this validator still only accepts a bare link or a 3-token () / [] wrapper. Typing example.com. followed by whitespace therefore never autolinks, even though the tokenizer has explicit support for that case.
Suggested fix
function isValidLinkStructure(tokens: LinkMatch[]) {
if (tokens.length === 1) {
return tokens[0].isLink;
}
+ if (tokens.length === 2) {
+ return tokens[0].isLink && tokens[1].value === ".";
+ }
+
if (tokens.length === 3 && tokens[1].isLink) {
- return ["()", "[]"].includes(tokens[0].value + tokens[2].value);
+ return ["()", "[]", "{}"].includes(tokens[0].value + tokens[2].value);
}
return false;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function isValidLinkStructure(tokens: LinkMatch[]) { | |
| if (tokens.length === 1) { | |
| return tokens[0].isLink; | |
| } | |
| if (tokens.length === 3 && tokens[1].isLink) { | |
| return ["()", "[]"].includes(tokens[0].value + tokens[2].value); | |
| } | |
| function isValidLinkStructure(tokens: LinkMatch[]) { | |
| if (tokens.length === 1) { | |
| return tokens[0].isLink; | |
| } | |
| if (tokens.length === 2) { | |
| return tokens[0].isLink && tokens[1].value === "."; | |
| } | |
| if (tokens.length === 3 && tokens[1].isLink) { | |
| return ["()", "[]", "{}"].includes(tokens[0].value + tokens[2].value); | |
| } | |
| return false; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/extensions/tiptap-extensions/Link/helpers/autolink.ts`
around lines 22 - 29, isValidLinkStructure currently only accepts a single link
token or a 3-token wrapper like () or [], but tokenizeLink/linkDetector can also
emit a 2-token form like [link, "."] (e.g., "example.com."). Update
isValidLinkStructure to also return true when tokens.length === 2 and the first
token is a link and the second token is a trailing punctuation dot
(tokens[0].isLink && tokens[1].value === ".") while keeping the existing
single-token and 3-token ("()", "[]") logic so the validator stays in sync with
tokenizeLink()/linkDetector behavior.
packages/core/src/extensions/tiptap-extensions/Link/helpers/linkDetector.ts
Show resolved
Hide resolved
packages/core/src/extensions/tiptap-extensions/Link/helpers/linkDetector.ts
Outdated
Show resolved
Hide resolved
packages/core/src/extensions/tiptap-extensions/Link/helpers/linkDetector.ts
Outdated
Show resolved
Hide resolved
packages/core/src/extensions/tiptap-extensions/Link/link.test.ts
Outdated
Show resolved
Hide resolved
Comment on lines
+822
to
+855
| it("does not apply link when pasting URL with empty selection", () => { | ||
| editor = createEditor(); | ||
| editor.replaceBlocks(editor.document, [ | ||
| { | ||
| id: "test-block", | ||
| type: "paragraph", | ||
| content: "some text", | ||
| }, | ||
| ]); | ||
|
|
||
| // Place cursor without selection | ||
| editor.setTextCursorPosition("test-block", "end"); | ||
| const view = editor._tiptapEditor.view; | ||
|
|
||
| const pastePlugin = view.state.plugins.find( | ||
| (p) => (p as any).key === "handlePasteLink$" | ||
| ); | ||
|
|
||
| if (pastePlugin && pastePlugin.props.handlePaste) { | ||
| const { Slice, Fragment } = require("@tiptap/pm/model"); | ||
| const textNode = view.state.schema.text("https://example.com"); | ||
| const slice = new Slice(Fragment.from(textNode), 0, 0); | ||
|
|
||
| const result = (pastePlugin.props.handlePaste as any)( | ||
| view, | ||
| new ClipboardEvent("paste"), | ||
| slice | ||
| ); | ||
|
|
||
| // Should return false because selection is empty | ||
| expect(result).toBe(false); | ||
| } | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Same conditional expect pattern - refactor for reliability.
Apply the same fix here to ensure the test actually runs its assertions.
Proposed fix
it("does not apply link when pasting URL with empty selection", () => {
// ... setup code ...
const pastePlugin = view.state.plugins.find(
(p) => (p as any).key === "handlePasteLink$"
);
+ expect(pastePlugin).toBeDefined();
+ expect(pastePlugin!.props.handlePaste).toBeDefined();
- if (pastePlugin && pastePlugin.props.handlePaste) {
- const { Slice, Fragment } = require("@tiptap/pm/model");
- const textNode = view.state.schema.text("https://example.com");
- const slice = new Slice(Fragment.from(textNode), 0, 0);
+ const textNode = view.state.schema.text("https://example.com");
+ const slice = new Slice(Fragment.from(textNode), 0, 0);
- const result = (pastePlugin.props.handlePaste as any)(
- view,
- new ClipboardEvent("paste"),
- slice
- );
+ const result = (pastePlugin!.props.handlePaste as any)(
+ view,
+ new ClipboardEvent("paste"),
+ slice
+ );
- // Should return false because selection is empty
- expect(result).toBe(false);
- }
+ // Should return false because selection is empty
+ expect(result).toBe(false);
});🧰 Tools
🪛 ESLint
[error] 841-841: Require statement not part of import statement.
(@typescript-eslint/no-var-requires)
[error] 852-852: Avoid calling expect conditionally`
(jest/no-conditional-expect)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/extensions/tiptap-extensions/Link/link.test.ts` around
lines 822 - 855, The test currently guards the paste plugin call with an if so
assertions may be skipped; instead assert the plugin and its handler exist
before invoking: locate the test "does not apply link when pasting URL with
empty selection" and replace the conditional block that finds pastePlugin with
explicit expectations (e.g., expect(pastePlugin).toBeDefined() and
expect(pastePlugin.props.handlePaste).toBeDefined()), then call
pastePlugin.props.handlePaste(...) and assert the returned value is false; keep
references to createEditor, editor.replaceBlocks, editor.setTextCursorPosition,
and pastePlugin.props.handlePaste to find and update the code.
…e conversion logic - Remove unused setLink/toggleLink/unsetLink TipTap commands from Link mark extension - Move editLink/deleteLink/getLinkMarkAtPos from LinkToolbar into StyleManager, exposing them as public API on BlockNoteEditor - LinkToolbar now delegates to editor API instead of doing raw mark operations - Rewrite contentNodeToInlineContent as a two-pass flatten-then-merge approach, replacing ~200 lines of nested state machine - Simplify linkToNodes in blockToNode.ts Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/core/src/extensions/LinkToolbar/LinkToolbar.ts (1)
53-55: Consider documenting or deprecating the unused_markTypeparameter.The
_markTypeparameter is no longer used since the function always delegates togetLinkAtPos. While removing it might break external consumers, a deprecation notice or JSDoc clarification would help maintain clarity.Suggested documentation
- getMarkAtPos(pos: number, _markType: string) { + /** + * `@deprecated` markType parameter is ignored; always returns link mark + */ + getMarkAtPos(pos: number, _markType: string) { return getLinkAtPos(pos); },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/extensions/LinkToolbar/LinkToolbar.ts` around lines 53 - 55, The getMarkAtPos method currently accepts an unused parameter _markType and simply calls getLinkAtPos; update the code by adding a JSDoc deprecation note for the _markType parameter (or for the whole method if appropriate) explaining it is unused and will be removed in a future release, and document that callers should rely on getLinkAtPos instead; ensure the JSDoc is placed above getMarkAtPos and references the parameter name _markType and the delegating function getLinkAtPos so consumers and linters can see the deprecation and migration path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/core/src/extensions/LinkToolbar/LinkToolbar.ts`:
- Around line 53-55: The getMarkAtPos method currently accepts an unused
parameter _markType and simply calls getLinkAtPos; update the code by adding a
JSDoc deprecation note for the _markType parameter (or for the whole method if
appropriate) explaining it is unused and will be removed in a future release,
and document that callers should rely on getLinkAtPos instead; ensure the JSDoc
is placed above getMarkAtPos and references the parameter name _markType and the
delegating function getLinkAtPos so consumers and linters can see the
deprecation and migration path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0ac9dddc-660b-4fbc-992a-9deca937d7ac
⛔ Files ignored due to path filters (7)
packages/xl-ai/src/prosemirror/__snapshots__/agent.test.ts.snapis excluded by!**/*.snap,!**/__snapshots__/**packages/xl-ai/src/prosemirror/__snapshots__/changeset.test.ts.snapis excluded by!**/*.snap,!**/__snapshots__/**tests/src/unit/core/formatConversion/export/__snapshots__/nodes/hardbreak/between-links.jsonis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/export/__snapshots__/nodes/hardbreak/link.jsonis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/export/__snapshots__/nodes/link/adjacent.jsonis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/export/__snapshots__/nodes/link/basic.jsonis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/export/__snapshots__/nodes/link/styled.jsonis excluded by!**/__snapshots__/**
📒 Files selected for processing (6)
packages/core/src/api/nodeConversions/blockToNode.tspackages/core/src/api/nodeConversions/nodeToBlock.tspackages/core/src/editor/BlockNoteEditor.tspackages/core/src/editor/managers/StyleManager.tspackages/core/src/extensions/LinkToolbar/LinkToolbar.tspackages/core/src/extensions/tiptap-extensions/Link/link.ts
- Fix no-useless-escape warnings in linkDetector.ts regex patterns - Fix curly brace requirements in linkDetector.ts and link.ts - Fix prefer-const in linkDetector.ts - Replace require() with ES imports in link.test.ts - Fix jest/no-conditional-expect by using view.someProp for paste tests Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/core/src/extensions/tiptap-extensions/Link/link.test.ts (1)
712-801: Consider extracting shared paste-test setup helpers.
applies linkanddoes not apply linkduplicate block setup + selection discovery. A small helper (setupEditorWithSelectedText,createPasteSlice) would reduce maintenance noise as cases grow.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/extensions/tiptap-extensions/Link/link.test.ts` around lines 712 - 801, Extract the duplicated setup into two small helpers in link.test.ts: create setupEditorWithSelectedText that initializes createEditor(), calls replaceBlocks with the test paragraph, sets the text cursor, finds and returns {editor, view, textStart, textEnd} (use the existing selection discovery logic), and create createPasteSlice that takes a string and returns a Slice built from view.state.schema.text(...); then refactor both tests to call setupEditorWithSelectedText() and createPasteSlice("...") and use the returned editor/view/positions to set the selection and dispatch the paste; keep test assertions identical and reuse getLinksInDocument as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/extensions/tiptap-extensions/Link/link.test.ts`:
- Around line 728-735: The test computes textStart/textEnd by scanning
doc.descendants but leaves them at 0 when the "click here" node isn't found,
causing a false-positive path; change the initialization to sentinel values
(e.g., textStart = -1, textEnd = -1) or add an explicit assertion immediately
after the descendants loop that the node was found (e.g.,
expect(textStart).not.toBe(-1) / expect(textEnd).toBeGreaterThan(textStart)),
and apply this assertion in both paste-related tests that compute
textStart/textEnd so the selected-text branch is actually exercised.
---
Nitpick comments:
In `@packages/core/src/extensions/tiptap-extensions/Link/link.test.ts`:
- Around line 712-801: Extract the duplicated setup into two small helpers in
link.test.ts: create setupEditorWithSelectedText that initializes
createEditor(), calls replaceBlocks with the test paragraph, sets the text
cursor, finds and returns {editor, view, textStart, textEnd} (use the existing
selection discovery logic), and create createPasteSlice that takes a string and
returns a Slice built from view.state.schema.text(...); then refactor both tests
to call setupEditorWithSelectedText() and createPasteSlice("...") and use the
returned editor/view/positions to set the selection and dispatch the paste; keep
test assertions identical and reuse getLinksInDocument as before.
🪄 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: 4d833076-f2cf-489c-8856-b93e87939b6f
📒 Files selected for processing (3)
packages/core/src/extensions/tiptap-extensions/Link/helpers/linkDetector.tspackages/core/src/extensions/tiptap-extensions/Link/link.test.tspackages/core/src/extensions/tiptap-extensions/Link/link.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/core/src/extensions/tiptap-extensions/Link/link.ts
- packages/core/src/extensions/tiptap-extensions/Link/helpers/linkDetector.ts
Comment on lines
+728
to
+735
| let textStart = 0; | ||
| let textEnd = 0; | ||
| doc.descendants((node, pos) => { | ||
| if (node.isText && node.text === "click here") { | ||
| textStart = pos; | ||
| textEnd = pos + node.nodeSize; | ||
| } | ||
| }); |
There was a problem hiding this comment.
Assert selection bounds before invoking paste behavior.
If the "click here" text node isn’t found, the test falls back to 0..0 selection and the non-URL case can pass without exercising the “selected text” path.
Proposed reliability fix
let textStart = 0;
let textEnd = 0;
doc.descendants((node, pos) => {
if (node.isText && node.text === "click here") {
textStart = pos;
textEnd = pos + node.nodeSize;
}
});
+ expect(textEnd).toBeGreaterThan(textStart);
const tr = view.state.tr.setSelection(
TextSelection.create(view.state.doc, textStart, textEnd)
);Apply the same assertion in both paste tests that compute textStart/textEnd.
Also applies to: 774-781
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/extensions/tiptap-extensions/Link/link.test.ts` around
lines 728 - 735, The test computes textStart/textEnd by scanning doc.descendants
but leaves them at 0 when the "click here" node isn't found, causing a
false-positive path; change the initialization to sentinel values (e.g.,
textStart = -1, textEnd = -1) or add an explicit assertion immediately after the
descendants loop that the node was found (e.g., expect(textStart).not.toBe(-1) /
expect(textEnd).toBeGreaterThan(textStart)), and apply this assertion in both
paste-related tests that compute textStart/textEnd so the selected-text branch
is actually exercised.
@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: |
@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: |
- Fix double mailto: prefix for mailto: URLs in linkToken() — values starting with "mailto:" are now classified as "url" not "email" - Support schemeless URLs with ? or # suffixes (e.g. example.com?x=1, example.com#frag) in both SCHEMELESS_RE and isSingleUrl() Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/extensions/tiptap-extensions/Link/helpers/linkDetector.ts`:
- Around line 393-396: The email detection is case-sensitive and can
mis-classify inputs like "MAILTO:..." causing a double "mailto:" prefix; in
linkDetector.ts update the mailto check to be case-insensitive by using
value.toLowerCase().startsWith("mailto:") (i.e., change the
!value.startsWith("mailto:") test to !value.toLowerCase().startsWith("mailto:"))
while keeping the rest of the expression (value.includes("@") and
!value.includes("://")) untouched so buildHref will no longer prepend a second
"mailto:".
🪄 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: c04a3e41-bb2f-46d0-acba-1fd01c4a2be4
📒 Files selected for processing (1)
packages/core/src/extensions/tiptap-extensions/Link/helpers/linkDetector.ts
Comment on lines
+393
to
+396
| const type = | ||
| value.includes("@") && !value.includes("://") && !value.startsWith("mailto:") | ||
| ? "email" | ||
| : "url"; |
There was a problem hiding this comment.
Case-sensitive mailto: check can still cause double prefix.
The fix for the double mailto: prefix issue is case-sensitive. Input like MAILTO:user@example.com would be classified as "email" (since "MAILTO:".startsWith("mailto:") is false), causing buildHref to return "mailto:MAILTO:user@example.com".
Proposed fix
function linkToken(
value: string,
start: number,
end: number,
defaultProtocol: string
): LinkMatch {
const type =
- value.includes("@") && !value.includes("://") && !value.startsWith("mailto:")
+ value.includes("@") && !value.includes("://") && !/^mailto:/i.test(value)
? "email"
: "url";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/extensions/tiptap-extensions/Link/helpers/linkDetector.ts`
around lines 393 - 396, The email detection is case-sensitive and can
mis-classify inputs like "MAILTO:..." causing a double "mailto:" prefix; in
linkDetector.ts update the mailto check to be case-insensitive by using
value.toLowerCase().startsWith("mailto:") (i.e., change the
!value.startsWith("mailto:") test to !value.toLowerCase().startsWith("mailto:"))
while keeping the rest of the expression (value.includes("@") and
!value.includes("://")) untouched so buildHref will no longer prepend a second
"mailto:".
The ^1.4.15 specifier allowed better-auth 1.5.6 to be installed, which removed the createAuthMiddleware export and broke the docs build. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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.
Summary
The goal of this change is that
@tiptap/extension-linkhas proved to be somewhat problematic in the past for a couple of reasons:Rationale
Given these, I felt it was appropriate to just inline the extension into BlockNote & write a simpler regex based matcher for link detection. It will not be quite as full featured as linkify was, but it is an order of magnitude simpler & lighter.
Changes
@tiptap/extension-linkinto BlockNote core. Simplfies interface since we don't need all the configuration options it hadlinkifysince it was a heavyweight dep, and somewhat inappropriate for this use-case anywayImpact
Testing
Screenshots/Video
Checklist
Additional Notes
Summary by CodeRabbit
New Features
Tests
Chores