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: bump shiki to v1, migrate to official transformers #1672

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

JounQin
Copy link
Contributor

@JounQin JounQin commented Dec 17, 2024

Summary

Related Issue

related #1634

Checklist

  • Tests updated (or not required).
  • Documentation updated (or not required).

Copy link

netlify bot commented Dec 17, 2024

Deploy Preview for aquamarine-blini-95325f ready!

Name Link
🔨 Latest commit d8cd2c2
🔍 Latest deploy log https://app.netlify.com/sites/aquamarine-blini-95325f/deploys/676122c7d7c0cb0008de9610
😎 Deploy Preview https://deploy-preview-1672--aquamarine-blini-95325f.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 94 (🟢 up 9 from production)
Accessibility: 97 (no change from production)
Best Practices: 92 (no change from production)
SEO: 100 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

@JounQin JounQin force-pushed the feat/bump_shiki branch 2 times, most recently from aa5cf68 to e11037c Compare December 17, 2024 06:24
@@ -54,7 +54,7 @@
"dependencies": {
"@rspress/shared": "workspace:*",
"hast-util-from-html": "2.0.3",
"shiki": "0.14.7",
"shiki": "1.24.2",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure why we lock dep versions in package.json.

],
}),
],
});
```

接着我们来介绍一下如何使用这些 Transformer 对应的语法。
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can not remove exports in this plugin, you should reexport in this plugin and keep all docs here to avoid breaking change.

Furthermore, we can give an example in docs to introduce how to import other transformers in this plugin.

Copy link
Contributor Author

@JounQin JounQin Dec 17, 2024

Choose a reason for hiding this comment

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

The transformers signatures are changed, so I believe we should make a breaking change for this plugin.

And those builtin transformers are from @shikijs/transformers, I didn't add it as direct dependency because maybe same users don't use them.

Furthermore, we can give an example in docs to introduce how to import other transformers in this plugin.

https://github.com/JounQin/rspress/blob/e11037c06d110575bda056f9a8200049d7d303b2/packages/document/docs/zh/plugin/official-plugins/shiki.mdx?plain=1#L71-L97

Copy link
Collaborator

Choose a reason for hiding this comment

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

breaking change for now is not acceptable, and we can make this breaking change in Rspress 2.0 since all core and package are using same version for now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If you want a canary version now, I can release for you for this plugin.

Copy link
Contributor Author

@JounQin JounQin Dec 17, 2024

Choose a reason for hiding this comment

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

I don't quite understand, all official plugin major versions are bound with rspress core? That would make plugin evolution too slow maybe.

since all core and package are using same version for now.

OK, I understand now. That seems a big challenge.

If you want a canary version now, I can release for you for this plugin.

Sweet. Or maybe I just fork the codes temporarily.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh it seems the release workflow can not run in a forked repo, I think you can just patch this plugin using pnpm patch or other tools in package manager level.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hah, I mean to copy the codes into our own repo as a thirty-party plugin, I'm building a company level document tool, so local patch won't work.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Got it, this package is MIT license, just do it.

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

Successfully merging this pull request may close these issues.

2 participants