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

Shadow breaking change in underlying shiki dependency #2197

Closed
benjag opened this issue Jul 25, 2023 · 6 comments
Closed

Shadow breaking change in underlying shiki dependency #2197

benjag opened this issue Jul 25, 2023 · 6 comments

Comments

@benjag
Copy link

benjag commented Jul 25, 2023

Environment

  • Operating System: Darwin
  • Node Version: v16.16.0
  • Nuxt Version: 3.6.5
  • Nitro Version: 2.5.2
  • Package Manager: npm@8.11.0
  • Builder: vite
  • User Config: app, modules, css, content
  • Runtime Modules: @nuxt/content@2.7.2, @vueuse/nuxt@9.13.0
  • Build Modules: -

Reproduction

https://stackblitz.com/edit/github-d56zne?file=content%2Findex.md

Describe the bug

There was a shadow breaking change in the underlying shiki highlighter dependency when using a material-* theme. Shiki renamed the themes causing any existing material theme references in the nuxt config to break and crash the app at runtime.

https://github.com/shikijs/shiki/releases/tag/v0.13.0

I did not see anything in the nuxt/content release notes or any other open issues so I figured you may want to call this out in your docs. I had to hunt for a bit to figure out what happened.

Additional context

No response

Logs

No response

@nobkd
Copy link
Contributor

nobkd commented Jul 25, 2023

You can make a PR to add some info to the docs.

I don't really think that it is needed, because:

  • The highlight option docs link to the Shiki themes list which states the up-to-date names.
  • It has been quite some time (6 month) since the renaming happened in Shiki.
  • The error message is relatively clear, as it states, that the theme asset is unknown / not found.

But that's just my thoughts.

@benjag
Copy link
Author

benjag commented Jul 25, 2023

Fair point, but in this case. It appears that the shiki update was brought in to @nuxt/content quite recently and that a test had to updated to address the breaking change.

Just feel like it's worth a mention in the release notes at least

@nobkd
Copy link
Contributor

nobkd commented Jul 25, 2023

Good that you mention this, I didn't see the dependency and test update.
It probably also explains, why #1936 didn't work back then.

Since this was only recently updated here in Nuxt Content, I agree with you, that it's probably good to mention it in the docs or some notes.

Though, only the material themes were renamed.

@benjag
Copy link
Author

benjag commented Jul 25, 2023

I do think you're correct that it doesn't need to be in the actual documentation, but a quick callout in the changelog would be helpful

@farnabaz
Copy link
Member

Sorry for causing inconvenient
I will add a note in the release notes, hope others will notice this change before wasting too much time

@farnabaz
Copy link
Member

Just add a note in both v2.7.1 and v2.7.2 release notes.
Thanks for the issue and raising concern.

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

No branches or pull requests

3 participants