-
-
Notifications
You must be signed in to change notification settings - Fork 623
fix: Partial block to block conversion in HTML export #2089
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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;
|
packages/core/src/api/exporters/html/util/serializeBlocksExternalHTML.ts
Outdated
Show resolved
Hide resolved
I originally ran into this issue recently when working on #2071. The toggle list item uses 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. |
@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: |
: isPartialTableCell(content) | ||
? { | ||
type: "tableCell", | ||
content: ([] as InlineContent<T, S>[]).concat(content.content as any), |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
Summary
This PR fixes numerous runtime errors that could occur when passing
PartialBlock
arrays toblocksToFullHTML
/blocksToHTMLLossy
/blocksToMarkdownLossy
rather thanBlock
arrays. This is because we need fullBlock
objects when doing the conversion as this is the type that gets passed torender
/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 inserializeBlocksInternalHTML
/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
Additional Notes