feat(core): support "plain" block content (BLO-335)#2868
Conversation
Adds a "plain" block content type for blocks that hold an unstyled text string (rather than rich inline content), and migrates the code block to use it. Plain content is `text*` with `marks: ""` at the ProseMirror level and a `string` in the block model. - Propagate "plain" through the block/inline-content type system (BlockFromConfig, PartialBlock, createBlockSpec, schema helpers) so a block/inline-content spec with `content: "plain"` infers `string`. - Wire the runtime: node spec (text*, marks: ""), nodeToBlock, createSpec parse/render, schemaToJSONSchema (plain -> string). - Update the default exporters (docx/odt/pdf/email) and the shared test util to read plain content as a string. - Add a Yjs pre-sync migration (stripDisallowedMarks) so legacy code blocks that carry marks load gracefully instead of being deleted by y-prosemirror; covers both pre-mount and after-mount-sync loads and tears down once migration is done. - Type-check tests/ via vp lint and add a root contributing guide. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughIntroduces a new Changes'plain' content type for code blocks
Yjs pre-sync migration for stripping disallowed marks
Sequence Diagram(s)sequenceDiagram
rect rgba(173, 216, 230, 0.5)
Note over SchemaMigration,Y.XmlFragment: Pre-sync migration setup
end
participant Editor
participant SchemaMigration
participant Y.XmlFragment
participant appendTransaction
Editor->>SchemaMigration: plugin init
SchemaMigration->>Y.XmlFragment: fragment.firstChild? → runPreSyncMigrations(preSyncMigrationRules)
SchemaMigration->>Y.XmlFragment: fragment.observeDeep(preSyncObserver)
Y.XmlFragment-->>SchemaMigration: remote update → preSyncObserver() → runPreSyncMigrations()
SchemaMigration->>Y.XmlFragment: stripDisallowedMarks: text.format(markKey, null)
appendTransaction->>SchemaMigration: first synced doc detected → stopPreSyncObserver()
Editor->>SchemaMigration: destroy() → stopPreSyncObserver()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 |
@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: |
|
|
Note that this PR takes the approach of mapping This means instantiated blocks can now hold a "string" as their content. An alternative would have been to map The advantage of the latter is that the type system of instantiated blocks is unaffected, while the former (what's implemented here) is more strictly typed. cc @nperez0111 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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`:
- Line 65: The code block content type has been updated to "plain" on line 65,
but the triple-backtick input-rule path still returns content as an array
instead of a string, causing a mismatch with the updated contract. Locate the
triple-backtick input-rule handler in this file (or in the input rules
configuration) and update its content payload to return a plain string value
instead of an array, ensuring consistency with the "plain" content type
declaration and preventing array content from leaking into string-expecting
paths.
In `@packages/core/src/yjs/extensions/schemaMigration/SchemaMigration.ts`:
- Around line 89-96: The stopPreSyncObserver() call halts all pre-sync migration
after the first sync without any version compatibility checking, leaving the
system vulnerable to silent document corruption if old-version peers send
disallowed content later. Either implement version negotiation and compatibility
gates at the collaboration provider level to ensure all peers are on compatible
versions before calling stopPreSyncObserver(), or conditionally keep the
pre-sync observer active when legacy client versions are detected to continue
filtering disallowed content from old-version peers throughout the session.
🪄 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: 71bae2d3-9dfb-4097-896b-d97bc44a08de
⛔ Files ignored due to path filters (16)
packages/xl-ai/src/api/schema/__snapshots__/schemaToJSONSchema.test.ts.snapis excluded by!**/*.snap,!**/__snapshots__/**tests/src/unit/core/formatConversion/exportParseEquality/__snapshots__/markdown/markdown/codeBlock.jsonis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/exportParseEquality/__snapshots__/markdown/markdown/complexDocument.jsonis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/exportParseEquality/__snapshots__/markdown/markdown/defaultBlocks.jsonis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/exportParseEquality/__snapshots__/markdown/markdown/specialCharEscaping.jsonis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/parse/__snapshots__/html/codeBlocks.jsonis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/parse/__snapshots__/html/codeBlocksMultiLine.jsonis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/parse/__snapshots__/markdown/codeBlockBasic.jsonis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/parse/__snapshots__/markdown/codeBlockIndented.jsonis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/parse/__snapshots__/markdown/codeBlockPython.jsonis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/parse/__snapshots__/markdown/codeBlockTildes.jsonis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/parse/__snapshots__/markdown/codeBlockWithLanguage.jsonis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/parse/__snapshots__/markdown/codeBlockWithSpecialChars.jsonis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/parse/__snapshots__/markdown/complexDocument.jsonis excluded by!**/__snapshots__/**tests/src/unit/core/formatConversion/parse/__snapshots__/markdown/headingThenCode.jsonis excluded by!**/__snapshots__/**tests/src/unit/core/schema/__snapshots__/blocks.jsonis excluded by!**/__snapshots__/**
📒 Files selected for processing (25)
CONTRIBUTING.mdpackages/core/src/api/blockManipulation/selections/textCursorPosition.tspackages/core/src/api/nodeConversions/nodeToBlock.tspackages/core/src/blocks/Code/block.test.tspackages/core/src/blocks/Code/block.tspackages/core/src/schema/blocks/createSpec.tspackages/core/src/schema/blocks/internal.tspackages/core/src/schema/blocks/types.tspackages/core/src/schema/contentTypePropagation.test.tspackages/core/src/schema/inlineContent/types.tspackages/core/src/yjs/extensions/schemaMigration/SchemaMigration.tspackages/core/src/yjs/extensions/schemaMigration/migrationRules/index.tspackages/core/src/yjs/extensions/schemaMigration/migrationRules/migrationRule.tspackages/core/src/yjs/extensions/schemaMigration/migrationRules/stripDisallowedMarks.test.tspackages/core/src/yjs/extensions/schemaMigration/migrationRules/stripDisallowedMarks.tspackages/core/src/yjs/utils.test.tspackages/xl-ai/src/api/schema/schemaToJSONSchema.tspackages/xl-docx-exporter/src/docx/defaultSchema/blocks.tspackages/xl-email-exporter/src/react-email/defaultSchema/blocks.tsxpackages/xl-email-exporter/src/react-email/reactEmailExporter.test.tsxpackages/xl-odt-exporter/src/odt/defaultSchema/blocks.tsxpackages/xl-pdf-exporter/src/pdf/defaultSchema/blocks.tsxshared/formatConversionTestUtil.tstests/src/utils/positionalMouse.tsvite.config.ts
| }, | ||
| }, | ||
| content: "inline", | ||
| content: "plain", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Update code-block input-rule payload to plain-string content.
After Line 65 switches code blocks to "plain", this file’s triple-backtick input-rule path still returns content: []. That no longer matches the code block contract and can leak array content into paths now expecting a string.
Suggested fix
return {
type: "codeBlock",
props: {
language: attributes.language,
},
- content: [],
+ content: "",
};🤖 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/block.ts` at line 65, The code block content
type has been updated to "plain" on line 65, but the triple-backtick input-rule
path still returns content as an array instead of a string, causing a mismatch
with the updated contract. Locate the triple-backtick input-rule handler in this
file (or in the input rules configuration) and update its content payload to
return a plain string value instead of an array, ensuring consistency with the
"plain" content type declaration and preventing array content from leaking into
string-expecting paths.
| // The initial document has now synced in and been migrated by the | ||
| // pre-sync observer (which runs before this, during the y-sync). New | ||
| // edits can't introduce content the schema rejects, so the observer | ||
| // is no longer needed — stop it instead of running on every later | ||
| // transaction. (Like the post-sync migration, this is one-shot: | ||
| // disallowed content arriving from an old-version peer *after* the | ||
| // first sync would not be re-migrated.) | ||
| stopPreSyncObserver(); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Read-only verification: check for protocol/client-version gating around collaboration sessions.
# Expected result: evidence of strict mixed-version prevention (or explicit compatibility policy).
rg -n -C3 --type=ts --type=tsx --type=js --type=jsx \
'\b(protocolVersion|schemaVersion|clientVersion|minVersion|compatibility|version gate|migration)\b' \
packages/core/src
# Inspect collaboration wiring and SchemaMigration integration points.
rg -n -C4 'withCollaboration\(|SchemaMigration\(|YSyncExtension\(' packages/core/srcRepository: TypeCellOS/BlockNote
Length of output: 17930
🏁 Script executed:
# Read the SchemaMigration.ts file to understand the observer teardown and context
cat -n packages/core/src/yjs/extensions/schemaMigration/SchemaMigration.ts | head -120Repository: TypeCellOS/BlockNote
Length of output: 5499
🏁 Script executed:
# Check for any version/compatibility checking in collaboration setup
rg -n 'version|Version|compatibility|Compatibility' packages/core/src/yjs/extensions/index.ts -A 5 -B 5Repository: TypeCellOS/BlockNote
Length of output: 158
🏁 Script executed:
# Look for deployment or environment-based guards for migrations
rg -n 'environment|ENV|process\.env|deployment|legacy|old.client|old.version' packages/core/src/yjs -iRepository: TypeCellOS/BlockNote
Length of output: 610
🏁 Script executed:
# Check the collaboration options type to see if version information is passed
ast-grep outline packages/core/src/yjs/extensions/index.tsRepository: TypeCellOS/BlockNote
Length of output: 586
🏁 Script executed:
# Check the migration rules to understand what content is considered disallowed
cat -n packages/core/src/yjs/extensions/schemaMigration/migrationRules/index.tsRepository: TypeCellOS/BlockNote
Length of output: 840
🏁 Script executed:
# Look at the stripDisallowedMarks rule mentioned in earlier tests
cat -n packages/core/src/yjs/extensions/schemaMigration/migrationRules/stripDisallowedMarks.ts | head -80Repository: TypeCellOS/BlockNote
Length of output: 3169
🏁 Script executed:
# Check if there's any documentation about version compatibility
find packages/core -name "*.md" -o -name "*.txt" | xargs rg -l "version|compatibility|deploy" 2>/dev/null | head -5Repository: TypeCellOS/BlockNote
Length of output: 246
🏁 Script executed:
# Search for any documented compatibility or version policy
rg -n "old.version|legacy|backward.compat|version.compat" packages/core -iRepository: TypeCellOS/BlockNote
Length of output: 453
🏁 Script executed:
# Check the README files for any documentation about version compatibility
cat packages/core/src/yjs/README.mdRepository: TypeCellOS/BlockNote
Length of output: 534
🏁 Script executed:
# Check if there's a main README with deployment/version guidance
cat README.md | head -100Repository: TypeCellOS/BlockNote
Length of output: 4436
🏁 Script executed:
# Look for any test cases that simulate old-version peer behavior
rg -n "old.*version|legacy.*sync|backward" packages/core/src/yjs/extensions/schemaMigration --type ts -A 5 -B 5Repository: TypeCellOS/BlockNote
Length of output: 2956
🏁 Script executed:
# Check if there's any provider/collaboration documentation about version restrictions
rg -n "version" packages/core/src/yjs/extensions/YSync.ts -B 2 -A 2Repository: TypeCellOS/BlockNote
Length of output: 158
🏁 Script executed:
# Check the moveColorAttributes post-sync migration rule
cat -n packages/core/src/yjs/extensions/schemaMigration/migrationRules/moveColorAttributes.ts | head -50Repository: TypeCellOS/BlockNote
Length of output: 2242
🏁 Script executed:
# Verify the exact check condition for when post-sync migration runs
cat -n packages/core/src/yjs/extensions/schemaMigration/SchemaMigration.ts | sed -n '67,87p'Repository: TypeCellOS/BlockNote
Length of output: 1205
Address mixed-version collaboration vulnerability—pre-sync observer stops after first sync without version gating.
The code acknowledges (lines 93–95) that disallowed content from old-version peers arriving after initial migration would not be re-migrated. However, no version negotiation, compatibility gate, or deployment policy exists to prevent such scenarios. After migrationDone is set, all migrations (pre- and post-sync) cease: appendTransaction returns early (line 69) and stopPreSyncObserver() halts further cleanup. If an old-version client sends disallowed marks in "plain" blocks after this point, y-prosemirror will reject and delete the node, silently corrupting the document across all peers.
Either implement version/compatibility checking at the collaboration provider level to prevent mixed-version sync, or keep pre-sync migration active when peers may include legacy clients.
🤖 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/yjs/extensions/schemaMigration/SchemaMigration.ts` around
lines 89 - 96, The stopPreSyncObserver() call halts all pre-sync migration after
the first sync without any version compatibility checking, leaving the system
vulnerable to silent document corruption if old-version peers send disallowed
content later. Either implement version negotiation and compatibility gates at
the collaboration provider level to ensure all peers are on compatible versions
before calling stopPreSyncObserver(), or conditionally keep the pre-sync
observer active when legacy client versions are detected to continue filtering
disallowed content from old-version peers throughout the session.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/yjs/extensions/schemaMigration/migrationRules/stripDisallowedMarks.test.ts`:
- Line 89: The arrow callback in the forEach method on the line with
Object.keys(op.attributes).forEach is using a concise arrow function body that
implicitly returns the value of remaining.add(k), which violates the Biome
linting rule requiring forEach callbacks to return void. Convert the concise
arrow callback to a block-bodied arrow callback by wrapping the remaining.add(k)
statement in curly braces, which will ensure the callback returns void instead
of the Set value.
🪄 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: eb09d941-7375-4485-94ca-8b7a11439ddd
📒 Files selected for processing (7)
packages/core/src/comments/mark.tspackages/core/src/extensions/tiptap-extensions/Suggestions/SuggestionMarks.tspackages/core/src/schema/blocks/createSpec.tspackages/core/src/schema/markGroups.test.tspackages/core/src/schema/markGroups.tspackages/core/src/yjs/extensions/schemaMigration/migrationRules/stripDisallowedMarks.test.tspackages/core/src/yjs/extensions/schemaMigration/migrationRules/stripDisallowedMarks.ts
✅ Files skipped from review due to trivial changes (2)
- packages/core/src/schema/markGroups.ts
- packages/core/src/extensions/tiptap-extensions/Suggestions/SuggestionMarks.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/core/src/yjs/extensions/schemaMigration/migrationRules/stripDisallowedMarks.ts
- packages/core/src/schema/blocks/createSpec.ts
| const remaining = new Set<string>(); | ||
| for (const op of delta) { | ||
| if (op.attributes) { | ||
| Object.keys(op.attributes).forEach((k) => remaining.add(k)); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and read around line 89
fd -t f "stripDisallowedMarks.test.ts" --exec cat -n {} \; 2>/dev/null | head -120 | tail -40Repository: TypeCellOS/BlockNote
Length of output: 1908
🏁 Script executed:
# Get the exact file and line
fd -t f "stripDisallowedMarks.test.ts" packages/Repository: TypeCellOS/BlockNote
Length of output: 251
🏁 Script executed:
# Check if Biome config exists and what rules are enabled
find . -name "biome.json*" -o -name ".biomerc*" | head -5Repository: TypeCellOS/BlockNote
Length of output: 158
🏁 Script executed:
# Search for Biome configuration or similar linting config
cat biome.json 2>/dev/null || cat biome.jsonc 2>/dev/null || echo "No biome config in root"Repository: TypeCellOS/BlockNote
Length of output: 182
🏁 Script executed:
# Search for the specific Biome rule about forEach callbacks
rg "no-useless-catch|no-useless-escape|noUnusedVariables|forEach" --type json --type jsonc biome.json* 2>/dev/null | head -20Repository: TypeCellOS/BlockNote
Length of output: 158
🏁 Script executed:
# Check if there are any ESLint/lint configs that might reference this rule
find . -maxdepth 3 -name "*.json" -o -name ".eslintrc*" | xargs grep -l "forEach\|callback" 2>/dev/null | head -10Repository: TypeCellOS/BlockNote
Length of output: 158
🌐 Web query:
Biome rule forEach callback should not return value
💡 Result:
In Biome, the rule responsible for checking return values in iterable callbacks is useIterableCallbackReturn [1]. By default, this rule mandates that callbacks passed to.forEach should not return a value [1]. Starting from version 2.4.0, Biome introduced a configuration option called checkForEach to control this behavior [2][1]. When checkForEach is set to true (which is the default behavior), the rule will report an error if a.forEach callback returns a value [2][1]. If you prefer to allow return values in.forEach callbacks—for instance, when using concise arrow functions that unintentionally return a value—you can disable this check by setting checkForEach to false in your biome.json configuration file [2][1]: { "linter": { "rules": { "suspicious": { "useIterableCallbackReturn": { "options": { "checkForEach": false } } } } } } This configuration brings Biome's behavior in line with typical ESLint practices for this rule, where ignoring the return value of forEach is often preferred to avoid false positives in idiomatic code [3][4][2].
Citations:
- 1: https://biomejs.dev/uk/linter/rules/use-iterable-callback-return/
- 2: biomejs/biome@a9025d4
- 3: feat: add the checkForEach option to useIterableCallbackReturn biomejs/biome#8289
- 4: useIterableCallbackReturn should be split into two separate rules biomejs/biome#8005
🌐 Web query:
TypeScript Set.prototype.add return value forEach callback
💡 Result:
The return value and callback behavior for Set methods are as follows: Set.prototype.add(value) returns the Set object itself [1][2]. This design allows for method chaining (e.g., mySet.add(1).add(2)) [1][2]. Set.prototype.forEach(callbackFn, thisArg) executes the provided callback function once for each element in the Set [3][4]. This method does not return a value (it returns undefined) [3][5][6]. The callback function provided to forEach is invoked with three arguments [3][4]: 1. The element value: The current value being processed [3][4]. 2. The element key: In a Set, this is identical to the element value [3][4][6]. This argument is provided to maintain signature consistency with Map.prototype.forEach and Array.prototype.forEach [3][4][6]. 3. The Set object: The Set being iterated over [3][4].
Citations:
- 1: https://devdoc.net/web/developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set/add.html
- 2: https://github.com/mdn/content/blob/main/files/en-us/web/javascript/reference/global_objects/set/add/index.md
- 3: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set/forEach
- 4: https://github.com/mdn/content/blob/main/files/en-us/web/javascript/reference/global_objects/set/foreach/index.md
- 5: https://devdoc.net/web/developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set/forEach.html
- 6: http://www.xahlee.info/js/js_set_prototype_foreach.html
🏁 Script executed:
# Search the codebase for similar patterns and how they're handled
rg "forEach\s*\(\s*\([^)]+\)\s*=>" packages/core/src --type ts --type tsx -A 1 | head -40Repository: TypeCellOS/BlockNote
Length of output: 186
🏁 Script executed:
# Search for similar forEach patterns in the codebase (ts files only)
rg "forEach\s*\(\s*\([^)]+\)\s*=>" packages/core/src --type ts -A 1 -B 1 | head -50Repository: TypeCellOS/BlockNote
Length of output: 4044
Avoid returning a value from the forEach callback.
remaining.add(k) returns the Set, making this concise arrow callback return a value, which violates the Biome rule. Use a block-bodied callback to ensure it returns void.
Proposed fix
- Object.keys(op.attributes).forEach((k) => remaining.add(k));
+ Object.keys(op.attributes).forEach((k) => {
+ remaining.add(k);
+ });📝 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.
| Object.keys(op.attributes).forEach((k) => remaining.add(k)); | |
| Object.keys(op.attributes).forEach((k) => { | |
| remaining.add(k); | |
| }); |
🧰 Tools
🪛 Biome (2.5.0)
[error] 89-89: This callback passed to forEach() iterable method should not return a value.
(lint/suspicious/useIterableCallbackReturn)
🤖 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/yjs/extensions/schemaMigration/migrationRules/stripDisallowedMarks.test.ts`
at line 89, The arrow callback in the forEach method on the line with
Object.keys(op.attributes).forEach is using a concise arrow function body that
implicitly returns the value of remaining.add(k), which violates the Biome
linting rule requiring forEach callbacks to return void. Convert the concise
arrow callback to a block-bodied arrow callback by wrapping the remaining.add(k)
statement in curly braces, which will ensure the callback returns void instead
of the Set value.
Source: Linters/SAST tools
matthewlipski
left a comment
matthewlipski
left a comment
There was a problem hiding this comment.
Looks good, main question is r.e. NON_FORMATTING_MARK_GROUP above. Will also need to remember to update docs.
"plain" blocks (e.g. code blocks) previously disallowed all marks, which also blocked comments and suggestions/diffs. Allow the non-formatting marks while still excluding formatting. - Add a shared "annotation" mark group to the comment mark and the suggestion marks (insertion/deletion/modification); plain blocks allow that group. The always-present suggestion marks keep the group non-empty, so the reference resolves even when comments aren't configured. - Make the Yjs stripDisallowedMarks migration schema-driven: it strips the marks a node's type disallows (NodeType.allowsMarkType) and keeps the rest, so legacy code blocks retain their comments/suggestions. It also resolves y-prosemirror's overlapping-mark keys (`name--<hash>`, used for comments) back to the base mark name before the schema lookup. - Run the pre-sync migration before y-sync (runsBefore: ["ySync"]) so it cleans the fragment before y-prosemirror reconstructs (and deletes) invalid nodes. - Tests build fixtures through real editors so marks are encoded exactly as y-prosemirror writes them, including a real comment (`comment--<hash>`). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
YousefED
force-pushed
the
feat/plain-block-content
branch
from
2990657 to
9ab5ffe
Compare
YousefED
changed the title
nperez0111
left a comment
nperez0111
left a comment
There was a problem hiding this comment.
I'm a little wary of making "plain" only output text instead of just an inline content, but I guess I might just be over thinking it. If "plain" is expected to ONLY contain plaintext then it is fine, but things like comments can never be represented in the JSON format then (which is intentional now, but unsure if we have cases in the future for this)
| }, | ||
| }, | ||
| content: "inline", | ||
| content: "plain", |
| // Plain blocks hold unstyled text only, so we parse the | ||
| // element's text content directly into a single text node. | ||
| const text = (node as HTMLElement).textContent ?? ""; | ||
| return text ? Fragment.from(schema.text(text)) : Fragment.empty; |
There was a problem hiding this comment.
I don't think this is accurate, it would drop comments no? I think we need to parse similar to inline & just use a DOMParser & let it choose what it understands or not
| const markKeys = new Set<string>(); | ||
| for (const op of text.toDelta() as { | ||
| attributes?: Record<string, unknown>; | ||
| }[]) { | ||
| if (op.attributes) { | ||
| for (const key of Object.keys(op.attributes)) { | ||
| markKeys.add(key); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Hm, there are better ways to do this. I'm unsure if this is technically correct in all cases or if we should be iterating the text elements instead
| // y-prosemirror encodes "overlapping" marks (those that don't exclude | ||
| // themselves, e.g. comments) as `${markName}--${hash}` so multiple | ||
| // instances can coexist on a range. Resolve the base mark name before | ||
| // looking it up in the schema. | ||
| const markName = key.split("--")[0]; |
There was a problem hiding this comment.
We can do this the same way as y-prosemirror, this isn't the most robust
| // here — in the extension factory, before the editor mounts — so it runs | ||
| // before y-prosemirror's own observer and can clean the fragment first. | ||
| // Removed once migration is done (see `appendTransaction`). | ||
| fragment.observeDeep(preSyncObserver); |
There was a problem hiding this comment.
Oof, I'm unsure about this. This is just asking for a race condition. This isn't a very robust migration
3 participants
Summary
Adds a "plain" block content type — for blocks that hold an unstyled text string rather than rich inline content — and migrates the code block to use it. At the ProseMirror level a plain block is
text*; in the block model itscontentis astring. Plain blocks disallow formatting marks but allow the non-formatting marks (comments and suggestions/diffs).Rationale
Some blocks (code blocks being the canonical example) should hold plain text only: no bold/italic/links, no inline-content array to reason about. Before this, the only options were
"inline"(rich content) or"none"(no content)."plain"fills that gap with a simpler, string-typed content shape — while still supporting comments and tracked-changes on the block's text.Changes
"plain"throughBlockFromConfig/PartialBlock,createBlockSpec, and the schema helpers so a block/inline-content spec declared withcontent: "plain"infersstring.text*;nodeToBlockreturns the text content as a string;createSpecparse/render handle plain;schemaToJSONSchemamapsplain → { type: "string" }(xl-ai).content: "plain"."annotation"mark group is added to the comment mark and the suggestion marks (insertion/deletion/modification); plain blocks allow that group, so formatting marks are excluded but comments and suggestions/diffs are not. The always-present suggestion marks keep the group non-empty, so the reference is valid even when comments aren't configured (the comment mark is conditional).stripDisallowedMarks). Legacy code blocks whose text carries formatting marks are otherwise deleted by y-prosemirror during reconstruction — and the deletion propagates to all peers. The migration strips only the disallowed (formatting) marks from theY.XmlFragmentbefore y-prosemirror touches it (keeping comments/suggestions), covering both documents present before mount and content that syncs in afterwards, and tears down once migration is done.tests/is now type-checked/linted byvp lint; added a Commands section toCONTRIBUTING.md.Impact
"inline"/"none"/"table"blocks are unaffected (additive to the content union)."annotation"group, which assumes the always-present suggestion marks keep it non-empty.Testing
contentTypePropagation.test.ts: type-level assertions that"plain"(and the baseline kinds) propagate to the rightcontenttype for blocks and inline content. Verified it fails to compile if propagation regresses.markGroups.test.ts: a code block allowsinsertion/deletion/modification(andcommentwhen comments are enabled) but notbold.stripDisallowedMarks.test.ts: rule unit tests + the two real collaborative flows (formatted code block present before mount, and synced in after mount) survive with content intact and the Yjs fragment uncorrupted; the migration keeps comment + suggestion marks while stripping formatting; the observer is removed once migration completes.tests/parse/export-equality JSON, xl-ai schema snapshot) to the new string shape.@blocknote/coresuite green; exporter, xl-ai, and xl-multi-column suites green.Screenshots/Video
N/A — no visual changes.
Checklist
Additional Notes
Plain blocks deliberately disallow formatting marks (so code blocks can't be bolded/linked) but allow the non-formatting marks — comments and suggestions/diffs — via a shared
"annotation"mark group. This keeps the "plain = unstyled string" semantics at the block-model level (those marks areblocknoteIgnoreand don't appear in the block's content) while letting collaboration features work on code.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation