Memory leaks when using `newInstance` to create multiple hljs instances

[immccn123]

Describe the issue/behavior that seems buggy

Calling newInstance multiple times poses a risk of memory leaks.

Sample Code or Instructions to Reproduce

see https://github.com/remarkjs/react-markdown/issues/791#issuecomment-1878795432

Expected behavior

newInstance should not call window.addEventListener('DOMContentLoaded', boot, false).

Additional context

In function HLJS:

https://github.com/highlightjs/highlight.js/blob/e1fa2eacf673ef6f8aab2f5c287fa9198edf4fa5/src/highlight.js#L838-L840

Related:

remarkjs/react-markdown#791 rehypejs/rehype-highlight#31

[joshgoebel]

I can see the issue here, though newInstance is not really intended to be used more than a few times (for custom usage scenarios) - there is a high startup cost if you are using lots grammars. React should not be calling it for say, every render loop. Instead some parent component should likely hold onto a single instance and use that for most rendering.

[joshgoebel]

How would anyone suggest we fix this, track some global state on window itself, or?

[immccn123]

I don't think a standalone hljs instance should have global side effects. Perhaps an additional configuration option would be better, but I'm not sure if this would break highlight.js and it might increase the bundle size.

It seems like that the operations on the window object are only these.

[joshgoebel]

What if this code was moved inside the conditional portion of highlightAll() such that it's only called if one requests highlighting of everything?

[immccn123]

What if this code was moved inside the conditional portion of highlightAll() such that it's only called if one requests highlighting of everything?

Seems like a good idea

[erha19]

@immccn123 is there any new version for this fix?

[callie-openai]

@joshgoebel @immccn123 Also curious about this, saw the updated CHANGES.md for 11.11.0 but no associated publish to npm yet.