Skip to content
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

feat: use slim shiki build and pre-compiled languages #1519

Merged
merged 20 commits into from
Mar 26, 2025
Merged

Conversation

nperez0111
Copy link
Contributor

This significantly cuts down on the bundle size of the package by leveraging shiki's new slim builds and precompiled languages

Fixes #1487 #1213

This could probably be reduced further by completely separating the default languages, but for this would require a breaking change to the API for code blocks to properly work.

The ideal would be for the user to specifically pick which languages and themes for the code block feature, rather than us providing any defaults for this.

Perhaps we can have defaults built into a separate package? To make it easier for those already using us?

Either way, this would require some thought, while the current change would still be compatible with the current API

Copy link

vercel bot commented Mar 11, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
blocknote ✅ Ready (Inspect) Visit Preview Mar 26, 2025 9:23am
blocknote-website ✅ Ready (Inspect) Visit Preview Mar 26, 2025 9:23am

Copy link
Collaborator

@YousefED YousefED left a comment

Choose a reason for hiding this comment

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

This is great!

If I understand it correctly; this mostly makes sure unused languages that are not in our "defaultSupportedLanguages" don't appear in the app output. However, languages that are, of course still appear in the app output. correct?

The ideal would be for the user to specifically pick which languages and themes for the code block feature, rather than us providing any defaults for this.

Perhaps we can have defaults built into a separate package? To make it easier for those already using us?

I think it's worthwhile to tackle the issue completely and invest a bit more to also address this.

  • I think it's ok to make this a breaking changes as long as it's clearly documented
  • if it's not too much overhead (in code / docs / maintenance) I think having some sensible defaults easily available makes sense indeed
  • we should decide whether by default we don't want any syntax highlighting at all, or only support 1-3 langs
  • while we're at it, maybe we should also make it easy for consumers to load other themes

@flipvh
Copy link

flipvh commented Mar 13, 2025

Great news. My biased opinion would be to not add any and let the consumers decide to include one or multiple somehow. I would see the code block as a more 'advanced functionality' that will be used by consumers that should be fine with doing some config. If you throw in an example of that on your site, the first round of beers in de Biergarten are on me 🍻 😸.

@nperez0111
Copy link
Contributor Author

nperez0111 commented Mar 14, 2025

Alright, I've exposed the codeBlock configuration on the editor instance, and so this would let you configure the languages & themes that are supported by your app. By default, no languages or themes are bundled, to configure syntax highlighting, you can install the new package @blocknote/code-block which provides a decent set of languages and light & dark themes to use.

If you want more control than this, I'll write a guide on how easy it will be to integrate your list of languages, using the shiki-codegen tool.

This isn't completely ready yet, as I'll need to:

Copy link
Collaborator

@YousefED YousefED left a comment

Choose a reason for hiding this comment

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

awesome!! only some comments / qs about naming and docs

<div class="bn-block-group" data-node-type="blockGroup"><div class="bn-block-outer" data-node-type="blockOuter" data-id="1"><div class="bn-block" data-node-type="blockContainer" data-id="1"><div class="bn-block-content" data-content-type="codeBlock"><pre><code class="bn-inline-content">console.log('Hello, world!');</code></pre></div></div></div></div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

is it easy / needed to add tests whether the exporting / importing to / from md / html still works for custom languages?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we have some existing tests for this: https://github.com/TypeCellOS/BlockNote/blob/013818d40a2fedaf192ddc7021a8457b2eac37bc/packages/core/src/api/exporters/html/__snapshots__/codeBlock/python/internal.html#L0-L1

Just didn't need to modify it because it was only the default language that changed (it went from being javascript by default to text)

/**
* Options for code blocks.
*/
codeBlock?: CodeBlockOptions;
Copy link
Collaborator

Choose a reason for hiding this comment

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

minor thought; is it confusing that the term codeBlock is used for all of:

  • the blocknote block (defined in core)
  • the option for the theme / highlighting (defined here)
  • the package for advanced theme / highlighting

Copy link
Contributor Author

@nperez0111 nperez0111 Mar 25, 2025

Choose a reason for hiding this comment

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

codeBlock (this option) makes sense as it is the bundle of options for configuring the code-block block type (like tables)
@blocknote/code-block(the package) I think could maybe be renamed to @blocknote/defaults with an export of codeBlock which could make sense if we decide to add more things like this. Or, similar.

@@ -0,0 +1,33 @@
# Code Block Highlighting
Copy link
Collaborator

Choose a reason for hiding this comment

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

The pattern we currently use is that we have:

  • max 1 or 2 paragraphs in the example as description
  • an optional "try it out" line to explain how the user can see the functionality in the demo
  • links to relevant docs

The idea is that the examples can really be a larger set of examples to click through and play with without too much reading - and that all documentation should be in /docs.

Could we change this? or lmk if you like a different setup

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Totally makes sense. Updated

@YousefED YousefED requested a review from areknawo March 25, 2025 05:46
Copy link
Collaborator

@matthewlipski matthewlipski left a comment

Choose a reason for hiding this comment

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

Looks good! Only a few suggestions

},
initialContent: [
{
id: "fc832df4-bd15-49d2-8d64-140c27f29692",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same thing as previous comment r.e. using PartialBlock[] instead of Block[]

```javascript
console.log('Hello, world!');
```
console.log('Hello, world!');
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure we can really do much about it either (since the HTML -> MD conversion is not handled by us), but this seems to be missing the code block formatting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea, I spent like 30 minutes looking into it. It does not want to properly convert it if it does not have a language specified

Copy link
Collaborator

Choose a reason for hiding this comment

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

Would it make sense to render a language-text attribute to the HTML to fix this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will just output language-text, I don't see the harm in it

This example proves it should work fine

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.

BlockNote codeBlock's embedded Shiki significantly increases bundle size
4 participants