Skip to content

feat: expose Shiki adapter#7

Closed
exKAZUu wants to merge 3 commits intomainfrom
expose-shiki-to-monaco
Closed

feat: expose Shiki adapter#7
exKAZUu wants to merge 3 commits intomainfrom
expose-shiki-to-monaco

Conversation

@exKAZUu
Copy link
Copy Markdown
Member

@exKAZUu exKAZUu commented Apr 18, 2026

Summary

  • re-export shikiToMonaco from @willbooster/monaco-react
  • add @shikijs/monaco as a package dependency
  • document Shiki integration and single-package installation
  • include built dist artifacts for GitHub package consumers

Verification

  • yarn check-for-ai
  • yarn build

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces Shiki integration by exporting the shikiToMonaco utility and adding a dedicated usage section to the documentation. It also refines the installation guide by clarifying peer dependencies and re-organizes the package.json structure. Feedback focuses on improving the clarity of the Shiki example in the README and optimizing the package's dependency management by moving @shikijs/monaco to peer dependencies to prevent unnecessary bundle size increases for users not utilizing the feature.

Comment thread README.md

<Editor
beforeMount={(monaco) => {
shikiToMonaco(highlighter, monaco);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The highlighter variable is used in the example but is not defined or initialized. This might be confusing for users unfamiliar with Shiki's initialization process. It would be helpful to add a comment or a brief initialization snippet to clarify where highlighter comes from.

Suggested change
shikiToMonaco(highlighter, monaco);
// highlighter is an instance created by Shiki's getHighlighter
shikiToMonaco(highlighter, monaco);

Comment thread package.json
@@ -54,6 +54,7 @@
"typecheck": "tsgo --noEmit"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Adding @shikijs/monaco as a direct dependency will increase the bundle size for all users of this package, even those who do not use the Shiki integration. Since Shiki and its dependencies (like @shikijs/core) are relatively heavy, consider moving this to peerDependencies and devDependencies, or using a dynamic import for the re-export if the environment supports it.

@exKAZUu
Copy link
Copy Markdown
Member Author

exKAZUu commented Apr 18, 2026

Closing this approach: @shikijs/monaco should remain a Shiki integration dependency owned by consumers, not be re-exported from @willbooster/monaco-react.

@exKAZUu exKAZUu closed this Apr 18, 2026
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.

1 participant