Syntax highlighting fails in React applications when using the new highlightAll() method

[jammykam]

Describe the issue
I am trying to use highlightjs on a React project, which uses Next.js (although I don't think the issue is related). I have added highlight.js using npm (v106.0) and loaded in a style sheet for a theme.

I have a simple function component set up like this:

import hljs from "highlight.js";
function Post(props) {
  useEffect(() => {
    hljs.highlightAll();
  }, []);

  return (
    <>
      <pre>
        <code> /* code blocks that i want to highlight */ </code>        
      </pre>
    </>
  );
}

This is using the new highlightAll() method call, but when the component is loaded the highlighting fails to take effect, and the <pre><code> block is left unstyled.

Expected behavior
I would expect the code block to be styled and the language highlighted using highlightjs and the stylesheet I have selected.

Which language seems to have the issue?
The error is not language specific, but all code blocks are failing to get instantiated and highlighted, and any styling applied what so ever.

What I think the issue is
I think the issue is that the new highlightAll() is not compatible with React and the way that React shadow loads components. Line #772 makes a check if the dom has loaded, else it breaks out:

  function highlightAll() {
    // if we are called too early in the loading process
    if (!domLoaded) { wantsHighlight = true; return; }

    const blocks = document.querySelectorAll('pre code');
    blocks.forEach(highlightElement);
  }

The only place that domLoaded is set to true is in the boot() method:

  function boot() {
    domLoaded = true;
    // if a highlight was requested before DOM was loaded, do now
    if (wantsHighlight) highlightAll();
  }

The problem is that the only place boot() is called from is an event listener:

  if (typeof window !== 'undefined' && window.addEventListener) {
    window.addEventListener('DOMContentLoaded', boot, false);
  }

Since React will not cause the fire of the DomContentLoaded event when a user navigates to a new page, boot() is never called to set this variable to true, and when I am calling highlightAll() from my code it's is exiting too early. The boot() is also not exposed to be able to call directly from my code.

Workaround
For now, the solution is to call hljs.initHighlighting() (Line 747) since that method does not make the domLoaded check.

But, this method is marked as deprecated, so a fix needs to be put in place to allow this highlighter to continue to work with React applications. Perhaps allow a true parameter to be passed to force it to run the syntax highter code.

[joshgoebel]

For now, the solution is to call hljs.initHighlighting() (Line 747) since that method does not make the domLoaded check.

Yep, that's why it was deprecated (not removed)... because the world is a big place and surely there were edge cases. Thanks for the detailed bug report.

which uses Next.js (although I don't think the issue is related)

The issue is the server-side rendering I think. In a browser DOMContentLoaded is always fired eventually (after the page is loaded). Do you have a way of confirming to what level Next.js is emulating the DOM? Ie, we use jsdom for our own Node.js simulated "browser" testing... It has always seemed to me that these simulated environments should simply fire the DOMContentLoaded trigger when the page is ready... rather than forcing us to deal with the differences between rendering on the server-side vs the client.

That's exactly what Next.js is supposed to be abstracting away for us, right?

[joshgoebel]

@jammykam Could you answer the first 5 questions of the Next.js GitHub issue report for me (which versions of everything you're using) and I'll open an issue and link to this? I'd like to understand why this can't be resolved on their end (to better simulate the browser environment).

We may end up solving this as only a docfix and refer you to: https://github.com/highlightjs/highlight.js#custom-scenarios Frameworks that subvert the traditional browser loading process may simply require manual startup. The whole point of that PR was to simplify the browser usage for most - avoiding two functions with confusing names.

[joshgoebel]

Could you confirm if you are doing SSR (server side rendering) or not?

[joshgoebel]

If you aren't doing SSR could you try the following patch to our bootstrap around line 788:

  const LOADED_STATES = ["interactive", "complete"];
  if (typeof window !== 'undefined' && window.addEventListener) {
    window.addEventListener('DOMContentLoaded', boot, false);
    if (LOADED_STATES.includes(document.readyState)) { domLoaded = true; }
  }

This should handle the case of someone adding us to the mix FAR after the page has been initialized/loaded.

[jammykam]

Hey @joshgoebel, thanks for the quick reply! 🙌

The issue is the server-side rendering I think. In a browser DOMContentLoaded is always fired eventually (after the page is loaded). Do you have a way of confirming to what level Next.js is emulating the DOM?

In this case we are running SSG (static site generation, sorry should have mentioned that), so it should be running as a regular SPA React app. There's an added complexity in our case since the content is dynamic and pulled from a CMS. In any case, I belive this to be an issue in pure React.

If you aren't doing SSR could you try the following patch to our bootstrap around line 788

This not not work unfortunately 🙁

I still think that this is an eventing issue that does not work for React or SPA applications. AFAIK the DOMContentLoaded event fires exactly once when a page loads:

https://developer.mozilla.org/en-US/docs/Web/API/Window/DOMContentLoaded_event

The DOMContentLoaded event fires when the initial HTML document has been completely loaded and parsed, without waiting for stylesheets, images, and subframes to finish loading.

For React applications this is the first page load. From there on when a user navigates to another page, the page partially reloads through the shadow update of DOM elements. Since the page is not actually reloaded, the DOMContentLoaded event does not fire on subsequent pages.

If i comment out if (!domLoaded) { wantsHighlight = true; return; } from highlightAll() it works as expected.

Alas initHighlighting() did not end up working as well as expected. It works the first time we visit a page with HLJS included (in our case, we only include this library on blog pages). So first blog page we visit it works, but second blog page the highlighting does not fire due to the if (initHighlighting.called) return; check - this has been set to true on our first blog page load and on the 2nd/3rd/etc pages this value has not been reset due to the same issue as above in React applications.

I have however been able to get this work correctly and as expected by simply updating my call to do what the highlightAll() function itself is doing.

Perhaps updating the documentation with this info will be sufficient?

import hljs from "highlight.js";
function Post(props) {
  useEffect(() => {
    const blocks = document.querySelectorAll('pre code');
    blocks.forEach(hljs.highlightBlock);
  }, []);

  return (
    <>
      <pre>
        <code> /* code blocks that i want to highlight */ </code>        
      </pre>
    </>
  );
}

I can create a simple example to demonstrate this of you if you still want though (I'm a NextJS newbie and a React novice).

[joshgoebel]

Since the page is not actually reloaded, the DOMContentLoaded event does not fire on subsequent pages.

Yes, I realized afterwards that this could be the issue rather than SSR, hence my follow-up post.

If i comment out if (!domLoaded) { wantsHighlight = true; return; } from highlightAll() it works as expected.

Could you please try the fix I posted instead? #3020 (comment)

Oh you did... hmmmm... that's strange.

[joshgoebel]

I can create a simple example to demonstrate this of you if you still want though (I'm a NextJS newbie and a React novice).

A tiny Github repo that I could build that reproduces just the React/SSG portion of the issue would be great.

I wonder what document.readyState is when HLJS is loaded and why that didn't fix things? I'm still confused whether you're running too early, or too late, or a combination of BOTH.

[joshgoebel]

Ping.

[trm217]

I am using highlight.js with Next.js 11 for a blog. It works perfectly fine for me, the only trouble I was having is regarding JSX/TSX code. That for I doubt that this is a general issue.

Can you share the page where you're trying to use highlicht.js (the entire file with the imports etc. would be the most helpful)

[trm217]

After checking the code you shared when opening the issue: Where are you importing a theme to style the code blocks with?

[joshgoebel]

I am using highlight.js with Next.js 11 for a blog. It works perfectly fine for me

I'm very glad, but there are a few different things that could be going on here - and potentially 3 ways you could use HLJS with a complex client-side framework/SPA:

• It's loaded normally with a <script> tag on INITIAL page load (it should work, this is our happy path)
• It's loaded later dynamically (it won't work since the DOMContentLoaded has fired in the past and will never trigger our startup code, the patch I posted above is a suggested fix)
• It's loaded on the server-side and used to statically render the app on the server (in which case there are no browser events, unless they are simulated?)

Which of the 3 are you doing? We need to make sure there are clean paths for all 3.

[joshgoebel]

The goal here is to avoid two APIs [like we have now] (renamed for clarity):

• highlightAllButWaitUntilPageFinishesLoading()
• highlightAllAssumePageIsLoaded()

It seems we should be able to deal with tracking page state and abstract this concern away from the implementor. But it get complex when there is no browser (ie, server-side), or when the page might have actually been loaded perhaps minutes or hours ago...

I still think we can handle all these cases, but we may need some small tweaks.

[joshgoebel]

One other way would be to just give up this abstraction and update our docs to tell users to hook the event themselves (for native use, or use their framework hooks), ie for raw JS:

window.addEventListener('DOMContentLoaded', () => hljs.highlightAll(), false);

But I still think the abstraction is nice to have if we can find a way to make it "just work" in all cases.

[trm217]

So I have the page /posts/[slug], I import the monokai theme css in the page-component via a es6 import (importing a css-file is handled by next.js).
I then have a React Hook that runs .highlightAll inside a useEffect React hook. The page itself uses incremental static generation.

[joshgoebel]

The page itself uses incremental static generation.

When doing static generation is the 'DOMContentLoaded' event fired in the pretend DOM environment? Do you know?

[trm217]

Seeing as this issue is based on a Next.js site, it only makes sense to run highlightAll inside the page component lifecycyle method. Thus it should always be executed on the client. So I fail to see how SSG would make a difference.

[trm217]

The page itself uses incremental static generation.

When doing static generation is the 'DOMContentLoaded' event fired in the pretend DOM environment? Do you know?

I don't know. But as said the highlight method would be fired in the component lifecycle not based on a browser event.

[joshgoebel]

Thus it should always be executed on the client. So I fail to see how SSG would make a difference.

Why - does something in Next.js prevent it from being rendered statically so that the initial load of the page can be given to the browser as static HTML?

And also users often do things that make no sense. :-) [Not saying that that's the case here]

But as said the highlight method would be fired in the component lifecycle not based on a browser event.

We have initialization/startup code that depends on browser events. So knowing the status of the DOM is critical for us to know the correct behavior when highlightAll() is called. It needs to know the DOM is ready or not.

It's far too easy (in many environments) to call it too soon, in which case it would fail.

[trm217]

I will check tomorrow if the said event is fired.

In regards to it being executed on the server:
I suppose it could be run when generating the page, but this would not work without any additional configuration. Next.js, when using SSG, injects the static propeties & then renders the initial component without executing life cycle hooks or any other logic placed inside the component. There is no way that I'd know off to run highlightAll inside the component so that it would be executed on the server during buildtime (or at least not without bending some rules).

[joshgoebel]

I tested myself with a vanilla JS (but with our library lazy loaded) and indeed highlightAll() fails since the event we hook for startup has already fired. #3039 resolves this. AFAIK this resolves 2 of the 3 potential issues here.

Next.js, when using SSG, injects the static propeties & then renders the initial component without executing life cycle hooks or any other logic placed inside the component. [emphases mine]

Ah, then if useEffect is a lifecycle hook... then yes I see what you mean here. I imagine it would still be possible for someone to wire up something custom ("bending the rules") that would pull the highlighting into the "renders the initial component" phase... in which case I'm not sure highlightAll() would work at that point. Although at that point I'm not sure highlightAll() would be the correct API either...

What if someone wanted to render say a page of documentation with the highlighting all done on the server-side via SSG. How might they attempt such a feat? Is that type of thing frowned upon for some reason or unusual to see done? Is there a nicer way or doing that other than hooking say a render() hook and spitting out HTML/DOM/ShadowDOM elements directly? Because in that cause I imagine someone would be calling the non-DOM highlight API instead.

[trm217]

I fail to see a reason why this should not be executed in the client when working with Next.js.
Next.js always hydrates the page state after it has been sent to the client. Next.js's whole purpose is to speed up the page loadtime of a React page, which naturally requires JS.
Enhancing the generated page after the rehydration ok the site is also fine.
If it was desired to render a final page, that doesn't execute any logic clientside, a React based page would most likely be a bad choice anyways.

[joshgoebel]

If it was desired to render a final page, that doesn't execute any logic clientside, a React based page would most likely be a bad choice anyways.

I'll take your word for it... I was just assuming if someone COULD do it while we're solving this I was curious what that might look like and see if we could solve that as well. I wasn't trying to suggest "best practice" or any such thing for Next.js. If my guess is correct about highlightAll() not being the correct API in that case anyways - then the problem has solved itself.

I'm a big fan of server-side rendering (from the Ruby on Rails world). :) For me personally I'd prefer to ship simple HTML pages as "cooked" as possible to the client - for the fastest load times. You may be right though in that perhaps SSG with React/Next.js may not be the best tool for accomplishing such things.

[joshgoebel]

@jammykam From everything said here so far I'm confused that the patch above wouldn't resolve your issue. Can you try #3039 and if that doesn't help then a small isolated example case would still be appreciated. :)

[trm217]

I didn't mean to say that you implied anything :)

I see your point as well, but as you said Next just isn't really made for that. Perhaps it would be possible to write a component that automatically highlight it's children, but I really just can't see a need for it in a React page.

(Thx for the conversation, it's nice to have fast replies 😄)

[joshgoebel]

Thanks for trying to help! Knowledgeable people on other ecosystems is always good to have.

[jammykam]

@jammykam From everything said here so far I'm confused that the patch above wouldn't resolve your issue. Can you try #3039 and if that doesn't help then a small isolated example case would still be appreciated. :)

Sorry, i have been snowed under wth client work and will try and get something to you soon, I didn't mean to open an issue and then run away. Looking at that commit now it seems like it should have worked, and entirely possible i messed something up when trying it.

[joshgoebel]

seems like it should have worked, and entirely possible i messed something up when trying it.

That's what I'm wondering. I feel like @trm217 kind of rules out the SSG being the issue. So if it's not that or this then I'm left scratching my head a bit.

[trm217]

Actually, I managed to reproduce the issue in my project. I am unsure why it worked before because now that I have further looked into it, this should definitely not have worked. Perhaps I was using an older version. 🤔

Anyways, I checked the code of @jammykam as well as his point on the boot function. He actually is right, if highlight JS is loaded on a component basis, the DOMContentLoaded event wouldn't fire.

A simple workaround would be to enhance the highlightAll function, by also checking if document.readyState === "loading".
I am unsure about if readyState is supported by all browsers though

This is my proposed change to the highlightAll function

/**
   * auto-highlights all pre>code elements on the page
   */
  function highlightAll() {
    console.log
    // if we are called too early in the loading process
    // CHANGE  - don't execute the highlighting if either the domLoaded event hasn't been played or doc
    if (!domLoaded && document.readyState === "loading") { wantsHighlight = true; return; }

    const blocks = document.querySelectorAll('pre code');
    blocks.forEach(highlightBlock);
  }

However, I am not certain if this will break the current expected behavior in anyway. I suppose not, since it wouldn't make a lot of sense for domLoaded to be true while document.readyState would still be loading.

Here a dumbed-down version of my PostPage to showcase how I use HighlightJS

import { FC, useEffect } from "react";
import ReactMarkdown from "react-markdown";
import HighlightJS from "highlight.js";
import "highlight.js/styles/monokai.css";
import Layout from "../../components/templates/Layout";
import { Post } from "../../utility/BlogPostUtility";
import SEO from "../../components/utilities/SEO";

interface PostPageProps {
  post: Post;
}

const PostPage: FC<PostPageProps> = ({
  post: { content, description, title },
}) => {
  useEffect(() => {
    HighlightJS.highlightAll();
  });

  return (
    <Layout>
      <SEO title={title} description={description} />
      <article className="prose lg:prose-xl">
        <h1>{title}</h1>
        <div>
          <ReactMarkdown escapeHtml source={content} />
        </div>
      </article>
    </Layout>
  );
};

[joshgoebel]

He actually is right, if highlight JS is loaded on a component basis, the DOMContentLoaded event wouldn't fire.

Doesn't #3039 already resolve this? I see only two cases, both now handled.

• DOM isn't loaded yet, we hook the event - because it will eventually fire.
• DOM is already loaded, we can confirm with readyState (making a hook unnecessary)

Your solution may indeed be simpler though, I'll give that some thought. :) But can you confirm 3039 covers both cases already?

I am unsure about if readyState is supported by all browsers though

Every browser we care about - all modern browsers.

[trm217]

I don't think it would. The problem is it's not guaranteed that the DOMContentLoaded event is fired when HighLight.js is being loaded. So defacto domLoaded would always be false.

Taking the code from #3039, I would further adapt it to this, to make sure it works as expected even if the DOMContentLoaded event has been fired before Highlight.js was loaded.

I would adapt the code to remove the domLoaded variable altogether and only check the document readystate in the highlight all function.

/**
 * auto-highlights all pre>code elements on the page
 */
function highlightAll() {
  // if we are called too early in the loading process
  if (document.readyState === "loading") { wantsHighlight = true; return; }
  const blocks = document.querySelectorAll('pre code');
  blocks.forEach(highlightElement);
}
function boot() {
  // if a highlight was requested before DOM was loaded, do now
  if (wantsHighlight) highlightAll();
}

// make sure we are in the browser environment
if (typeof window !== 'undefined' && window.addEventListener) {
  window.addEventListener('DOMContentLoaded', boot, false);
}

I think this would work as expected and also simplify the changed code from #3039.

Would it be possible for me to propose my changes on the PR?

[joshgoebel]

@jammykam Just bumped a newer #3039 when you have a chance to test again.

[sleeptil3]

Did you try swapping out the useEffect for the useLayoutEffect hook? It causes the side effect to happen synchronously after DOM manipulation has finished. This is has fixed some issues for me in this situation. Its analygous to firing in the class-component componentDidMount and componentDidUpdate life cycles.

https://reactjs.org/docs/hooks-reference.html#uselayouteffect