Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
👋 Codeowner Review RequestThe following codeowners have been identified for the changed files: Team reviewers: @nodejs/nodejs-website Please review the changes when you have a chance. Thank you! 🙏 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8632 +/- ##
==========================================
+ Coverage 75.09% 75.11% +0.01%
==========================================
Files 104 104
Lines 9088 9094 +6
Branches 314 314
==========================================
+ Hits 6825 6831 +6
Misses 2261 2261
Partials 2 2 ☔ View full report in Codecov by Sentry. |
1cca458 to
6550a6b
Compare
|
cc @nodejs/nodejs-website |
There was a problem hiding this comment.
Pull request overview
This PR adds TypeScript type publishing support for @node-core/rehype-shiki (via generated declaration files) to improve downstream TypeScript consumption, and refactors internal imports to use a package-scoped #rs/* alias.
Changes:
- Add
tsconfig.json+ TypeScript tooling/scripts to emit declaration files for@node-core/rehype-shiki, and expose them viapackage.jsontypes/exports. - Introduce
#rs/*internal import map and update internal imports/JSDoc type imports to use it. - Remove
incrementalfrom@node-core/ui-componentsTypeScript config.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ui-components/tsconfig.json | Removes incremental setting (unrelated to rehype-shiki type export goal). |
| packages/rehype-shiki/tsconfig.json | New TS config to emit declarations only into dist/. |
| packages/rehype-shiki/package.json | Adds types + exports types condition, imports map, and TS-related scripts/devDependency. |
| packages/rehype-shiki/eslint.config.js | Enables type-aware linting via local tsconfig.json. |
| packages/rehype-shiki/src/plugin.mjs | Switches to #rs/* import and updates JSDoc type import paths accordingly. |
| packages/rehype-shiki/src/minimal.mjs | Switches internal import to #rs/*. |
| packages/rehype-shiki/src/index.mjs | Switches internal import + dynamic import to #rs/*. |
| packages/rehype-shiki/src/highlighter.mjs | Adds JSDoc-typed helper export and adjusts exposed Shiki core type annotation. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
📦 Build Size ComparisonSummary
Changes➕ Added Assets (1)
➖ Removed Assets (1)
|
|
I don't think this is necessary, but I could be wrong |
Kinda. Technically speaking VS Code will automatically resolve with its own IntelliSense, yes and use the JSDocs. But, adding this actually allows us to check for type errors. For example, this allowed me to see that HighlighterCoreSync type didn't exist anymore. So this allows us to catch errors. Even if the source is pure .mjs and TS is not needed tho. We could even simply not ship the d.ts, but I want to also be a TypeScript citizen without really needing TypeScript lol |
d1ba4b6 to
4e4e092
Compare
|
cc @nodejs/nodejs-website |
avivkeller
left a comment
There was a problem hiding this comment.
Minus the ui-components changes, LGTM
AugustinMauroy
left a comment
There was a problem hiding this comment.
LGMT ! that's good idea
This PR introduces type exports for @node-core/rehype-shiki, which IMO is important for downstream consumers/importers of the package, such as doc-kit. Even if the runtime doesn't require the types.