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

🐛 BUG: Shiki missing language registration should not fail markdown processing #3881

Closed
1 task done
crutchcorn opened this issue Jul 10, 2022 · 3 comments · Fixed by #3911
Closed
1 task done
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority) feat: markdown Related to Markdown (scope)

Comments

@crutchcorn
Copy link
Contributor

crutchcorn commented Jul 10, 2022

What version of astro are you using?

1.0.0-beta.65

Are you using an SSR adapter? If so, which one?

None

What package manager are you using?

npm

What operating system are you using?

Windows

Describe the Bug

When using Shiki, the following Markdown will fail:

```nasm
100001 # addu
```

With the following error:

 error   Failed to parse Markdown file "C:\Users\crutchcorn\git\Astro\unicorn-astro\content\blog\how-computers-speak\index.md":
  No language registration for nasm
    at getGrammar (C:\Users\crutchcorn\git\Astro\unicorn-astro\node_modules\shiki\dist\index.js:2175:19)
    at codeToThemedTokens (C:\Users\crutchcorn\git\Astro\unicorn-astro\node_modules\shiki\dist\index.js:2184:30)
    at Object.codeToHtml (C:\Users\crutchcorn\git\Astro\unicorn-astro\node_modules\shiki\dist\index.js:2201:24)
    at file:///C:/Users/crutchcorn/git/Astro/unicorn-astro/node_modules/@astrojs/markdown-remark/dist/remark-shiki.js:17:30
    at overload (file:///C:/Users/crutchcorn/git/Astro/unicorn-astro/node_modules/unist-util-visit/index.js:50:16)
    at node (code) (file:///C:/Users/crutchcorn/git/Astro/unicorn-astro/node_modules/unist-util-visit-parents/index.js:104:31)
    at node (root) (file:///C:/Users/crutchcorn/git/Astro/unicorn-astro/node_modules/unist-util-visit-parents/index.js:121:79)
    at visitParents (file:///C:/Users/crutchcorn/git/Astro/unicorn-astro/node_modules/unist-util-visit-parents/index.js:61:30)
    at visit (file:///C:/Users/crutchcorn/git/Astro/unicorn-astro/node_modules/unist-util-visit/index.js:42:7)
    at file:///C:/Users/crutchcorn/git/Astro/unicorn-astro/node_modules/@astrojs/markdown-remark/dist/remark-shiki.js:16:5 (x2)

As @Princesseuh pointed out on Discord, it seems like this behavior is thrown from Shiki itself

https://github.com/shikijs/shiki/blob/b0cade2f48f3d6e5c0897c81d0dd06c81f5916ab/packages/shiki/src/highlighter.ts#L108

But Shiki's official remark plugin seems to catch this error:

https://github.com/shikijs/twoslash/blob/main/packages/remark-shiki-twoslash/src/index.ts#L167-L172

I think this should be the same behavior we seek to implement in Astro. Otherwise, it enables other's content to break a build, which seems to be too harsh of an action

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-nxlpqd?file=src%2Fpages%2Fposts%2Findex.md&on=stackblitz

Participation

  • I am willing to submit a pull request for this issue.
@natemoo-re natemoo-re added the feat: markdown Related to Markdown (scope) label Jul 11, 2022
@matthewp
Copy link
Contributor

What behavior are you expecting to happen here? I can't tell from the linked code.

@natemoo-re natemoo-re added s0-extra-small - P4: important Violate documented behavior or significantly impacts performance (priority) labels Jul 11, 2022
@Princesseuh
Copy link
Member

What I'd personally expect is the following:

  • Fallback to plaintext automatically whenever the language doesn't exist instead of throwing an error and crashing the server
  • Show a warning / error in the console, something like "The chosen language {language} doesn't exist, falling back to plaintext"

@crutchcorn
Copy link
Contributor Author

100% agree with @Princesseuh in her expected behavior. This is what the ""upstream"" remark-shiki-twoslash plugin does.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority) feat: markdown Related to Markdown (scope)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants