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

1.18.0 seems causing bundle errors on Next.js >= 14.2.x #784

Closed
3 of 5 tasks
arvinxx opened this issue Sep 20, 2024 · 12 comments
Closed
3 of 5 tasks

1.18.0 seems causing bundle errors on Next.js >= 14.2.x #784

arvinxx opened this issue Sep 20, 2024 · 12 comments

Comments

@arvinxx
Copy link

arvinxx commented Sep 20, 2024

Validations

Describe the bug

Thank you for bring this amazing work! LobeChat have this beautiful code highlight all credit to you~

But it seems there is a breaking change for 1.18.0 which case the code highlight invalid.

So I pin to 1.17.7 temporarily with the PR to fix it.

image

Our usage code is:

import { useThemeMode } from 'antd-style';
import { Loader2 } from 'lucide-react';
import { memo } from 'react';
import { Flexbox } from 'react-layout-kit';

import Icon from '@/Icon';
import { useHighlight } from '@/hooks/useHighlight';
import { DivProps } from '@/types';

import { useStyles } from './style';

export interface SyntaxHighlighterProps extends DivProps {
  children: string;
  language: string;
}

const SyntaxHighlighter = memo<SyntaxHighlighterProps>(
  ({ children, language, className, style }) => {
    const { styles, cx } = useStyles();
    const { isDarkMode } = useThemeMode();

    const { data, isLoading } = useHighlight(children.trim(), language, isDarkMode);

    return (
      <>
        {isLoading || !data ? (
          <div className={cx(styles.unshiki, className)} style={style}>
            <pre>
              <code>{children.trim()}</code>
            </pre>
          </div>
        ) : (
          <div
            className={cx(styles.shiki, className)}
            dangerouslySetInnerHTML={{
              __html: data as string,
            }}
            style={style}
          />
        )}
        {isLoading && (
          <Flexbox
            align={'center'}
            className={styles.loading}
            gap={8}
            horizontal
            justify={'center'}
          >
            <Icon icon={Loader2} spin />
          </Flexbox>
        )}
      </>
    );
  },
);

export default SyntaxHighlighter;

and the code in useHighlight is:

import {
  transformerNotationDiff,
  transformerNotationErrorLevel,
  transformerNotationFocus,
  transformerNotationHighlight,
  transformerNotationWordHighlight,
} from '@shikijs/transformers';
import { type Highlighter, getHighlighter } from 'shiki';
import useSWR from 'swr';

import { themeConfig } from '@/Highlighter/theme';

import languageMap from './languageMap';

export const FALLBACK_LANG = 'txt';

const FALLBACK_LANGS = [FALLBACK_LANG];

let cacheHighlighter: Highlighter;

const initHighlighter = async (lang: string): Promise<Highlighter> => {
  let highlighter = cacheHighlighter;
  const language = lang.toLowerCase();

  if (highlighter && FALLBACK_LANGS.includes(language)) return highlighter;

  if (languageMap.includes(language as any) && !FALLBACK_LANGS.includes(language)) {
    FALLBACK_LANGS.push(language);
  }

  highlighter = await getHighlighter({
    langs: FALLBACK_LANGS,
    themes: [themeConfig(true), themeConfig(false)],
  });

  cacheHighlighter = highlighter;

  return highlighter;
};

export const useHighlight = (text: string, lang: string, isDarkMode: boolean) =>
  useSWR(
    [lang.toLowerCase(), isDarkMode ? 'dark' : 'light', text].join('-'),
    async () => {
      try {
        const language = lang.toLowerCase();
        const highlighter = await initHighlighter(language);
        const html = highlighter?.codeToHtml(text, {
          lang: languageMap.includes(language as any) ? language : FALLBACK_LANG,
          theme: isDarkMode ? 'dark' : 'light',
          transformers: [
            transformerNotationDiff(),
            transformerNotationHighlight(),
            transformerNotationWordHighlight(),
            transformerNotationFocus(),
            transformerNotationErrorLevel(),
          ],
        });
        return html;
      } catch {
        return '';
      }
    },
    { revalidateOnFocus: false },
  );

export { default as languageMap } from './languageMap';

the source is here: https://github.com/lobehub/lobe-ui/blob/master/src/hooks/useHighlight.ts#L41


I dig it a little and find that the dom seems different:

v1.17.7 's dom:

image

but in the v1.18.0's dom:

image

it means that in the v1.18.0 the control flow is go to the condition of
isLoading || !data ? ( <div className={cx(styles.unshiki, className)} style={style}> <pre> <code>{children.trim()}</code> </pre> </div> ).

and then I try to log the error, and it show up with:

ShikiError: Language `ts` not found, you may need to load it first
    at Object.getLanguage (mf-dep____vendor.6f98842d.async.js:57165:13)
    at codeToTokensBase (mf-dep____vendor.6f98842d.async.js:56270:29)
    at codeToTokens (mf-dep____vendor.6f98842d.async.js:56559:14)
    at codeToHast (mf-dep____vendor.6f98842d.async.js:56622:7)
    at codeToHtml (mf-dep____vendor.6f98842d.async.js:56842:74)
    at Object.codeToHtml (mf-dep____vendor.6f98842d.async.js:57271:37)
    at _callee2$ (useHighlight.ts:48:1)
    at tryCatch (mf-dep____vendor.6f98842d.async.js:35914:16)
    at Generator.<anonymous> (mf-dep____vendor.6f98842d.async.js:36002:17)
    at Generator.next (mf-dep____vendor.6f98842d.async.js:35943:21)
    at asyncGeneratorStep (mf-dep____vendor.6f98842d.async.js:35533:24)
    at _next (mf-dep____vendor.6f98842d.async.js:35552:9)
lockdown-install.js:1 
image

Why it's worked in v1.17.7 but breaking in the 1.18.0 ?

Reproduction

https://github.com/lobehub/lobe-chat/tree/reproduction/shiki-1.18.0

Contributes

  • I am willing to submit a PR to fix this issue
  • I am willing to submit a PR with failing tests
@arvinxx arvinxx changed the title 1.18.0 case a Breaking Change for highlight in some case 1.18.0 case a breaking change for highlight in some case Sep 20, 2024
@arvinxx arvinxx changed the title 1.18.0 case a breaking change for highlight in some case 1.18.0 seems causing a breaking change for highlight in some case Sep 20, 2024
@fuma-nama
Copy link
Contributor

Hmm I'm not quite sure what does the logic in initHighlighter do. Normally to lazy load required languages, we use async highlighter.loadLanguage function in advanced of codeToHast/codeToHtml.

Here's my example tested in CodeSandbox:

"use client";

import { useEffect, useState } from "react";
import { BundledLanguage, createHighlighter } from "shiki";

const snippets = [
  {
    lang: "ts",
    code: `export const hello: string = "Hello"`,
  },
  {
    lang: "java",
    code: `System.out.println("Hello")`,
  },
  {
    lang: "py",
    code: `print(f"Hello")`,
  },
];

export default function Page() {
  return <div>{snippets.map(item => <Highlight key={item.lang} {...item} />)}</div>;
}

const highlighter = createHighlighter({
  langs: ["ts"], // preload language
  themes: ["vitesse-dark"],
});

function Highlight({ code, lang }: { code: string; lang: string }) {
  const [html, setHtml] = useState<string>();

  useEffect(() => {
    // TEST: do it on browser only
    async function run() {
      const instance = await highlighter;
      if (!instance.getLoadedLanguages().includes(lang)) {
        console.log('load', lang)
        // the language may not exist, we assume it is always valid
        await instance.loadLanguage(lang as BundledLanguage);
      }

      setHtml(instance.codeToHtml(code, {
        lang,
        theme: 'vitesse-dark'
      }))
    }
    void run()
  }, [code, lang]);

  if (!html) return null
  return <div dangerouslySetInnerHTML={{ __html: html }} />
}

Notice that the function codeToHast/codeToHtml exported from shiki does lazy load by default, this is only required for highlighter API.

As you are using React, I would suggest you to check https://shiki.style/packages/next#custom-components, which illustrates the way to render JSX instead of HTML with Shiki.

@antfu
Copy link
Member

antfu commented Sep 23, 2024

Shiki v1.18 does not introduce any functionality changes (v1.17.7...v1.18.0). The only thing notable is #781 - which seems not related to your case.

The example and repo has too much React related logic for me to take a look, can you provide a minimal reproduction instead?

@silas
Copy link

silas commented Sep 25, 2024

I'm not sure if it's the same issue, but we had to revert to v1.17.7 because we started seeing the following error:

SyntaxError: Bad control character in string literal in JSON at position 2044 (line 1 column 2045)
index.tsx:379 SyntaxError: Bad control character in string literal in JSON at position 2044 (line 1 column 2045)
    at JSON.parse (<anonymous>)
    at 58090 (javascript.mjs:1:1)
    at Function.y (bootstrap:21:1)
    at async index.mjs:1399:1
    at async Promise.all (index 0)
    at async nx (index.mjs:1398:1)
    at async Object.loadLanguage (index.mjs:1658:1)
    at async Promise.all (index 1)
    at async n (index.mjs:1839:1)
    at async codeToHtml (index.mjs:1855:1)

We don't see this issue in the dev, so I'm trying to dig into the production build files to figure out exactly what's going on.

@silas
Copy link

silas commented Sep 25, 2024

So my issue was specifically related to #781 and the [email protected] bundler (webpack I think) incorrectly bundling shiki/dist/langs/javascript.mjs.

The v1.17.7 version of javascript.mjs (plain object):

Object.freeze({ "displayName": ... })

And the v1.18.0 version (backticks with serialized JSON):

Object.freeze(JSON.parse(`{"displayName": ... `))

And the Next.js bundled version of v1.18.0 (single quotes with corrupted serialized JSON):

Object.freeze(JSON.parse('{"displayName": ... '))

Both of Shikijs versions are valid, but the Next.js bundler corrupts the \b in the arrow-function match regex, which ends up as \\\\\b (5 backslashes) in the Next.js bundled version, whereas it's \\\\b (4 backslashes) in the dist version of [email protected].

lobe-chat is also a Next.js app and when I run the build I see the same \\\\\b issue.

@antfu
Copy link
Member

antfu commented Sep 26, 2024

Thanks for the investigation! I guess that sounds more like a Webpack bug? Would you create a repro to webpack instead?

@antfu
Copy link
Member

antfu commented Sep 26, 2024

I tried briefly with Webpack 5.87.0, it seems to bundle fine, maybe related to Next.js' specific setup?

@fuma-nama
Copy link
Contributor

Oh you are right, also encountered the same problem when using Shiki in a Next.js app.

I was using Shiki in a package in my monorepo. I found installing shiki to the app workspace can fix this issue, it’s very likely some production build optimisation that bundles peer dependency. I’m not familiar with Next.js bundler so perhaps opening an issue on Next.js helps.

Also maybe we can revert this change for now? It’s a performance improvement so reverting the change shouldn’t affect anything.

@antfu
Copy link
Member

antfu commented Sep 26, 2024

If this is confirmed as a bug of Next.js, I think it would be a rather severe one. It would be better to not revert on our side which would shallow the bug for them.

@silas
Copy link

silas commented Sep 26, 2024

As most of you have already noticed I created a bug in Next.js, but really have no idea when it will get fixed (might need to get pushed further upstream to webpack and/or babel and that could take a while).

It might be worth updating the issue title to specifically mention Next.js >= 14.2.x (I don't see the issue in Next.js 14.1.x) for future debuggers riding the dependabot update train.

@antfu antfu changed the title 1.18.0 seems causing a breaking change for highlight in some case 1.18.0 seems causing bundle errors on Next.js Sep 26, 2024
@antfu antfu changed the title 1.18.0 seems causing bundle errors on Next.js 1.18.0 seems causing bundle errors on Next.js >= 14.2.x Sep 26, 2024
@fuma-nama
Copy link
Contributor

I can confirm this is fixed by PR #795 on the latest release of Shiki, tested on Next.js 14.2.13.
For previous versions, Next.js 15 canary has fixed the issue, but they haven't ported the fix to v14.

@silas
Copy link

silas commented Sep 29, 2024

Same, #795 fixed the issue for me as well.

@arvinxx
Copy link
Author

arvinxx commented Sep 30, 2024

I have checked with v1.21.0. And it seems work now. and I have unpinned the shiki and upgrade to the latest version.(lobehub/lobe-chat#4218)

I think this issue can be closed now. Thanks for your all great job! 🎉

@arvinxx arvinxx closed this as completed Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants