Conversation
There was a problem hiding this comment.
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.
|
|
||
| <Editor | ||
| beforeMount={(monaco) => { | ||
| shikiToMonaco(highlighter, monaco); |
There was a problem hiding this comment.
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.
| shikiToMonaco(highlighter, monaco); | |
| // highlighter is an instance created by Shiki's getHighlighter | |
| shikiToMonaco(highlighter, monaco); |
| @@ -54,6 +54,7 @@ | |||
| "typecheck": "tsgo --noEmit" | |||
There was a problem hiding this comment.
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.
|
Closing this approach: @shikijs/monaco should remain a Shiki integration dependency owned by consumers, not be re-exported from @willbooster/monaco-react. |
Summary
Verification