search

withastro / astro

The web framework for content-driven websites. ā­ļø Star to support our work!
https://astro.build
Other
46.61k stars 2.47k forks source link

šŸ’” RFC: Replace Prism with Shiki #1212

Closed FredKSchott closed 2 years ago

FredKSchott commented 3 years ago

Background & Motivation

There's been significant interest in our Discord to replace Prism with Shiki:

There are several objective reasons to want Shiki over Prism, besides just hype:

There's some neat things going on behind the scenes, as well: Shiki controls theming, so that the end result is <span style="color: #XXXXXX"> instead of <span class="token token-comment">. This is objectively better for a tool like Astro because it accomplishes the exact same thing but without global classes that leak into our users projects.

Proposed Solution

Phase 1: Today

Phase 2: Soon

Phase 3: v1.0

Detailed Design

---
// Example Implementation
// Usage: import {Code} from 'astro/components';
import shiki from 'shiki';
const { code, lang = 'plaintext', theme = 'nord' } = Astro.props;
const highlighter = await shiki.getHighlighter({theme});
const html = highlighter.codeToHtml(code, lang);
---
{html}

Draft PR

https://github.com/snowpackjs/astro/pull/1208

okikio commented 3 years ago

I don't know if removing Prism entirely for V1.0.0 is a good idea

matthewp commented 3 years ago

Some initial thoughts/questions I have:

FredKSchott commented 3 years ago

Great questions! Answering inline:

  • Do you have to set the theme every time you use <Code>? It's kind of cool that you can use multiple themes in a single project this way, but I would think that most of the time you use the same theme throughout.
    • You could maybe have a wrapper component that sets the theme once.
    • Unless there's a way to set the theme globally this seems rather like a tradeoff (creating a wrapper component vs adding a CSS import) rather than clear win for Shiki.

I think the wrapper component is a workable solution for today. Before v1.0 I would like to consider the idea of a general context API (where a component could read from a user's config to control default behaviors) so that the user could set their default theme via project config.

The slower migration strategy of the proposalĀ is meant to address this, where both Prism and Code would be available to you until we had that "setting a global theme" feature in <Code>. If this is a concern, I'm happy to add this as a requirement before we can move on to Phase 2 ("Soon").

  • How does one customize highlighting? Do you need to create your own theme? It's nice with class-based highlighters that you can tweak things with normal CSS.

Shiki supports loading any VSCode theme, via JSON: https://github.com/shikijs/shiki/blob/main/docs/themes.md#loading-theme. This is a good call to add support for this in #1212.

  • How would you set the theme when using code blocks in Markdown?

Either use <Code> directly, or set the global theme using the missing feature described above. Note that in phase 1, Markdown continues to use <Prism> for this reason.

  • Next.js and Eleventy both provide a way to customize which syntax highlighter is used in Markdown. It would seem pretty easy for us to as well. Essentially we need need to know the name of the import specifier to use so that we write out import Code from 'wherever';. So anything that took the props and returned HTML could implement their own syntax highlighting. This would create an easy transition for Prism users. It would be nice if this RFC added this ability.

Can we treat this as a separate RFC? I don't have any strong opinions about this so I don't think I'm the person to design it. I also don't think it's blocking or directly tied to this proposal. If this is a concern, I'm happy to add this as a requirement before we can move on to Phase 2 ("Soon").

Update: Added proposed requirements to "phase 2."

matthewp commented 3 years ago

In the call someone brought up a good question: How do you do dark mode with shiki given that it does style="color: blue". This would be a good thing to research as part of this RFC.

FredKSchott commented 3 years ago

Yes! Here's the discussion on the shiki repo: https://github.com/shikijs/shiki/issues/33

It seems like best recommendation (used on Typescript's website) is to render two: one for dark mode & one for light mode. Then, you control when to show each in your own CSS, using something like this: https://github.com/shikijs/shiki/issues/33#issuecomment-842368315

I don't love this tbh, but I understand why they would reach for this. It does make sense given their constraints.

In our case, instead of using Prism's two-theme-css-files solution (which, to be fair, I also don't love :) we would have something like <Code theme={['name-light", "name-dark"]}, where multiple themes = multiple code blocks rendered. And then, it would be on you to provide the CSS to turn on/off based on how your site controls dark/light mode.

Update: See below, I think we've found a better solution that uses CSS variables.

matthewp commented 3 years ago

Can we treat this as a separate RFC? I don't have any strong opinions about this so I don't think I'm the person to design it. I also don't think it's blocking or directly tied to this proposal. If this is a concern, I'm happy to add this as a requirement before we can move on to Phase 2 ("Soon").

I think we should have this before switching markdown backticks over to Code, otherwise Prism users wouldn't have a way to preserve their styles. Also I think in the future it's likely that new popular highlighters will come around; the choice here is personal preference that should be customizable as it is in Next.js and 11ty.

jasikpark commented 3 years ago

It sounds like the Shiki might have more tokens and therefore more CSS selectors, but I've found generating CSS from a theme easiest to do... if you could do npm install name-light name-dark and then use those themes as imported CSS, that would be amazing... all that would be needed is a vscode theme to CSS transpiler.

https://github.com/jasikpark/jasik-xyz/blob/f1d15f587b72df4197d6953db384f600bbe85c2a/assets/css/syntax.css

matthewp commented 3 years ago

Move our recommendation in docs to use <Code> over <Prism>, but keep references to <Prism>.

I think this should be in Phase 2, because otherwise if you follow the docs you'll see different styles on pages created with .astro files and those created on .md files. I think we'd get bug reports asking why styles looked different.

FredKSchott commented 3 years ago

Ah good call, making this change now

FredKSchott commented 3 years ago

Resolved the dark mode issue by creating a theme that uses CSS variables instead of hex colors! We can ship this as a theme within Astro, but I'd like to get it built into Shiki as an official theme if they're interested. More info here: https://github.com/shikijs/shiki/issues/33#issuecomment-905953016

<!-- normal developer, loves built-in themes -->
<Code code="const helloWorld = 42;"  theme="nord" />

<!-- crazed developer, needs dark mode support, defines their own css variables in CSS -->
<Code code="const helloWorld = 42;"  theme="custom" />
/** available variables when using theme="custom" */
--code-foreground: #XXXXXX;
--code-background: #XXXXXX;
--code-token-default: #XXXXXX;
--code-token-constant: #XXXXXX;
--code-token-string: #XXXXXX;
--code-token-comment: #XXXXXX;
--code-token-keyword: #XXXXXX;
--code-token-parameter: #XXXXXX;
--code-token-function: #XXXXXX;
--code-token-string-expression: #XXXXXX;
--code-token-info: #XXXXXX;
--code-token-warn: #XXXXXX;
--code-token-warn: #XXXXXX;
--code-token-debug: #XXXXXX;
--code-token-strong: #XXXXXX;
--code-token-punctuation: #XXXXXX;
--code-token-link: #XXXXXX;
jasikpark commented 3 years ago

That looks amazing! That'll also help for when you want to have like 5 user-selectable themes on your website, which would require either 5 Shiki divs or 5 css theme files.

jasikpark commented 3 years ago

https://github.com/shikijs/shiki/pull/212 -- css theme merged into shiki

jasikpark commented 3 years ago

I think it'd be great if someone in the community eventually forked the Prism component, once it's dropped from core, since I believe there might still be use-cases that are still met by Prism.

FredKSchott commented 3 years ago
jasikpark commented 3 years ago

https://github.com/shikijs/shiki/blob/main/docs/themes.md docs for the dark mode support!

eyelidlessness commented 3 years ago

Feel free to reference my (messy incomplete but working) Shiki solution for my personal site. I think baking this into Astro is going to be a pain and a half with tons of followup issues, and itā€™s best to allow it as a configuration option rather than have an opinionated syntax highlighting solution.

Shiki is freaking awesome (and Shiki Twoslash even moreso), but several use cases are complicated. Static rendering especially so. Users might be tempted to pair two themes that have different token references, which will break in a bunch of unexpected ways no matter how you variable.

FredKSchott commented 3 years ago

@eyelidlessness appreciate the feedback, if you can give specifics that would be really helpful.

This RFC was just approved today with consensus, but with specific checkins over the next few months to make sure no issues arise as users migrate over. If any issues come up for others we'll obviously reevaluate.

eyelidlessness commented 3 years ago

@FredKSchott The awesome thing about Shiki is also its most challenging aspect. Because it uses VSCode themes, which are TextMate themes (also used by Sublime and many other editors), itā€™s incredibly flexible. The trade off of that flexibility is things can conflict if you try to combine them.

TM/VSC themes style based on scoped token specifiers, which have similar specificity overriding logic to CSS. Combining a dark theme with a light theme with different token specifiers (a) produces different markup and (b) even if you unify the markup isnā€™t likely to produce predictable results.

In my usage I picked a dark/light theme combo (Yi) with the same tokens, and swapped out with class names (Fela generated) rather than CSS variables, but both would have the same specificity conflicts it the themes selected different tokens.

My messy incomplete implementation is here, and a bit behind Shiki/Twoslash. You can see where I marry the two themesā€™ respective classes. Itā€™s all pretty hacky but itā€™s a product of lots of fussing with inconsistent styles between themes before I decided it wasnā€™t solvable that way without building my own specificity engine or piggybacking on more of VSCode internals than I felt comfortable with.

tusharsadhwani commented 3 years ago

Couldn't someone do something like:

---
const highlighter = await shiki.getHighlighter({theme: 'GitHub Light'});
const highlighterDark = await shiki.getHighlighter({theme: 'One Dark'});

const html = highlighter.codeToHtml(code, lang);
const htmlDark = highlighterDark.codeToHtml(code, lang);
---
{darkMode ? htmlDark : html}

Not exactly this, but essentially having two different versions of the same code snippet pre-rendered, swapping it out when changing themes?

okikio commented 3 years ago

@tusharsadhwani that requires too much code duplication, however, a possible solution could be to use something like solidjs together with shikijs on the frontend, as well as the backend, so, e.g. astro prebuilds shiki in light mode but if darkmode is activated shikijs and solidjs on the frontend are loaded and rerender the code snippet for dark mode

jasikpark commented 3 years ago

Couldn't someone do something like:

---
const highlighter = await shiki.getHighlighter({theme: 'GitHub Light'});
const highlighterDark = await shiki.getHighlighter({theme: 'One Dark'});

const html = highlighter.codeToHtml(code, lang);
const htmlDark = highlighterDark.codeToHtml(code, lang);
---
{darkMode ? htmlDark : html}

Not exactly this, but essentially having two different versions of the same code snippet pre-rendered, swapping it out when changing themes?

that is one of the options people have used, except you'd more likely toggle the visibility of a code block via css

Tc-001 commented 3 years ago

Don't know if alreadt discussed, but there is the good old filter: invert() hue-rotate(180deg);, as it can actually be quite good, especially with the fact that it's almost zero config

image (github code block from above)

FredKSchott commented 3 years ago

Yup, both of these are also valid solutions! You can see the different suggestions being made over the last year in this thread: https://github.com/shikijs/shiki/issues/33

jonathantneal commented 2 years ago

Hey everyone! This RFC is still accepted, and it has been moved to https://github.com/withastro/rfcs/blob/main/active-rfcs/replace-prism-with-shiki.md

In an effort to improve our RFC process, we made some changes to better organize things, which include a dedicated RFC repository.