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

It tries to load all *.tmLanguage.json unexpectedly #221

Closed
JounQin opened this issue Sep 3, 2021 · 7 comments · Fixed by #384
Closed

It tries to load all *.tmLanguage.json unexpectedly #221

JounQin opened this issue Sep 3, 2021 · 7 comments · Fixed by #384
Labels
bug Something isn't working

Comments

@JounQin
Copy link

JounQin commented Sep 3, 2021

Jietu20210904-010141-HD.mp4

My source code:

import { Skeleton } from 'antd'
import type { FC } from 'react'
import { setCDN, getHighlighter } from 'shiki'

import './index.less'

import { Rehype } from 'components'
import { usePromise } from 'hooks'

export interface ShikiProps {
  theme?: string
  lang?: string
  children?: string
}

setCDN('/shiki/')

export const Shiki: FC<ShikiProps> = ({
  children,
  theme = 'dracula',
  lang,
}) => {
  const [highlighter] = usePromise(() => getHighlighter({ theme }))
  return highlighter ? (
    <Rehype>{children && highlighter.codeToHtml(children, lang)}</Rehype>
  ) : (
    <Skeleton className="shiki" />
  )
}
@JounQin
Copy link
Author

JounQin commented Sep 3, 2021

Additionally, if there are several <Shiki> used at a same time, a lot of duplicate requests will be sent and could fail...

image

image

@JounQin
Copy link
Author

JounQin commented Sep 3, 2021

The error seems happening when lang is mismatch, (the source code and lang info comes from user, so it can be wrong)

image

@octref octref added the bug Something isn't working label Sep 18, 2021
@octref
Copy link
Collaborator

octref commented Sep 18, 2021

Probably something I've fixed in fd178bf. I'll release a new version tomorrow, please give it a try.

@Gerrit0
Copy link
Contributor

Gerrit0 commented Sep 19, 2021

Additionally, if there are several <Shiki> used at a same time, a lot of duplicate requests will be sent and could fail...

This is a problem with your implementation, not Shiki. You are creating one highlighter per element, and Shiki stores the loaded languages/themes on a highlighter instance rather than in a global cache, which is rather unintuitive, but not necessarily wrong...

setCDN('/shiki/')
const hl = getHighlighter({})

export const Shiki: FC<ShikiProps> = ({
  children,
  theme = 'dracula',
  lang,
}) => {
  const [highlighter] = usePromise(async () => {
    const highlighter = await hl
    await highlighter.loadTheme(theme)
    return highlighter
  })
  return highlighter ? (
    <Rehype>{children && highlighter.codeToHtml(children, lang)}</Rehype>
  ) : (
    <Skeleton className="shiki" />
  )
}

Loading grammars up front is also necessary for codeToHtml to be synchronous. If you know the language ahead of time, you can pass it to getHighlighter in langs. If you don't, you could ask Shiki to just load one language (it will load everything if there's nothing in langs), and then add a call to highlighter.loadLanguage(lang) within usePromise

@JounQin
Copy link
Author

JounQin commented Sep 19, 2021

@Gerrit0 Thank you very much, I didn't seen any document about that highlighter should be singleton or I should use langs in getHighlighter first. I'll give it a try after the new release.

But loading all languages by default is still unexpectedly to me. If it is expected in shiki, it should be documented.

@octref
Copy link
Collaborator

octref commented Oct 25, 2021

I agree the documentation could be better. Let me start a proper doc website...

@KevinBatdorf
Copy link
Contributor

To stop them all from loading it seems you need to pass in the list of languages you need up front.

I've also noticed it loads the resources twice (and only twice) regardless.

Three possible opportunities:

  1. I believe this code is where it's deciding to load everything. Instead I think it could check the options after filtering, then use an empty array if nothing passed in, then rendering to plain text.
  2. Loading twice should probably not even be possible. So an improvement would be to cache any theme/languages requests
  3. Possibly update the parameters to include a lang property similar to theme (types)

Here's a basic hook for swr I'm using which seems to work (I'll update here if I run into issues later):

import { getHighlighter, Lang, setCDN, Theme } from 'shiki'
import useSWRImmutable from 'swr/immutable'
type Params = { theme: Theme; lang: Lang }

setCDN('https://unpkg.com/shiki/')
const fetcher = ({ theme, lang }: Params) =>
  getHighlighter({ langs: [lang], theme })

export const useTheme = ({ theme, lang }: Params) => {
  const { data: highlighter, error } = useSWRImmutable(
    { theme, lang },
    fetcher,
  )
  return { highlighter, error, loading: !highlighter && !error }
}

btw, love shiki so far. If you'd like me to PR any of the above i'd be happy to contribute.

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 a pull request may close this issue.

4 participants