use plugin rehype-highlight v7.0.0 memory leak

[zzzgydi]

Initial checklist

• I read the support docs
• I read the contributing guide
• I agree to follow the code of conduct
• I searched issues and couldn’t find anything (or linked relevant results below)

Affected packages and versions

rehype-highlight v7.0.0

Link to runnable example

https://github.com/zzzgydi/rhv7-memory-leak-test

Steps to reproduce

• pnpm i
• pnpm dev
• Visit http://localhost:5173
• Open Chrome's task manager and find the corresponding tab to observe the memory situation
• Click start button
• observe the memory situation
• Click stop and clear button and observe

As a comparison

• Change rehype-highlight v7.0.0 to rehype-highlight v6.0.0 in package.json
• Repeat the steps above

Expected behavior

same as rehype-highlight v6.0.0

Actual behavior

Runtime

No response

Package manager

pnpm

OS

macOS

Build and bundle tools

Vite

[immccn123]

can reproduce in https://remarkjs.github.io/react-markdown/

just type freely in the editor and a large increase of memory will be found

[ChristianMurphy]

Thanks @zzzgydi and @immccn123!
Memory should not be ballooning in that way.
Would either of you be interested in tracing this further and potentially opening a PR? https://developer.chrome.com/docs/devtools/memory-problems

[wooorm]

I’m pretty sure V8 and friends actually use up as much memory as they can before freeing it? Maybe you’re seeing memory being used?

[zzzgydi]

I guess that lowlight has registered languages internally, but did not unregisterLanguage. After creating multiple instances like this may cause memory leaks.

Below I have written three comparative examples. In my understanding, if there is no memory leak, the memory should be released after 100 iterations of the for loop.

import { createLowlight, common } from "lowlight"; // version: ^3.1.0
import HighlightJs from "highlight.js"; // version: ^11.9.0
import "./App.css";

function customCreateLowlight(grammars: typeof common) {
  const high = HighlightJs.newInstance();
  if (grammars) {
    Object.keys(grammars).forEach((name) =>
      high.registerLanguage(name, grammars[name])
    );
  }
  return high;
}

function App() {
  return (
    <div style={{ display: "flex", flexDirection: "column", gap: 12 }}>
      <button
        type="button"
        onClick={() => {
          for (let i = 0; i < 100; i++) {
            const lowlight = createLowlight(common);
          }
        }}
      >
        createLowlight
      </button>

      <button
        type="button"
        onClick={() => {
          for (let i = 0; i < 100; i++) {
            const lowlight = customCreateLowlight(common);
          }
        }}
      >
        customCreateLowlight
      </button>

      <div>
        <button
          type="button"
          onClick={() => {
            for (let i = 0; i < 100; i++) {
              const lowlight = customCreateLowlight(common);
              Object.keys(common).forEach((name) =>
                lowlight.unregisterLanguage(name)
              );
            }
          }}
        >
          customCreateLowlight & unregisterLanguage
        </button>
      </div>
    </div>
  );
}
export default App;

I obtained the following results. The results are divided into three rounds. First, I refresh the page, recorded, and then click on the button to run once, and recorded next.

Through the above graph, it is a comparison of the results between snapshot 6 and snapshot 5. The largest occupation is string type, and these values are related to language settings within highlightjs.

[wooorm]

After creating multiple instances like this may cause memory leaks.

If there is indeed a memory leak instead of chrome using up the memory it can use up, then that seems like a highlightjs issue?

It’s JavaScript; you’re not supposed to need to manage your own memory by removing registered things. The language does garbage collection for you.

[TonyL1u]

The solution is you can contribute to open source and figure out a solution instead of spamming duplicate info.

good advice

[alielmorsy]

Did anyone find a solution for that?
I am using that code and yes there is a weird memory leak because it renders code blocks over and over:

   <Markdown rehypePlugins={[rehypeRaw, rehypeHighlight]} components={{
                code({node, className, children, ...props}) {
                    return <div className={className}>
                        {children}
                    </div>
                },
            }}>
                {post.content}
            </Markdown>

[remcohaszing]

If possible, you should provide components with a stable identity to avoid rerenders.

function Code({ node, className, children, ...props }) {
  return (
    <div className={className}>
      {children}
    </div>
  )
}

export function Post({ post }) {
  return (
    <Markdown
      rehypePlugins={[rehypeRaw, rehypeHighlight]}
      components={{code: Code}}
    >
      {post.content}
    </Markdown>
  )
}

Also, if possible, you should provide props with a stable identity to reduce rerenders even further.

function Code({node, className, children, ...props}) {
  return (
    <div className={className}>
      {children}
    </div>
  )
}

const components = {
  code: Code
}

const rehypePlugins = [
  rehypeRaw,
  rehypeHighlight
]

export function Post({ post }) {
  return (
    <Markdown
      rehypePlugins={rehypePlugins}
      components={components}
    >
      {post.content}
    </Markdown>
  )
}

[aeharding]

This memory leak appears to happen by simply including rehype-remark, even without any actual highlighted content.

If possible, you should provide components with a stable identity to avoid rerenders.

This does not noticeably address the leak in my testing

As others suggest, my current workaround is to use v6.

[tgram-3D]

I'm not sure if anyone else has run into this, but one thing of note is if you downgrade to rehype-highlight 6.0.0 you get a TS error similar to this:

hashicorp/next-mdx-remote#86

The code works fine with no memory leak, and you can add @ts-expect-error to get rid of it as per the above thread, but I'm curious if there's a better solution.

[immccn123]

This issue seems to be caused by the DOMContentLoaded event listener on the window not being removed in a timely manner. You can type anything on https://remarkjs.github.io/react-markdown/ and use getEventListeners(window).DOMContentLoaded in a Chromium-based browser to compare the length before and after.

rep_o9Lvk9Iu5T.mp4

window.addEventListener("DOMContentLoaded",...); appears only once in the minimized code.

Results before and after executing the following code：

getEventListeners(window).DOMContentLoaded.forEach(x => window.removeEventListener("DOMContentLoaded", x.listener))

Snapshot 1: Before typing
Snapshot 2: After typing
Snapshot 3: After executing the code

[wooorm]

Thanks for investigating this and looking for a proper fix, Imken!

[wooorm]

Thanks again, closed in highlightjs/highlight.js#4095!