Skip to content
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

Merged
merged 2 commits into from
Aug 23, 2024
Merged

Conversation

RXminuS
Copy link
Contributor

@RXminuS RXminuS commented Aug 23, 2024

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.

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

Copy link
Contributor Author

RXminuS commented Aug 23, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @RXminuS and the rest of your teammates on Graphite Graphite

@RXminuS RXminuS added the bug Something isn't working label Aug 23, 2024 — with Graphite App
@RXminuS RXminuS marked this pull request as ready for review August 23, 2024 15:47
@RXminuS
Copy link
Contributor Author

RXminuS commented Aug 23, 2024

@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.

@RXminuS RXminuS requested review from umpox and sqs August 23, 2024 15:49
Copy link
Member

@sqs sqs left a 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!

@umpox umpox merged commit 0a5b683 into main Aug 23, 2024
20 checks passed
@umpox umpox deleted the rnauta/revert/rehype-highlight-bump branch August 23, 2024 16:26
umpox pushed a commit that referenced this pull request Aug 23, 2024
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
ichim-david added a commit to ichim-david/cody that referenced this pull request Oct 12, 2024
- As requested in review as it seems there were
  performance issues with rehype-highlight 7.0.0 as
  seen in pull request sourcegraph#5313
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants