-
Notifications
You must be signed in to change notification settings - Fork 299
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
Revert: rehype-highlight bump #5313
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
@sqs By popular vote users prefer to avoid OOM causing grey-screens over saving 300 kb on bundle size 🧌 😅 All joking aside, if this was the cause again I'll invest some time in figuring out how we can do some better benchmarking / profiling as part of the test suite. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the catch!
There has been a (long ongoing) issue with `rehype-highlight` / `highight-js` causing OOM issues on `>v6.0.0`. This reverts a recent version bump that might have [re-introduced this issue](#4459) in Cody chat causing grey-screens and crashes. > Important: I wasn't able to re-produce the issue myself for some reason. So this is a bit of a "shot-in-the-dark" fix. Hopefully we can have those who reported the issue validate if it actually worked. Lowering the version shouldn't hurt any other changes though as it was merely done to reduce the bundle size somewhat. ## Test plan CI
- As requested in review as it seems there were performance issues with rehype-highlight 7.0.0 as seen in pull request sourcegraph#5313
There has been a (long ongoing) issue with
rehype-highlight
/highight-js
causing OOM issues on>v6.0.0
. This reverts a recent version bump that might have re-introduced this issue in Cody chat causing grey-screens and crashes.Test plan
CI