stackernews / stacker.news

Internet communities that pay you Bitcoin
https://stacker.news
MIT License
440 stars 113 forks source link

Syntax highlighting #1242

Open ekzyis opened 5 months ago

ekzyis commented 5 months ago

Is your feature request related to a problem? Please describe.

Code blocks don't use syntax highlighting. This makes code hard to read.

Describe the solution you'd like

If you use ```[language] instead of simply ``` to start a code block, it should highlight the syntax as the selected language like Github does. Here with python as an example:

def foo():
  bar = 1
  return bar

Describe alternatives you've considered

n/a

Additional context

We already use react-syntax-highlighter but we only use language='text':

https://github.com/stackernews/stacker.news/blob/78520b787b928756c81700ab5b3ac16170ac9084/components/text.js#L134-L136

However, it requires more than passing the language since if I manually set it to python, it does not do syntax highlighting.

huumn commented 5 months ago

We used to do this. It made the bundle size HUGE for not much gain. It can probably be done efficiently with selective, dynamic imports.

A solution should use https://nextjs.org/docs/app/building-your-application/optimizing/bundle-analyzer to make sure it doesn't bloat the bundle.

tsmith123 commented 4 months ago

How about just importing the files for a few popular languages to tackle this one? If so, which languages would you want to support?

huumn commented 4 months ago

If they’re dynamically imported, it shouldn’t matter how many we support. But maybe we can find the top 5 somewhere

tsmith123 commented 4 months ago

I'm not sure how you'd decide when to "dynamically import" a language file. Selectively importing languages on build would keep the size down but you'd be limited those that you choose. If you mean importing them on the fly then that's a whole different ball game.

huumn commented 4 months ago

If you mean importing them on the fly then that's a whole different ball game.

That's the ball game I mean.

Unless we use dynamic imports, this isn't worth the bundle size bloat. Maybe we can dynamically import all languages. It's been a long time since I've looked at the code, but I think each language can be imported only when they're used.

tsmith123 commented 4 months ago

Well there is such a thing as dynamic imports but that isn't what you refer to as dynamic imports. You can choose which languages you want ahead of time but not during runtime.

huumn commented 4 months ago

https://mieszkogulinski.github.io/next-js-conditional-dynamic-import/

tsmith123 commented 4 months ago

That's cool but once you've imported how do you decide when to drop it or does it just "exist" for the duration of the page view?

huumn commented 4 months ago

You don't decide to drop it. The browser caches it or doesn't. The main thing is that it's only downloaded if it's used.