fix(react): stop pulling @y/prosemirror into the default UI bundle#2902
Conversation
BlockNoteDefaultUI and AttributionTooltipController imported the AttributionExtension value from @blocknote/core/y, which pulls in @y/prosemirror even for editors without collaboration. Look up the extension by its string key instead, and inline the two small DOM helpers it needed, so the collaboration entry point is never resolved unless the app actually uses it. Fixes #2901
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe React extension hook now accepts string identifiers. Attribution UI resolves the extension by ChangesAttribution tooltip integration
Estimated code review effort: 3 (Moderate) | ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/react/src/components/AttributionTooltip/AttributionTooltipController.tsx (1)
42-68: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract shared logic for finding the target element.
The target element resolution logic is duplicated across
getBoundingClientRectandgetClientRects. Extracting this into a local helper function within theuseMemowill improve readability and maintainability.♻️ Proposed refactor
const reference = useMemo<GenericPopoverReference | undefined>( - () => - state - ? { - element: state.anchor, - getBoundingClientRect: () => { - const content = state.anchor.firstElementChild ?? state.anchor; - const rect = content.getBoundingClientRect(); - const el = - rect.width || rect.height - ? content - : (content.firstElementChild ?? content); - return el.getBoundingClientRect(); - }, - getClientRects: () => { - const content = state.anchor.firstElementChild ?? state.anchor; - const rect = content.getBoundingClientRect(); - const el = - rect.width || rect.height - ? content - : (content.firstElementChild ?? content); - return el.getClientRects(); - }, - } - : undefined, + () => { + if (!state) return undefined; + const getTargetElement = () => { + const content = state.anchor.firstElementChild ?? state.anchor; + const rect = content.getBoundingClientRect(); + return rect.width || rect.height + ? content + : (content.firstElementChild ?? content); + }; + + return { + element: state.anchor, + getBoundingClientRect: () => getTargetElement().getBoundingClientRect(), + getClientRects: () => getTargetElement().getClientRects(), + }; + }, [state], );🤖 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/components/AttributionTooltip/AttributionTooltipController.tsx` around lines 42 - 68, In the useMemo callback that creates reference, extract the duplicated target-element resolution from getBoundingClientRect and getClientRects into a local helper, then have both methods call it before retrieving their respective geometry. Preserve the existing fallback behavior using state.anchor, its first element child, and the child fallback when the resolved content has no dimensions.
🤖 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/react/src/components/AttributionTooltip/AttributionTooltipController.tsx`:
- Around line 42-68: In the useMemo callback that creates reference, extract the
duplicated target-element resolution from getBoundingClientRect and
getClientRects into a local helper, then have both methods call it before
retrieving their respective geometry. Preserve the existing fallback behavior
using state.anchor, its first element child, and the child fallback when the
resolved content has no dimensions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5601ed66-4a49-4df4-8e07-6eb38fccc6aa
📒 Files selected for processing (3)
packages/react/src/components/AttributionTooltip/AttributionTooltipController.tsxpackages/react/src/editor/BlockNoteDefaultUI.tsxpackages/react/src/hooks/useExtension.ts
@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: |
|
Summary
@blocknote/react's default UI importedAttributionExtensiondirectly from@blocknote/core/y, which forces bundlers to resolve@y/prosemirror/@y/yeven for editors that never enable collaboration.Rationale
Every consumer of
BlockNoteView(viaBlockNoteDefaultUI) hit this import at build/runtime, causingCould not resolve "@y/prosemirror"errors for non-collaborative apps that don't have those optional peer dependencies installed.Changes
BlockNoteDefaultUI.tsx: look up the attribution extension viaeditor.getExtension("attribution")(string key) instead of importing theAttributionExtensionvalue.AttributionTooltipController.tsx: onlyimport typefrom@blocknote/core/y(erased at build time), use the string-keyeduseExtensionState("attribution", ...), and inline the two small DOM helpers it previously imported.useExtension.ts: widenuseExtensionState'spluginparameter to accept a string key while still inferring the extension's state type from the generic.Impact
Non-collaborative editors no longer pull in
@y/prosemirror/@y/ythrough@blocknote/react. No behavior change for apps using collaboration/attribution.Testing
vp run lint— 0 errorsvp run test— all tests passvp run build— verified@blocknote/core/yno longer appears inpackages/react/dist/blocknote-react.jsFixes #2901
Summary by CodeRabbit
Bug Fixes
Refactor