Skip to content

Conversation

matthewlipski
Copy link
Collaborator

Summary

This PR fixes numerous runtime errors that could occur when passing PartialBlock arrays to blocksToFullHTML/blocksToHTMLLossy/blocksToMarkdownLossy rather than Block arrays. This is because we need full Block objects when doing the conversion as this is the type that gets passed to render/toExternalHTML, and were not doing the conversion properly.

Rationale

This issue wasn't really noticed as it's rare that a block's render/toExternalHTML functions will ever access anything aside from the block's props, which were previously correctly getting converted. However, it's a pretty major issue as the object being passed to those functions was basically half missing.

Changes

Implemented partialBlockToBlock function and used it in serializeBlocksInternalHTML/serializeBlocksExternalHTML.

Impact

N/A

Testing

Added a custom block to the unit test editor schema which validates that the block passed to render/toExternalHTML is of the correct shape. Added unit tests which export this block but specifically omit fields to ensure that they are fixed during the export.

Screenshots/Video

Checklist

  • Code follows the project's coding standards.
  • Unit tests covering the new feature have been added.
  • All existing tests pass.
  • The documentation has been updated to reflect the new feature

Additional Notes

Copy link

vercel bot commented Oct 8, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Updated (UTC)
blocknote Ready Ready Preview Oct 10, 2025 1:06pm
blocknote-website Ready Ready Preview Oct 10, 2025 1:06pm

@YousefED
Copy link
Collaborator

YousefED commented Oct 8, 2025

Thanks @matthewlipski . Could you give some background on how you ran into this / what was the background / trigger / scenario you're solving?

Asking this because;

  • I was revisiting the zod propschemas today, this seems relevant
  • maybe we should zoom out and look at the concept of "partial" more holistically

@matthewlipski
Copy link
Collaborator Author

Thanks @matthewlipski . Could you give some background on how you ran into this / what was the background / trigger / scenario you're solving?

Asking this because;

  • I was revisiting the zod propschemas today, this seems relevant
  • maybe we should zoom out and look at the concept of "partial" more holistically

I originally ran into this issue recently when working on #2071. The toggle list item uses block.children in its render function, and after adding the block to our list/default block tests, they were throwing an error. This is because serializeBlocksInternalHTML/serializeBlocksExternalHTML were only converting the props, and if no children were passed, block.children would be undefined while the toggle block's render expected an array.

In that PR, I made a quick fix for the children but left the other fields (id, type, and content) unconverted were since they were out of scope. But the whole conversion seemed like a major oversight so I wanted to get this PR out right away. Admittedly though, there are not many use cases I can think of where a block's ID or content should change how it's rendered/exported.

Copy link

pkg-pr-new bot commented Oct 10, 2025

Open in StackBlitz

@blocknote/ariakit

npm i https://pkg.pr.new/TypeCellOS/BlockNote/@blocknote/ariakit@2089

@blocknote/code-block

npm i https://pkg.pr.new/TypeCellOS/BlockNote/@blocknote/code-block@2089

@blocknote/core

npm i https://pkg.pr.new/TypeCellOS/BlockNote/@blocknote/core@2089

@blocknote/mantine

npm i https://pkg.pr.new/TypeCellOS/BlockNote/@blocknote/mantine@2089

@blocknote/react

npm i https://pkg.pr.new/TypeCellOS/BlockNote/@blocknote/react@2089

@blocknote/server-util

npm i https://pkg.pr.new/TypeCellOS/BlockNote/@blocknote/server-util@2089

@blocknote/shadcn

npm i https://pkg.pr.new/TypeCellOS/BlockNote/@blocknote/shadcn@2089

@blocknote/xl-ai

npm i https://pkg.pr.new/TypeCellOS/BlockNote/@blocknote/xl-ai@2089

@blocknote/xl-docx-exporter

npm i https://pkg.pr.new/TypeCellOS/BlockNote/@blocknote/xl-docx-exporter@2089

@blocknote/xl-email-exporter

npm i https://pkg.pr.new/TypeCellOS/BlockNote/@blocknote/xl-email-exporter@2089

@blocknote/xl-multi-column

npm i https://pkg.pr.new/TypeCellOS/BlockNote/@blocknote/xl-multi-column@2089

@blocknote/xl-odt-exporter

npm i https://pkg.pr.new/TypeCellOS/BlockNote/@blocknote/xl-odt-exporter@2089

@blocknote/xl-pdf-exporter

npm i https://pkg.pr.new/TypeCellOS/BlockNote/@blocknote/xl-pdf-exporter@2089

commit: 4ee7870

: isPartialTableCell(content)
? {
type: "tableCell",
content: ([] as InlineContent<T, S>[]).concat(content.content as any),
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nperez0111 what's up with the concat here? I get that it's to make sure it's an array, but does it matter since the items in the array have the wrong type anyway? Same for line 39

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's just to coerce it to always be an array, since a string is a valid inline content, this just gives it more structure than allowing just anything

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants