feat: math block#2857
Open
matthewlipski wants to merge 3 commits into
Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
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 selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe PR moves Shiki syntax highlighting from code block options to a new editor-level ChangesCode Block Refactor and SyntaxHighlighting Extension
New
Sequence Diagram(s)sequenceDiagram
participant User
participant BlockNoteView
participant createPreviewWithSourcePopup
participant FloatingUI
participant TemmlOrShiki as Temml / Shiki
User->>BlockNoteView: click on math/code preview
BlockNoteView->>createPreviewWithSourcePopup: mousedown on preview DOM
createPreviewWithSourcePopup->>createPreviewWithSourcePopup: show source popup, move cursor to end
createPreviewWithSourcePopup->>FloatingUI: autoUpdate(preview, popup, computePosition)
FloatingUI-->>createPreviewWithSourcePopup: positioned popup visible
User->>BlockNoteView: type LaTeX / code
BlockNoteView->>createPreviewWithSourcePopup: update(node) — source text changed
createPreviewWithSourcePopup->>TemmlOrShiki: re-render preview in place
User->>BlockNoteView: Arrow key out of block
BlockNoteView->>createPreviewSourceNavigationExtension: handleKeyDown
createPreviewSourceNavigationExtension-->>BlockNoteView: dispatch selection to adjacent block
createPreviewWithSourcePopup->>createPreviewWithSourcePopup: selection listener hides popup
createPreviewWithSourcePopup->>FloatingUI: stop autoUpdate
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
🚥 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 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 |
|
![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
+72
to
+80
| export function getLanguageId( | ||
| options: CodeBlockOptions, | ||
| languageName: string, | ||
| ): string | undefined { | ||
| return Object.entries(options.supportedLanguages ?? {}).find( | ||
| ([id, { aliases }]) => { | ||
| return aliases?.includes(languageName) || id === languageName; | ||
| }, | ||
| )?.[0]; |
Contributor
There was a problem hiding this comment.
Normalize alias matching to be case-insensitive.
getLanguageId currently does exact comparisons only. Inputs like TS/Js fall through to raw values and won’t map to canonical configured ids, which can skip expected highlighting/preview behavior.
Suggested fix
export function getLanguageId(
options: CodeBlockOptions,
languageName: string,
): string | undefined {
+ const normalizedLanguage = languageName.trim().toLowerCase();
return Object.entries(options.supportedLanguages ?? {}).find(
([id, { aliases }]) => {
- return aliases?.includes(languageName) || id === languageName;
+ return (
+ id.toLowerCase() === normalizedLanguage ||
+ aliases?.some((alias) => alias.toLowerCase() === normalizedLanguage)
+ );
},
)?.[0];
}📝 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
| export function getLanguageId( | |
| options: CodeBlockOptions, | |
| languageName: string, | |
| ): string | undefined { | |
| return Object.entries(options.supportedLanguages ?? {}).find( | |
| ([id, { aliases }]) => { | |
| return aliases?.includes(languageName) || id === languageName; | |
| }, | |
| )?.[0]; | |
| export function getLanguageId( | |
| options: CodeBlockOptions, | |
| languageName: string, | |
| ): string | undefined { | |
| const normalizedLanguage = languageName.trim().toLowerCase(); | |
| return Object.entries(options.supportedLanguages ?? {}).find( | |
| ([id, { aliases }]) => { | |
| return ( | |
| id.toLowerCase() === normalizedLanguage || | |
| aliases?.some((alias) => alias.toLowerCase() === normalizedLanguage) | |
| ); | |
| }, | |
| )?.[0]; | |
| } |
🤖 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/blocks/Code/CodeBlockOptions.ts` around lines 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.
Comment on lines
+19
to
+22
| code.className | ||
| .split(" ") | ||
| .find((name) => name.includes("language-")) | ||
| ?.replace("language-", ""); |
Contributor
There was a problem hiding this comment.
Tighten language-class detection to avoid false language IDs.
On Line 21, includes("language-") can match unrelated classes (for example, names containing language- mid-string), which can produce invalid parsed language values. Restrict matching to class names that start with language-.
🤖 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/blocks/Code/helpers/parse/parsePreCode.ts` around lines 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.
Comment on lines
+8
to
+25
| const language = block.props.language || options.defaultLanguage || "text"; | ||
|
|
||
| const pre = document.createElement("pre"); | ||
| const code = document.createElement("code"); | ||
| pre.appendChild(code); | ||
|
|
||
| const dom = document.createDocumentFragment(); | ||
|
|
||
| let removeSelectChangeListener: (() => void) | undefined; | ||
| if (options.supportedLanguages) { | ||
| const select = document.createElement("select"); | ||
| Object.entries(options.supportedLanguages).forEach(([id, { name }]) => { | ||
| const option = document.createElement("option"); | ||
| option.value = id; | ||
| option.text = name; | ||
| select.appendChild(option); | ||
| }); | ||
| select.value = language; |
Contributor
There was a problem hiding this comment.
Normalize the selected language id before assigning select.value.
Line 25 sets the dropdown value directly from block.props.language. If the stored value is an alias (e.g., js) or a stale id, the selector can render with no selected option even though the block has a language. Resolve to a canonical supported-language key (with alias fallback) before assigning select.value.
Suggested patch
- const language = block.props.language || options.defaultLanguage || "text";
+ const rawLanguage =
+ block.props.language || options.defaultLanguage || "text";
+ const language =
+ options.supportedLanguages &&
+ !(rawLanguage in options.supportedLanguages)
+ ? Object.entries(options.supportedLanguages).find(([, config]) =>
+ config.aliases?.includes(rawLanguage),
+ )?.[0] ?? (options.defaultLanguage || "text")
+ : rawLanguage;📝 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
| const language = block.props.language || options.defaultLanguage || "text"; | |
| const pre = document.createElement("pre"); | |
| const code = document.createElement("code"); | |
| pre.appendChild(code); | |
| const dom = document.createDocumentFragment(); | |
| let removeSelectChangeListener: (() => void) | undefined; | |
| if (options.supportedLanguages) { | |
| const select = document.createElement("select"); | |
| Object.entries(options.supportedLanguages).forEach(([id, { name }]) => { | |
| const option = document.createElement("option"); | |
| option.value = id; | |
| option.text = name; | |
| select.appendChild(option); | |
| }); | |
| select.value = language; | |
| const rawLanguage = | |
| block.props.language || options.defaultLanguage || "text"; | |
| const language = | |
| options.supportedLanguages && | |
| !(rawLanguage in options.supportedLanguages) | |
| ? Object.entries(options.supportedLanguages).find(([, config]) => | |
| config.aliases?.includes(rawLanguage), | |
| )?.[0] ?? (options.defaultLanguage || "text") | |
| : rawLanguage; | |
| const pre = document.createElement("pre"); | |
| const code = document.createElement("code"); | |
| pre.appendChild(code); | |
| const dom = document.createDocumentFragment(); | |
| let removeSelectChangeListener: (() => void) | undefined; | |
| if (options.supportedLanguages) { | |
| const select = document.createElement("select"); | |
| Object.entries(options.supportedLanguages).forEach(([id, { name }]) => { | |
| const option = document.createElement("option"); | |
| option.value = id; | |
| option.text = name; | |
| select.appendChild(option); | |
| }); | |
| select.value = language; |
🤖 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/blocks/Code/helpers/render/createSourceBlock.ts` around
lines 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.
Comment on lines
+482
to
+517
| position: absolute; | ||
| z-index: 1; | ||
|
|
||
| min-width: 240px; | ||
|
|
||
| background-color: rgb(22 22 22); | ||
| color: white; | ||
| border-radius: 8px; | ||
| box-shadow: 0 4px 12px rgba(0, 0, 0, 0.25); | ||
| } | ||
| /* The source popup reuses the default source rendering (language select + | ||
| `<pre>`), so it gets the same "code editor" styling as a regular code block. */ | ||
| .bn-code-block-source-popup > div > select { | ||
| outline: none !important; | ||
| appearance: none; | ||
| user-select: none; | ||
| border: none; | ||
| cursor: pointer; | ||
| background-color: transparent; | ||
|
|
||
| font-size: 0.8em; | ||
| color: white; | ||
|
|
||
| padding: 8px 16px 0; | ||
| } | ||
| .bn-code-block-source-popup > div > select > option { | ||
| color: black; | ||
| } | ||
| .bn-code-block-source-popup > pre { | ||
| white-space: pre; | ||
| overflow-x: auto; | ||
| margin: 0; | ||
| width: 100%; | ||
| tab-size: 2; | ||
|
|
||
| padding: 16px; |
Contributor
There was a problem hiding this comment.
Fix Stylelint declaration-empty-line-before violations in the popup block.
The current block contains empty lines before declarations (including Lines 485, 487, 502, 505, and 517), which triggers Stylelint errors. Remove those extra blank lines to keep CSS lint-clean.
🧰 Tools
🪛 Stylelint (17.13.0)
[error] 485-485: Expected no empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
[error] 487-487: Expected no empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
[error] 502-502: Expected no empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
[error] 505-505: Expected no empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
[error] 517-517: Expected no empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
🤖 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/editor/Block.css` around lines 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.
Source: Linters/SAST tools
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
+8
to
+11
| if (Array.isArray(block.content)) { | ||
| return block.content | ||
| .map((node) => ("text" in node ? node.text : "")) | ||
| .join(""); |
Contributor
There was a problem hiding this comment.
Guard non-object inline nodes before using the in operator.
Line 10 can throw at runtime when an array item is not an object ("text" in node on primitives/null). Since block.content is unknown, this path should be defensive instead of crashing preview/export.
Proposed fix
if (Array.isArray(block.content)) {
return block.content
- .map((node) => ("text" in node ? node.text : ""))
+ .map((node) =>
+ node && typeof node === "object" && "text" in node
+ ? String((node as { text?: unknown }).text ?? "")
+ : "",
+ )
.join("");
}📝 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
| if (Array.isArray(block.content)) { | |
| return block.content | |
| .map((node) => ("text" in node ? node.text : "")) | |
| .join(""); | |
| if (Array.isArray(block.content)) { | |
| return block.content | |
| .map((node) => | |
| node && typeof node === "object" && "text" in node | |
| ? String((node as { text?: unknown }).text ?? "") | |
| : "", | |
| ) | |
| .join(""); |
🤖 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/getMathSource.ts` around lines 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.
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
| 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?
| editor: BlockNoteEditor<any>, | ||
| ) => { | ||
| dom: HTMLElement; | ||
| ignoreMutation?: (mutation: ViewMutationRecord) => boolean; |
Collaborator
There was a problem hiding this comment.
if a user wants to implement their own preview, would they need to implement ignoreMutation? It's a very prosemirror-style (magic 🤯) API. Can we figure an API design that abstracts this out? (maybe same for dom and destroy actually)
YousefED
reviewed
Jun 17, 2026
YousefED
left a comment
YousefED
left a comment
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
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
2 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
New Features
Documentation