feat: math block#2857
Open
matthewlipski wants to merge 13 commits into
Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
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:
📝 WalkthroughWalkthroughThis PR refactors the code block implementation in ChangesCode Block Refactor and SyntaxHighlighting Extension
New
Sequence DiagramsequenceDiagram
participant App
participant BlockNoteEditor
participant SyntaxHighlightingExtension
participant lazyShikiPlugin
participant createSourceBlockWithPreview
participant FloatingUI
App->>BlockNoteEditor: useCreateBlockNote({ syntaxHighlighting: { createHighlighter }, schema })
BlockNoteEditor->>SyntaxHighlightingExtension: scan blockSpecs for inline content types
SyntaxHighlightingExtension->>lazyShikiPlugin: build ProseMirror highlight plugin
lazyShikiPlugin-->>SyntaxHighlightingExtension: plugin (lazy per-language load on parse)
App->>createSourceBlockWithPreview: render math/code block
createSourceBlockWithPreview->>FloatingUI: autoUpdate + computePosition for popup
FloatingUI-->>createSourceBlockWithPreview: positioned popup DOM
createSourceBlockWithPreview-->>App: { dom, contentDOM, update, ignoreMutation, destroy }
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 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 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (1)
packages/math-block/src/helpers/toExternalHTML/createMathML.ts (1)
8-17: Consider explicitly pinning the Temml trust mode.At lines 8–13, while Temml's default
trustvalue isfalse(which is secure), explicitly settingtrust: falsemakes the security intent clear and protects against potential upstream default changes.Suggested change
const mathml = temml.renderToString(getMathSource(block), { displayMode: true, annotate: true, + trust: false, // Export gracefully renders invalid LaTeX rather than throwing. throwOnError: false, });🤖 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/math-block/src/helpers/toExternalHTML/createMathML.ts` around lines 8 - 17, The temml.renderToString() call on line 8 does not explicitly set the trust property in its options object. Add trust: false to the options passed to temml.renderToString() alongside the existing displayMode, annotate, and throwOnError properties to explicitly document the security intent and protect against potential upstream default changes to Temml's trust setting.Source: Linters/SAST tools
🤖 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 `@examples/06-custom-schema/09-math-block/index.html`:
- Line 1: Add the HTML5 DOCTYPE declaration as the very first line of the file
before the <html> tag. Insert `<!doctype html>` on line 1, which will move the
existing `<html lang="en">` tag to line 2. This ensures the browser renders in
standards mode instead of triggering quirks mode, and makes the HTML document
compliant with proper HTML5 structure requirements.
In `@examples/06-custom-schema/09-math-block/main.tsx`:
- Line 4: The import statement for App specifies the wrong file extension (.jsx
instead of .tsx). Change the import path from ./src/App.jsx to ./src/App.tsx to
match the actual file name and allow TypeScript module resolution to succeed
with the allowJs: false configuration.
In `@examples/06-custom-schema/09-math-block/vite.config.ts`:
- Around line 16-27: The vite alias configuration in the conditional block is
using `../../` in the path.resolve calls for both `@blocknote/core` and
`@blocknote/react`, but it should use `../../../` to resolve the packages from the
correct directory level. Update each path string passed to path.resolve (for
"`@blocknote/core`" and "`@blocknote/react`") to include one additional `../` in the
relative path so the source-alias branch activates correctly when the packages
source directory exists.
In `@packages/core/src/blocks/Code/CodeBlockOptions.ts`:
- Around line 72-80: The getLanguageId function performs case-sensitive
comparisons when matching the languageName parameter against language aliases
and IDs, causing inputs like `TS` or `Js` to fail to map to their canonical
lowercase configured IDs. Fix this by normalizing all comparisons to be
case-insensitive: convert the incoming languageName parameter to lowercase, and
convert both the aliases from the supportedLanguages configuration and the id
values to lowercase before performing the comparison checks in the find
callback.
In `@packages/core/src/blocks/Code/helpers/parse/parsePreCode.ts`:
- Around line 19-22: The language-class detection in the code is too broad
because it uses includes("language-") which matches the substring anywhere in
the class name, potentially capturing false positives like classes containing
"language-" in the middle of the string. Replace the includes("language-") check
with startsWith("language-") to ensure only class names that actually begin with
"language-" are considered valid language identifiers, restricting the match to
the beginning of the string only.
In `@packages/core/src/blocks/Code/helpers/render/createCodeBlockWrapper.ts`:
- Around line 10-12: The language lookup on line 11 does not normalize language
aliases before accessing the supportedLanguages map. If the language value is an
alias such as "ts", the lookup will fail and createPreview will be skipped,
causing fallback to source-only rendering. Resolve the language string through a
shared language-ID resolver to normalize it to its canonical form before using
it as the lookup key in supportedLanguages on line 11.
In `@packages/core/src/blocks/Code/helpers/render/createSourceBlock.ts`:
- Around line 8-25: The language value being assigned to select.value is not
validated against the supported languages, which can result in no option being
selected if the stored language is an alias or outdated identifier. Before
assigning select.value in the language dropdown initialization, add logic to
resolve the language variable to a canonical key from
options.supportedLanguages. Check if the language exists as a key in the
supportedLanguages object; if not, fall back to the default language or the
first available option. Only after normalizing the language should you assign it
to select.value.
In `@packages/core/src/editor/Block.css`:
- Around line 482-517: Remove the empty lines that appear before CSS property
declarations in the `.bn-code-block-source-popup` and its related child
selectors (including `.bn-code-block-source-popup > div > select`,
`.bn-code-block-source-popup > div > select > option`, and
`.bn-code-block-source-popup > pre`). These blank lines before declarations are
triggering Stylelint violations. Go through each CSS rule block and delete the
extra blank lines while maintaining proper formatting between distinct rule
blocks.
In `@packages/core/src/extensions/SyntaxHighlighting/shiki.ts`:
- Around line 29-32: The code caches the Shiki highlighter and parser on
globalThis, causing all editor instances to share the same cache regardless of
their individual syntaxHighlighting.createHighlighter configuration. This
violates the editor-level configuration contract. Remove the global caching
mechanism by eliminating the globalThis symbol-keyed properties and instead
store the highlighter and parser at the instance level (as properties on an
editor-specific object or context). Specifically, refactor the
globalThisForShiki type definition and related caching logic at the three
affected sites in packages/core/src/extensions/SyntaxHighlighting/shiki.ts
(lines 29-32, 44-46, and 74-76) to use instance-specific storage instead of
globalThis, ensuring each editor instance maintains its own separate highlighter
and parser cache.
In `@packages/math-block/src/helpers/getMathSource.ts`:
- Around line 8-11: In the getMathSource function's array mapping logic, add a
defensive check before using the `in` operator on the node variable. Since
block.content is of unknown type, the array items could be primitives, null, or
non-objects that would throw a TypeError when used with the `in` operator.
Modify the map callback to first verify that node is an object (using typeof
node === "object" && node !== null or a similar guard) before attempting to
access the "text" property, ensuring the function gracefully handles unexpected
data types without crashing.
In `@packages/math-block/vite.config.ts`:
- Around line 52-56: The vite.config.ts build configuration is currently
including devDependencies in the externals check along with dependencies and
peerDependencies. Remove the spread of pkg.devDependencies from the
Object.keys() call in the external configuration block so that only actual
runtime dependencies (dependencies and peerDependencies) are marked as external.
This ensures that accidental imports of dev-only packages will fail during the
build rather than being shipped unresolved to library consumers.
---
Nitpick comments:
In `@packages/math-block/src/helpers/toExternalHTML/createMathML.ts`:
- Around line 8-17: The temml.renderToString() call on line 8 does not
explicitly set the trust property in its options object. Add trust: false to the
options passed to temml.renderToString() alongside the existing displayMode,
annotate, and throwOnError properties to explicitly document the security intent
and protect against potential upstream default changes to Temml's trust setting.
🪄 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: e9b64910-b7e1-4084-b088-4b77233bd8c5
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (54)
docs/content/docs/features/blocks/code-blocks.mdxdocs/package.jsonexamples/04-theming/06-code-block/src/App.tsxexamples/04-theming/07-custom-code-block/src/App.tsxexamples/06-custom-schema/09-math-block/.bnexample.jsonexamples/06-custom-schema/09-math-block/README.mdexamples/06-custom-schema/09-math-block/index.htmlexamples/06-custom-schema/09-math-block/main.tsxexamples/06-custom-schema/09-math-block/package.jsonexamples/06-custom-schema/09-math-block/src/App.tsxexamples/06-custom-schema/09-math-block/tsconfig.jsonexamples/06-custom-schema/09-math-block/vite.config.tspackages/code-block/package.jsonpackages/code-block/src/index.test.tspackages/code-block/src/index.tspackages/core/package.jsonpackages/core/src/blocks/Code/CodeBlockOptions.tspackages/core/src/blocks/Code/block.test.tspackages/core/src/blocks/Code/block.tspackages/core/src/blocks/Code/helpers/extensions/createCodeKeyboardShortcutsExtension.tspackages/core/src/blocks/Code/helpers/extensions/createPreviewSourceNavigationExtension.tspackages/core/src/blocks/Code/helpers/extensions/createPreviewSourceSelectionExtension.tspackages/core/src/blocks/Code/helpers/parse/parsePreCode.tspackages/core/src/blocks/Code/helpers/render/createCodeBlockWrapper.tspackages/core/src/blocks/Code/helpers/render/createPreviewWithSourcePopup.tspackages/core/src/blocks/Code/helpers/render/createSourceBlock.tspackages/core/src/blocks/Code/helpers/toExternalHTML/createPreCode.tspackages/core/src/blocks/Code/shiki.tspackages/core/src/blocks/index.tspackages/core/src/editor/Block.csspackages/core/src/editor/BlockNoteEditor.tspackages/core/src/editor/managers/ExtensionManager/extensions.tspackages/core/src/extensions/SyntaxHighlighting/SyntaxHighlighting.test.tspackages/core/src/extensions/SyntaxHighlighting/SyntaxHighlighting.tspackages/core/src/extensions/SyntaxHighlighting/shiki.tspackages/core/src/extensions/index.tspackages/core/src/index.tspackages/core/src/schema/blocks/createSpec.tspackages/core/src/schema/blocks/types.tspackages/math-block/.gitignorepackages/math-block/LICENSEpackages/math-block/package.jsonpackages/math-block/src/block.test.tspackages/math-block/src/block.tspackages/math-block/src/helpers/getMathSource.tspackages/math-block/src/helpers/parse/parseMathML.tspackages/math-block/src/helpers/render/createMathPreview.tspackages/math-block/src/helpers/toExternalHTML/createMathML.tspackages/math-block/src/index.tspackages/math-block/src/vite-env.d.tspackages/math-block/tsconfig.jsonpackages/math-block/vite.config.tspackages/math-block/vitestSetup.tsplayground/src/examples.gen.tsx
💤 Files with no reviewable changes (1)
- packages/core/src/blocks/Code/shiki.ts
Comment on lines
+29
to
+32
| const globalThisForShiki = globalThis as { | ||
| [shikiHighlighterPromiseSymbol]?: Promise<HighlighterGeneric<any, any>>; | ||
| [shikiParserSymbol]?: Parser; | ||
| }; |
Contributor
There was a problem hiding this comment.
Avoid global Shiki cache collisions across editor instances.
Line 44 and Line 74 cache the highlighter/parser on globalThis, so the first editor instance can silently control highlighting behavior for later editors with different syntaxHighlighting.createHighlighter options. This violates the editor-level configuration contract and can produce incorrect themes/language availability in multi-editor pages.
Suggested fix
- const globalThisForShiki = globalThis as {
- [shikiHighlighterPromiseSymbol]?: Promise<HighlighterGeneric<any, any>>;
- [shikiParserSymbol]?: Parser;
- };
+ let highlighterPromise: Promise<HighlighterGeneric<any, any>> | undefined;
let highlighter: HighlighterGeneric<any, any> | undefined;
let parser: Parser | undefined;
@@
if (!highlighter) {
- globalThisForShiki[shikiHighlighterPromiseSymbol] =
- globalThisForShiki[shikiHighlighterPromiseSymbol] ||
- options.createHighlighter();
+ highlighterPromise = highlighterPromise || options.createHighlighter();
- return globalThisForShiki[shikiHighlighterPromiseSymbol].then(
+ return highlighterPromise.then(
(createdHighlighter) => {
highlighter = createdHighlighter;
},
);
}
@@
if (!parser) {
- parser =
- globalThisForShiki[shikiParserSymbol] ||
- createParser(highlighter as any);
- globalThisForShiki[shikiParserSymbol] = parser;
+ parser = createParser(highlighter as any);
}Also applies to: 44-46, 74-76
🤖 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/core/src/extensions/SyntaxHighlighting/shiki.ts` around lines 29 -
32, The code caches the Shiki highlighter and parser on globalThis, causing all
editor instances to share the same cache regardless of their individual
syntaxHighlighting.createHighlighter configuration. This violates the
editor-level configuration contract. Remove the global caching mechanism by
eliminating the globalThis symbol-keyed properties and instead store the
highlighter and parser at the instance level (as properties on an
editor-specific object or context). Specifically, refactor the
globalThisForShiki type definition and related caching logic at the three
affected sites in packages/core/src/extensions/SyntaxHighlighting/shiki.ts
(lines 29-32, 44-46, and 74-76) to use instance-specific storage instead of
globalThis, ensuring each editor instance maintains its own separate highlighter
and parser cache.
Comment on lines
+52
to
+56
| Object.keys({ | ||
| ...pkg.dependencies, | ||
| ...((pkg as any).peerDependencies || {}), | ||
| ...pkg.devDependencies, | ||
| }).some((dep) => source === dep || source.startsWith(dep + "/")) |
Contributor
There was a problem hiding this comment.
Do not externalize devDependencies in the published library build.
Including pkg.devDependencies in external can ship unresolved runtime imports to consumers if a dev-only package is imported by mistake. Keep externals to dependencies/peerDependencies so declaration mistakes fail fast during packaging.
Proposed fix
Object.keys({
...pkg.dependencies,
...((pkg as any).peerDependencies || {}),
- ...pkg.devDependencies,
}).some((dep) => source === dep || source.startsWith(dep + "/"))📝 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
| Object.keys({ | |
| ...pkg.dependencies, | |
| ...((pkg as any).peerDependencies || {}), | |
| ...pkg.devDependencies, | |
| }).some((dep) => source === dep || source.startsWith(dep + "/")) | |
| Object.keys({ | |
| ...pkg.dependencies, | |
| ...((pkg as any).peerDependencies || {}), | |
| }).some((dep) => source === dep || source.startsWith(dep + "/")) |
🤖 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/math-block/vite.config.ts` around lines 52 - 56, The vite.config.ts
build configuration is currently including devDependencies in the externals
check along with dependencies and peerDependencies. Remove the spread of
pkg.devDependencies from the Object.keys() call in the external configuration
block so that only actual runtime dependencies (dependencies and
peerDependencies) are marked as external. This ensures that accidental imports
of dev-only packages will fail during the build rather than being shipped
unresolved to library consumers.
YousefED
reviewed
Jun 17, 2026
YousefED
left a comment
YousefED
left a comment
Collaborator
There was a problem hiding this comment.
Exciting!
Looking at the code from a high level (no user testing or deep dive into the functions), I have the following high-level questions:
- Should we use React or Vanilla for the components? (especially thinking about customizability)
- Curious if the current code design (with the keyboard handlers) also scales to interfaces where the editor is not in a pop-up (e.g.: Notion / TypeCell style editors that can collapse)
| **Syntax Highlighting** | ||
|
|
||
| BlockNote also provides a generic set of options for syntax highlighting in the `@blocknote/code-block` package, which support a wide range of languages: | ||
| Syntax highlighting is handled by a separate editor extension, configured at the editor level via the `syntaxHighlighting` option (not on the code block itself), so it can highlight any block's content: |
Collaborator
There was a problem hiding this comment.
Curious if we can find a better way to do this. This will make it difficult to create easily installable block-plugins later I think
Contributor
There was a problem hiding this comment.
I don't think syntaxHighlighting should be a top-level option like this. But as a single extension, maybe yea.
What's annoying is that it wants to be a singleton, but multiple places need to configure it. I'll think on it.
| const { block } = editor.getTextCursorPosition(); | ||
| if (block.type === blockType) { | ||
| // TODO should probably only tab when at a line start or already tabbed in | ||
| tr.insertText(" "); |
Collaborator
There was a problem hiding this comment.
spaces or \t? or configurable?
| @@ -0,0 +1,170 @@ | |||
| import { | |||
Collaborator
There was a problem hiding this comment.
do we prefer react / vanilla for these components? (cc @nperez0111 )
Collaborator
There was a problem hiding this comment.
first thought; vanilla makes it pretty complex to customize them in a block-view style manner right?
YousefED
reviewed
Jun 17, 2026
Collaborator
There was a problem hiding this comment.
Here's some feedback after a super early round of user-testing:
- pressing up after “s” doesn’t go to the first popup:
-
esc should close popup
-
formattingtoolbar appears when highlighting text.
-
I’m allowed to “bold” things. I think we should see if we can make sure at a lower level code blocks cannot allow formatted text. This probably also caused the issue with the exporters recently (remember?)
-
preview opens when dragging block
-
keyboard is buggy when two math blocks sit above/below combined with the “gapcursor”
-
this is a bit misaligned:
This might also be nice for testing:
- slashmenu support in demos
- collaboration example
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/math-block/src/block.test.ts (1)
762-762: ⚡ Quick winPrefer semantic MathML assertions over exact full-string equality.
This assertion is fragile to harmless serialization differences. Assert the exported node structure and
application/x-texannotation content instead of matching one exact serialized string.Suggested test hardening
- expect(serializedMathML).toBe( - `"<math xmlns="http://www.w3.org/1998/Math/MathML" ... </math>"`, - ); + const parsed = new DOMParser().parseFromString(serializedMathML, "text/html"); + const math = parsed.querySelector("math"); + expect(math).not.toBeNull(); + const tex = math + ?.querySelector('annotation[encoding="application/x-tex"]') + ?.textContent + ?.trim(); + expect(tex).toBe("a^2 + b^2 = c^2");🤖 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/math-block/src/block.test.ts` at line 762, The test assertion at line 762 performs brittle exact full-string matching of the entire serialized MathML output, which is fragile to harmless serialization differences. Instead of comparing the complete serialized string, extract and assert specific structural elements: validate the exported node structure and specifically check the application/x-tex annotation content value rather than relying on full-string equality to make the test more resilient to formatting or serialization changes.
🤖 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/core/src/blocks/Code/helpers/extensions/createPreviewSourceNavigationExtension.ts`:
- Around line 175-218: The Enter and Escape handlers are missing
scrollIntoView() calls after setting the selection, creating an inconsistency
with the arrow handlers (lines 127, 147) which do include these calls. Add
tr.scrollIntoView() after each tr.setSelection() call in both the Enter handler
and the Escape handler to ensure the viewport scrolls to keep the focused block
visible when the selection changes.
---
Nitpick comments:
In `@packages/math-block/src/block.test.ts`:
- Line 762: The test assertion at line 762 performs brittle exact full-string
matching of the entire serialized MathML output, which is fragile to harmless
serialization differences. Instead of comparing the complete serialized string,
extract and assert specific structural elements: validate the exported node
structure and specifically check the application/x-tex annotation content value
rather than relying on full-string equality to make the test more resilient to
formatting or serialization changes.
🪄 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: 49fca915-9fa2-4f91-9469-351dd45a6d53
⛔ Files ignored due to path filters (2)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamltests/src/unit/core/schema/__snapshots__/blocks.jsonis excluded by!**/__snapshots__/**
📒 Files selected for processing (14)
packages/core/src/blocks/Code/CodeBlockOptions.tspackages/core/src/blocks/Code/helpers/extensions/createPreviewSourceNavigationExtension.tspackages/core/src/blocks/Code/helpers/parse/parsePreCode.tspackages/core/src/blocks/Code/helpers/render/createCodeBlockWrapper.tspackages/core/src/blocks/Code/helpers/render/createPreviewWithSourcePopup.tspackages/core/src/blocks/Code/helpers/render/createSourceBlock.tspackages/core/src/blocks/index.tspackages/core/src/editor/Block.csspackages/math-block/package.jsonpackages/math-block/src/block.test.tspackages/math-block/src/block.tspackages/math-block/src/helpers/parse/parseMathML.tspackages/math-block/src/helpers/render/createMathPreview.tspackages/math-block/src/helpers/toExternalHTML/createMathML.ts
💤 Files with no reviewable changes (2)
- packages/core/src/blocks/index.ts
- packages/math-block/src/helpers/parse/parseMathML.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/core/src/blocks/Code/helpers/parse/parsePreCode.ts
- packages/core/src/blocks/Code/helpers/render/createCodeBlockWrapper.ts
- packages/core/src/blocks/Code/helpers/render/createSourceBlock.ts
- packages/math-block/src/block.ts
- packages/core/src/blocks/Code/CodeBlockOptions.ts
- packages/core/src/editor/Block.css
- packages/core/src/blocks/Code/helpers/render/createPreviewWithSourcePopup.ts
Collaborator
Author
|
@blocknote/ariakit
@blocknote/code-block
@blocknote/core
@blocknote/mantine
@blocknote/math-block
@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: |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/math-block/src/block.test.ts (1)
193-217: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert
defaultPreventedin edge-arrow tests.Both tests say the key handling “defers to the default,” but they only assert cursor position. Add
expect(pressKey(...)).toBe(false)to verify no interception happened.Suggested test tightening
it("ArrowLeft with no previous block defers to the default", () => { @@ - pressKey("math", "ArrowLeft"); + expect(pressKey("math", "ArrowLeft")).toBe(false); expect(editor.getTextCursorPosition().block.id).toBe("math"); }); it("ArrowRight with no next block defers to the default", () => { @@ - pressKey("math", "ArrowRight"); + expect(pressKey("math", "ArrowRight")).toBe(false); expect(editor.getTextCursorPosition().block.id).toBe("math"); });🤖 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/math-block/src/block.test.ts` around lines 193 - 217, In both tests within the "at the document edges" describe block, you need to add assertions to verify that the arrow key events were not intercepted. After each pressKey() call (in "ArrowLeft with no previous block defers to the default" and "ArrowRight with no next block defers to the default" tests), add an expectation that pressKey returns false to confirm the default behavior was not prevented, rather than only asserting the cursor position remains unchanged.
🤖 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/core/src/blocks/Code/block.ts`:
- Around line 36-46: The render function in the Code block is directly accessing
createPreview property from options.supportedLanguages without checking if the
language key exists first. Add a guard condition to verify that
block.props.language is a valid key in options.supportedLanguages before
attempting to access its createPreview property. If the language is not
supported, the language configuration object should either be omitted or handled
gracefully to prevent throwing an error when unsupported languages are
encountered.
In `@packages/core/src/blocks/Code/helpers/render/createSourceBlock.ts`:
- Around line 29-39: The handleLanguageChange function unconditionally calls
editor.updateBlock without checking if the editor is currently editable at event
time. Although the listener attachment is guarded by editor.isEditable at mount,
this guard does not apply if the editor's editability changes after the listener
is attached. Move the editor.isEditable check inside the handleLanguageChange
function body to guard the editor.updateBlock call itself, ensuring that state
changes are prevented when the editor is read-only at the moment the event
fires.
In
`@packages/core/src/blocks/Code/helpers/render/createSourceBlockWithPreview.ts`:
- Line 297: The sourceBlockPopup element is set to inert at line 297, but the
setSourcePopupOpen function (lines 313-314) that controls opening and closing
the popup never toggles the inert state back to false when opening. Update the
setSourcePopupOpen function to conditionally set sourceBlockPopup.inert based on
whether the popup is being opened or closed, ensuring it becomes interactive
when open and inert when closed.
---
Nitpick comments:
In `@packages/math-block/src/block.test.ts`:
- Around line 193-217: In both tests within the "at the document edges" describe
block, you need to add assertions to verify that the arrow key events were not
intercepted. After each pressKey() call (in "ArrowLeft with no previous block
defers to the default" and "ArrowRight with no next block defers to the default"
tests), add an expectation that pressKey returns false to confirm the default
behavior was not prevented, rather than only asserting the cursor position
remains unchanged.
🪄 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: f917fcf8-de6f-4a16-9ac2-fd01f7f596b3
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (37)
examples/06-custom-schema/09-math-block/.bnexample.jsonexamples/06-custom-schema/09-math-block/package.jsonexamples/06-custom-schema/09-math-block/src/App.tsxexamples/06-custom-schema/09-math-block/vite-env.d.tspackages/core/src/blocks/Code/CodeBlockOptions.tspackages/core/src/blocks/Code/block.tspackages/core/src/blocks/Code/helpers/render/createSourceBlock.tspackages/core/src/blocks/Code/helpers/render/createSourceBlockWithPreview.tspackages/core/src/blocks/index.tspackages/core/src/editor/Block.csspackages/core/src/i18n/locales/ar.tspackages/core/src/i18n/locales/de.tspackages/core/src/i18n/locales/en.tspackages/core/src/i18n/locales/es.tspackages/core/src/i18n/locales/fa.tspackages/core/src/i18n/locales/fr.tspackages/core/src/i18n/locales/he.tspackages/core/src/i18n/locales/hr.tspackages/core/src/i18n/locales/is.tspackages/core/src/i18n/locales/it.tspackages/core/src/i18n/locales/ja.tspackages/core/src/i18n/locales/ko.tspackages/core/src/i18n/locales/nl.tspackages/core/src/i18n/locales/no.tspackages/core/src/i18n/locales/pl.tspackages/core/src/i18n/locales/pt.tspackages/core/src/i18n/locales/ru.tspackages/core/src/i18n/locales/sk.tspackages/core/src/i18n/locales/uk.tspackages/core/src/i18n/locales/uz.tspackages/core/src/i18n/locales/vi.tspackages/core/src/i18n/locales/zh-tw.tspackages/core/src/i18n/locales/zh.tspackages/math-block/src/block.test.tspackages/math-block/src/block.tsplayground/src/examples.gen.tsxplayground/vite.config.ts
✅ Files skipped from review due to trivial changes (17)
- playground/vite.config.ts
- examples/06-custom-schema/09-math-block/vite-env.d.ts
- examples/06-custom-schema/09-math-block/.bnexample.json
- packages/core/src/i18n/locales/es.ts
- packages/core/src/i18n/locales/ko.ts
- packages/core/src/i18n/locales/fr.ts
- packages/core/src/i18n/locales/ru.ts
- packages/core/src/i18n/locales/de.ts
- packages/core/src/i18n/locales/ja.ts
- packages/core/src/i18n/locales/pl.ts
- packages/core/src/i18n/locales/it.ts
- examples/06-custom-schema/09-math-block/package.json
- packages/core/src/i18n/locales/uz.ts
- packages/core/src/i18n/locales/nl.ts
- packages/core/src/i18n/locales/vi.ts
- packages/core/src/i18n/locales/en.ts
- playground/src/examples.gen.tsx
nperez0111
reviewed
Jun 23, 2026
| **Syntax Highlighting** | ||
|
|
||
| BlockNote also provides a generic set of options for syntax highlighting in the `@blocknote/code-block` package, which support a wide range of languages: | ||
| Syntax highlighting is handled by a separate editor extension, configured at the editor level via the `syntaxHighlighting` option (not on the code block itself), so it can highlight any block's content: |
Contributor
There was a problem hiding this comment.
I don't think syntaxHighlighting should be a top-level option like this. But as a single extension, maybe yea.
What's annoying is that it wants to be a singleton, but multiple places need to configure it. I'll think on it.
Comment on lines
+356
to
+360
| ignoreMutation: (mutation: ViewMutationRecord) => | ||
| // Ignore mutations outside of the inline content container. Used mainly to ignore DOM | ||
| // changes caused preview updates. | ||
| !sourceBlock.contentDOM.parentElement || | ||
| !sourceBlock.contentDOM.parentElement.contains(mutation.target), |
Contributor
There was a problem hiding this comment.
I'll need to look into this later. But, I think we should make this the default no? Why would we ever want a mutation observer on this element?
Comment on lines
+326
to
+351
| const keyboardNavigationHandler = handleKeyboardNavigation( | ||
| block, | ||
| editor, | ||
| isSourcePopupOpen, | ||
| setSourcePopupOpen, | ||
| ); | ||
|
|
||
| const previewMouseDownHandler = handlePreviewMouseDown( | ||
| block, | ||
| editor, | ||
| previewContainer, | ||
| setSourcePopupOpen, | ||
| ); | ||
|
|
||
| const selectionMoveOutHandler = handleSelectionChange( | ||
| block, | ||
| editor, | ||
| previewWithSourcePopup, | ||
| isSourcePopupOpen, | ||
| setSourcePopupOpen, | ||
| ); | ||
|
|
||
| const sourcePopupPositioner = positionSourcePopup( | ||
| previewContainer, | ||
| sourceBlockPopup, | ||
| ); |
Contributor
There was a problem hiding this comment.
I'm unsure whether this should be here (on rendering) or part of the block's extensions. I think the reason that it is here is for the isSourcePopupOpen and setSourcePopupOpen? But we can do that by getting the block's id in the dom and getting/setting the attr there.
Comment on lines
+208
to
+229
| const destroy = autoUpdate(preview, sourcePopup, async () => { | ||
| const { x, y } = await computePosition(preview, sourcePopup, { | ||
| placement: "bottom-start", | ||
| middleware: [ | ||
| offset(4), | ||
| flip(), | ||
| shift({ padding: 4 }), | ||
| // Match the popup's width to the block. | ||
| size({ | ||
| apply({ rects, elements }) { | ||
| const blockContent = preview.closest(".bn-block-content"); | ||
| const width = | ||
| blockContent?.getBoundingClientRect().width ?? | ||
| rects.reference.width; | ||
| elements.floating.style.width = `${width}px`; | ||
| }, | ||
| }), | ||
| ], | ||
| }); | ||
| sourcePopup.style.left = `${x}px`; | ||
| sourcePopup.style.top = `${y}px`; | ||
| }); |
Contributor
There was a problem hiding this comment.
I kind of question this, I get that it is nice to have the slip behavior, but other than that I feel like it is sort of overkill? I think we can simplify the rendering without this and just have it always render to the bottom.
Contributor
|
Alright @matthewlipski, so we decided on the React topic that it is still undecided for now, but we would like to write an example using React for the math block and then see from there whether we would want it to be the default implementation or not. I think we can move some of the logic that is currently in the render path right into the extension, but we can see how that plays out. |
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
3 participants
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
This PR adds a LaTeX math block. To facilitate that, changes have been made to the code block and how syntax highlighting is handled.
Code Block & Syntax Highlighting Changes
Code Block
The core code block remains unchanged functionally. It's just been made more composable as helpers for
render,parse, extensions, etc, have been extracted to separate files.Additionally, helper functions have been added to render previews for source code.
Syntax Highlighting
Syntax highlighting configuration has been moved out of the code block entirely and into a new
syntaxHighlightingeditor option. This has the following fields:createHighlighter: The same field as used to be in the code block options. Moved here as syntax highlighting can now be applied to multiple block types rather than just the code block.highlightBlock: Function which tells the editor for which blocks to apply syntax highlighting. Returning a language string will apply highlighting on that block for that language, while returning undefined will do nothing.This is done in order to only have a single highlighter plugin from
prosemirror-highlight, rather than having an instance per block. TheSyntaxHighlightingExtensionis now loaded by the editor whenever thesyntaxHighlightingeditor option is passed.Math Block
A math block has been added, which renders an equation from LaTeX, stored in its inline content. It uses the following dependencies:
temml: LateX to MathML for rendering & external HTML export.mathml-to-latex: MathML to LaTeX for external HTML parsing.The math block has been added as a separate package.
Rationale
We have wanted to implement a math block for a while. While existing PRs exist for that, this one has a broader scope which includes the above code block/syntax highlighting changes.
Changes
See above.
Impact
N/A
Testing
Added unit tests for
SyntaxHighlightingextension & math block.Screenshots/Video
TODO
Checklist
Additional Notes
Summary by CodeRabbit
Release Notes
New Features
Documentation
Internationalization