skeleton icon indicating copy to clipboard operation
skeleton copied to clipboard

Resolve issues with Highlight.js and CodeBlock export

Open endigo9740 opened this issue 1 year ago • 1 comments

NOTE: moving this here to track progress

For reference - https://github.com/highlightjs/highlight.js/issues/3527 there is a screen grab showing the exact error we get.

I've also gone ahead and asked them for any advice https://github.com/highlightjs/highlight.js/issues/3597

Originally posted by @niktek in https://github.com/Brain-Bones/skeleton/issues/83#issuecomment-1214314346

endigo9740 avatar Aug 14 '22 21:08 endigo9740

@niktek Not sure if you saw the reply in your your support message linked above. The CDN version sounds interesting. I'm still thinking we go ahead and find a means to move the dependency outside or separate from the component. However, I'd like to also consider looking at Prism as well. It's more widely used than Highlight, even though I think they each have pros and cons. Neither seems to have embraced true ESM support yet, which is a bummer.

endigo9740 avatar Aug 15 '22 14:08 endigo9740

Unsure if this is related, but I am getting this error

Uncaught (in promise) SyntaxError: The requested module '/node_modules/highlight.js/lib/index.js?v=299b7da5' does not provide an export named 'default' (at index.js?v=299b7da5:2:8)

Now when I try to import any of the Components into a different project than recently discussed

luke-hagar-sp avatar Aug 17 '22 19:08 luke-hagar-sp

Highlight.js is a syntax highlighting library we used for CodeBlock components. Given the number of issues (like this) it's slated to be removed in the next release. See the pinned ticket here: https://github.com/Brain-Bones/skeleton/issues/86

This is part of a bigger effort to decouple Skeleton from SvelteKit. Essentially the live/production version only really plays well in a SvelteKit project. For anything else there was issues: https://github.com/Brain-Bones/skeleton/issues/68

All fixes for this are already in place and ready to go with the next deployment, but they need to be properly tested before that occurs. Sorry if these are causing trouble for you in the meantime!

endigo9740 avatar Aug 17 '22 19:08 endigo9740

I see, I am using the code block quite heavily on my site at the moment here: https://lukehagar.com/plex-api-oauth so I will have to figure out a solution.

But the error that I put above I was getting when I tried to Import Cards or Buttons or anything, and this is in a Standard SvelteKit Project

luke-hagar-sp avatar Aug 17 '22 20:08 luke-hagar-sp

What I might advise is implementing highlight.js directly for now. I found it pretty easy to setup per their instructions: https://highlightjs.org/usage/

Then you can reference the source for the CodeBlock here: https://github.com/Brain-Bones/skeleton/blob/dev/src/lib/CodeBlock/CodeBlock.svelte

The CodeBlock component will return, but not until we can investigate the best manner to handle it. The way it's implemented now has too many edge cases to account for.

endigo9740 avatar Aug 17 '22 20:08 endigo9740

@luke-hagar-sp made a great attempt at migrating to Prism, but it's had it's own slew of of issues: https://github.com/Brain-Bones/skeleton/pull/133 https://discord.com/channels/1003691521280856084/1010384499877629984

I'm going to go ahead and assign this issue to myself, as I feel like we're approaching this problem from the wrong manner. Rather than trying to cram a syntax highlighting library into our components, we should allow them to live alongside it as a progressive enhancement.

Something like:

  • Install Skeleton
  • You'll have a basic, non-highlighted code block component
  • Install either Highlight or Prism, configure them
  • Then implement a setting on the component to enable highlighting for your project.

The main thing that's causing us trouble is that both highlighting libraries are very-DOM focused. They rely on settings that should be applied in a global scope to search for code block elements and run their processing methods on them. Likewise, I think both Highlight and Prism are a bit old school and not yet caught up to the ESM import methods for doing things. This is what has led to a lot of the problems we've encountered when folks use Skeleton's package.

I think the progressive approach would be the best overall. It simplifies the component, and makes the extra dependency optional for users to choose to accept - rather than it being forced on them. I would prefer that myself for my projects! If someone is not using code block they don't get force fed an extra dep they won't want!

endigo9740 avatar Aug 20 '22 17:08 endigo9740

https://svelte.dev/repl/3fef33867c47431b8f4edc99275640e8?version=3.12.1

This may be helpful

luke-hagar-sp avatar Aug 21 '22 02:08 luke-hagar-sp

It's not unlike what I've tried in the past. The dynamic language loading is a nice touch, but may actually be a bit annoying if you have lots of different code blocks on the page.

Since there's been a lot of interest in this one I'll dedicate some time to it tomorrow. Can't promise it'll be resolved by next release though. Especially since we have so many issues ready to be tested! That stuff must take priority the next few days.

endigo9740 avatar Aug 21 '22 02:08 endigo9740

I've managed to successfully decouple Highlight.js from the CodeBlock component. This is ready to test on the feat/codeblock-v2 branch. Optionally can use this command with the GitHub CLI to make this process trivial:

gh pr checkout 138

Notes:

  • Highlight.js is no longer a dependency of Skeleton.
  • If you wish to use the the CodeBlock it will have no syntax highlighting by default (just black and white)
  • You can optionally install and configure Highlight.js per the instructions in the CodeBlock docs
  • Syntax highlighting will then work as expected.

How it Works:

We now allow the user to choose to install and import Highlight in the root of their project. They setup their highlight.js CSS theme same as before. The difference is now you pass a reference to the Highlight.js import to a special writable store that lives alongside the CodeBlock. It looks like this:

export const storeHighlightJs: Writable<any> = writable(undefined);

The CodeBlock checks for the presence of this store and if it has a value. If so it'll automatically trigger the following in the component:

function highlight(): void {
    if ($storeHighlightJs === undefined) return;
    code = $storeHighlightJs.highlight(code, { language }).value;
}

Essentially we're optionally passing a reference to hljs to the store. Then the component reacts accordingly. Since we're defining a dedicated store for Highlight.js, this means we could add similar store references for Prism and other syntax tools. However, I think this is beyond the initial scope. Let's just get the basics working first!

Create a Test Package

If you wish to test this yourself, you'll need to create a package yourself. Here's instructions for doing this:

  1. Pull down the branch and run the following in the root of the Skeleton project npm run package
  2. Move into the package directory via cd package
  3. Create a local tarball (like a zip) via: npm pack
  4. Cut and paste the .tgz file into the root of your new Skeleton test project
  5. Finally run npm install {tarballFile}.tgz to install locally
  6. Follow all instructions in the documentation provided.

Keep in mind this is a package build based on the dev branch, which is not fully tested and not ready for production. This should not be used for a production environment.

What to Test

  • Do you get errors when installing Skeleton without Highlight.js installed?
  • Do you get errors when Highlight.js is installed, but not configured?
  • Do you get errors when Highlight.js is installed and configured?
  • Is there any scenario with the configuration that causes errors to occur?
  • Can you navigate page to page without highlighting breaking?
  • Can you trigger Vite's HRM when working on other pages or components that breaks highlighting?
  • Is there any other condition that exists that can break highlighting?

Known Issues:

  • When editing code within the CodeBlock component itself in the Skeleton project, it'll sometimes run Highlight.js's parser multiple times, meaning you get scrambled looking code in the blocks. A page refresh will fix this. Given how rare this scenario is I won't aim to make adjustments yet. Let's just see if we can get the basics working!

Thanks for your help in testing this! Note that we're not aiming to release this in the next upcoming release, but instead the one after that. Thanks for your patience in the meantime!

endigo9740 avatar Aug 21 '22 21:08 endigo9740

Still getting * import error when importing any component if the CodeBlock is included in the index.ts export list in the resultant package. :(

niktek avatar Aug 23 '22 05:08 niktek

Ok, well CodeBlock won't be part of the next release so we'll have time to test further, but very odd. There should be no import in the component on this branch. It's only part of the layout at that point.

Do my favor and shoot me a copy of your layout file and the package you built for this on here or Discord please. I'll debug tomorrow. Thanks!

endigo9740 avatar Aug 23 '22 06:08 endigo9740

Here's a new suggestion from @niktek to review: https://shiki.matsu.io/

endigo9740 avatar Aug 23 '22 16:08 endigo9740