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

feat: migrate to shikiji #3237

Merged
merged 13 commits into from
Nov 23, 2023
Merged

feat: migrate to shikiji #3237

merged 13 commits into from
Nov 23, 2023

Conversation

antfu
Copy link
Member

@antfu antfu commented Nov 22, 2023

Benefits:

  • Smaller client payload size, as the code is no longer duplicated in HTML (https://github.com/antfu/shikiji#lightdark-dual-themes)
  • Shikiji is hast based, which makes it much easier to transform and customize. A lot of things can be done more robustly without RegExp. shikiji-transformers is created to mimic the behavior of shiki-processor. It also allows users to provide their own transformers.
  • More efficient langs/themes loading based on ESM rather than fs
  • Install size: 17MB -> 9MB

This should be quite compatible with the previous behavior. The only change users might notice is that .vp-code-dark and .vp-code-light are no longer used, instead, a single .vp-code takes over.

In general, I think this PR is good to do, marking it as draft as I need to handle some details on shikiji side and release a stable version before this gets merged. Should be ready :)

@antfu antfu marked this pull request as draft November 22, 2023 22:24
@antfu antfu requested a review from brc-dd November 22, 2023 22:24
@antfu antfu marked this pull request as ready for review November 22, 2023 23:48
docs/.vitepress/config.ts Outdated Show resolved Hide resolved
@brc-dd

This comment was marked as resolved.

@brc-dd

This comment was marked as resolved.

@brc-dd
Copy link
Member

brc-dd commented Nov 23, 2023

Rest looks good to me. I'm fine with introducing some breaking changes this once, as long as we can mention the migration path here, so that we can just redirect users to visit this PR for solutions.

Co-authored-by: Divyansh Singh <40380293+brc-dd@users.noreply.github.com>
antfu added a commit to antfu/shikiji that referenced this pull request Nov 23, 2023
@antfu
Copy link
Member Author

antfu commented Nov 23, 2023

Another thing, is the syntax for adding custom languages still something like?

Yes, it support custom langs. But path option is not, as shikiji does not have fs access. Users need to load by themselves and pass the json. I will update the docs in shikiji and mention about that.

Edit: Updated https://github.com/antfu/shikiji/blob/main/docs/languages.md#migrate-from-shiki

@antfu
Copy link
Member Author

antfu commented Nov 23, 2023

Should be good to go now

@brc-dd
Copy link
Member

brc-dd commented Nov 23, 2023

Just curious,

const vineGrammar = JSON.parse(fs.readFileSync(join(__dirname, './vine-ts.tmLanguage.json'), 'utf8'))

how is this different from import vineGrammar from './vine-ts.tmLanguage.json' 👀

@antfu
Copy link
Member Author

antfu commented Nov 23, 2023

No, both would work. Tho in real ESM you can't directly import .json

@brc-dd
Copy link
Member

brc-dd commented Nov 23, 2023

Yeah I meant with assert { type: 'json' } (or the with syntax once node supports that 😅)

@brc-dd brc-dd merged commit 75f18e4 into main Nov 23, 2023
7 checks passed
@brc-dd brc-dd deleted the feat/shikiji branch November 23, 2023 12:54
@brc-dd
Copy link
Member

brc-dd commented Nov 23, 2023

Also, in https://github.com/antfu/shikiji/blob/main/docs/languages.md#migrate-from-shiki

I don't think id exists on that type. Needs to be removed from the second block?

For vue-vine guys, they need to change name to vue-vine in their language file. Looks like shikiji uses name as the id too? Other stuff seems to be working fine.

@brc-dd brc-dd mentioned this pull request Nov 23, 2023
4 tasks
@brc-dd
Copy link
Member

brc-dd commented Nov 23, 2023

released in v1.0.0-rc.30 🎉

@husayt
Copy link

husayt commented Nov 23, 2023

The way we used to import lang definitions from shiki was through tmLanguage.json files in languages folder in shiki dist.

 import htmlLang from "shiki/languages/html.tmLanguage.json" assert { type: "json" };

I noticed there are no *.tmLanguage.json with shikiji.

What shall we use here?

@brc-dd
Copy link
Member

brc-dd commented Nov 23, 2023

@husayt Hi, what's your use case for that?

@husayt
Copy link

husayt commented Nov 23, 2023

it is for me to add more aliases

export const markdown: MarkdownOptions = {
languages: [
    {
      grammar: htmlLang,
      id: "myHtml",
      scopeName: "text.html.basic",
      aliases: ["html:preview", "html:preview:expanded:no-codepen"],
    } as any,
  ],
};

@brc-dd
Copy link
Member

brc-dd commented Nov 23, 2023

Can you try if this works:

export const markdown: MarkdownOptions = {
  languageAlias: {
    'myHtml': 'html',
    'html:preview': 'html',
    'html:preview:expanded:no-codepen': 'html',
  }
} 

@elringus
Copy link

elringus commented Nov 24, 2023

@brc-dd @antfu Can you please clarify how to migrate the following (proposed migration guides don't cover types and themes):

import { defineConfig, MarkdownOptions } from "vitepress";
import { ILanguageRegistration } from "shiki";

export default defineConfig({
    ...
    markdown: md
});

const md: MarkdownOptions = {
    languages: [mylang],
    theme: {
        light: "./mylang-theme-light.json",
        dark: "./mylang-theme-dark.json"
    }
};

const mylang: ILanguageRegistration = {
    id: "mylang",
    aliases: ["mylang-alias"],
    scopeName: "source.mylang",
    path: "./mylang-textmate.json"
};

@antfu
Copy link
Member Author

antfu commented Nov 24, 2023

import { defineConfig } from "vitepress";
import { MarkdownOptions } from "vitepress";
import { LanguageRegistration } from "shikiji";

import grammar from "./mylang-textmate.json"
import themeDark from "./mylang-theme-dark.json"
import themeLight from "./mylang-theme-light.json"

export default defineConfig({
    ...
    markdown: md
});

const md: MarkdownOptions = {
    languages: [mylang],
    theme: {
        light: themeDark,
        dark: themeLight,
    }
};

const mylang: LanguageRegistration = {
    id: "mylang",
    aliases: ["mylang-alias"],
    scopeName: "source.mylang",
    ...grammar,
};

@elringus
Copy link

Thank you, that seem to be working fine. Had type errors for imported grammar and theme jsons, but I didn't type-check them previously, so it's probably an issue on my side. At runtime everything seem to work same as before.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants