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: use nuxt-shiki #164

Closed
wants to merge 2 commits into from

Conversation

cpreston321
Copy link

@cpreston321 cpreston321 commented Mar 14, 2024

resolves #148

In this PR I added the module nuxt-shiki.

Which controls the main core level to shiki like loading wasm binary, exposes functions for other projects to then consume in there projects.

cc:// @Atinux @pi0

src/module.ts Show resolved Hide resolved
Copy link
Contributor

@pi0 pi0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. LGTM with overall changes.

Hnading over to @farnabaz to see what is needed.

@cpreston321 cpreston321 marked this pull request as ready for review March 14, 2024 18:53
@cpreston321
Copy link
Author

cpreston321 commented Mar 14, 2024

@pi0 Since the module is installed and loadShiki is under #imports, whats a good way to test this? I haven't found a good way to import without the ci:test throwing errors. I can mock it but I need to pull that from exports from nuxt-shiki to test.

CleanShot 2024-03-14 at 17 01 48@2x

@pi0
Copy link
Contributor

pi0 commented Mar 14, 2024

Checking locally, we need to update #mdc-highlighter / mdc-highlighter.js template as well.

I will check tomorrow more too. (also /cc @antfu if you have any recommendations for migration)

@farnabaz
Copy link
Collaborator

farnabaz commented Mar 15, 2024

Using nuxt-shiki will break the possibility of using @nuxtjs/mdc package outside Nuxt scope.

Currently, there are some known use-cases that we are using MDC module to parse/render markdown files in non-Nuxt applications.

@nuxtjs/mdc is an entry point for users to use MDC syntax and should work outside Nuxt too.

@pi0
Copy link
Contributor

pi0 commented Mar 15, 2024

Interesting point... I am open to also expose runtime of nuxt-shiki to be directly usable if that would help adoption/unify usage.

@pi0
Copy link
Contributor

pi0 commented Mar 15, 2024

on it

@farnabaz
Copy link
Collaborator

What do you think of improving the usage in MDC instead of separating the logic into another package?
For me, Shiki could be part of MDC module and users can use it separately if they need to.

@pi0
Copy link
Contributor

pi0 commented Mar 15, 2024

  • Maintaining shiki/wasm support within another module is super tricky today and we faced multiple regressions
  • When users need to use Shiki outside of mdc, it will have become super tricky as well with multiple implementations

I am building nuxt-shiki in good faith for our entire ecosystem and if you have any more needs that need to be considered 💯 welcome. (PS: you are already a maintainer of nuxt-shiki :)

@farnabaz
Copy link
Collaborator

I understand how tricky it is. I'm not against building nuxt-shiki, IMO we can just take similar approach in MDC module to improve usability, unify the implementation.

I just don't get why it needs to be done in a separate module. What is the limitation that we need to separate it? Maybe I'm missing something.

@pi0
Copy link
Contributor

pi0 commented Mar 15, 2024

Simply not duplicating the same thing and reducing maintenance/sync costs. Sent you a message in Slack if you like to talk :)

@farnabaz
Copy link
Collaborator

  • Maintaining shiki/wasm support within another module is super tricky today and we faced multiple regressions
  • When users need to use Shiki outside of mdc, it will have become super tricky as well with multiple implementations

I am building nuxt-shiki in good faith for our entire ecosystem and if you have any more needs that need to be considered 💯 welcome. (PS: you are already a maintainer of nuxt-shiki :)

Again I'm not against nuxt-shiki nor standing in front of it, I asked to improve it in MDC module. And so far I didn't get why we can't do all improvments inside MDC module and expose same/similar helpers.

Anyway like I said in nuxt/ui#1361 (comment) we can adopt it.
Feel free to update this PR and we can merge it :)

@pi0
Copy link
Contributor

pi0 commented Apr 8, 2024

@cpreston321 @farnabaz I am no longer interested in this topic. feel free to continue on this PR or close...

@cpreston321
Copy link
Author

I will close this PR.

@cpreston321 cpreston321 closed this Apr 8, 2024
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

Successfully merging this pull request may close these issues.

Use nuxt-shiki for syntax highlighting
3 participants