Optimization suggestions for the 'hast' type in Shiki.js

NWYLZW commented 3 months ago

Validations

[X] Follow our Code of Conduct
[X] Read the Contributing Guide.
[X] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.

Describe the bug

While using Shiki, I encountered some issues that prevented me from correctly introducing certain type information into Shiki derivatives. Firstly, when I attempted to pass the correct type to the transformer, I found that Shiki/Core does not export the relevant types (related to 'hast'). The types I needed were bundled into the output instead of being an external dependency, although this behavior differs in other secondary applications. Here, I briefly investigated the package dependencies and potential issues involved.

@shikijs/rehyp -> hast-util-to-string has a dependency on 'hast'.
@shikijs/transformers -> @shiki -> @shiki/core all use 'hast' in the dependency chain but do not have a direct dependency on 'hast'.
@shikijs/twoslash does not have a 'hast' dependency but depends on 'shikijs core', which bundles 'hast' types.
@shikijs/vitepress-twoslash has a dependency on 'hast' through 'mdast-util-to-hast'.

These cases can be categorized as follows:

Using 'hast' types without marking them as dependencies, only as development dependencies, which may cause type-related issues when used independently.
Using 'hast' types with corresponding 'hast' dependencies in sub-dependencies, which are ghost dependencies that are not detected. It is not entirely safe but not a significant problem. (Why are there ghost dependencies even with pnpm? Because '@types/hast' is marked as a global dependency in the workspace.)
Using 'hast' types without marking them as external dependencies, resulting in bundling 'hast' types directly into the final output and making it impossible to use the exported types. Version issues may arise. For the above three cases, I suggest using peerDependencies to mark '@types/hast' and removing the workspace's '@types/hast' dependency to ensure proper detection of ghost dependencies.

I would like to see if there is a better way to handle this or if there are any other suggestions. If possible, I would like to fix this issue myself.

Reproduction

undefined

Contributes

[X] I am willing to submit a PR to fix this issue
[X] I am willing to submit a PR with failing tests

antfu commented 3 months ago

I would love to see a minimal reproduction repo where you encounter the error to further discuss the potential solutions.

@shikijs/core does not deps on hast in any form in the final bundle, as @types/hast get inlined into our bundle.

@shikijs/rehype does dep on hast, which we have the @types/hast in the dependencies already.

From high level I see the current behavior makes sense

NWYLZW commented 2 months ago

Hi, antfu. Thank you very much for your work on this issue. This fix has resolved my problem.

However, I found that twoslash also has this problem. I had written the code a couple of days ago and was ready to provide a fix for the hast type issue today. But I discovered that you have already solved it, hahaha. I will provide a fix for twoslash to shiki later.

shikijs / shiki