feat: enable Twoslash on Cloudflare#8837
feat: enable Twoslash on Cloudflare#8837dario-piotrowicz wants to merge 1 commit intonodejs:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryMedium Risk Overview Adds a build-time script that bundles TypeScript Reviewed by Cursor Bugbot for commit 841a898. Bugbot is set up for automated code reviews on this repo. Configure here. |
👋 Codeowner Review RequestThe following codeowners have been identified for the changed files: Team reviewers: @nodejs/nodejs-website @nodejs/web-infra 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 #8837 +/- ##
==========================================
- Coverage 73.90% 73.85% -0.05%
==========================================
Files 105 105
Lines 8889 8889
Branches 326 327 +1
==========================================
- Hits 6569 6565 -4
- Misses 2319 2323 +4
Partials 1 1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Pull request overview
Enables Twoslash-powered code annotations in the Cloudflare Workers deployment by providing a virtual filesystem (VFS) twoslasher and wiring in a build-time generated TypeScript/@types map, while updating the rehype-shiki Twoslash transformer to accept a custom twoslasher.
Changes:
- Add a VFS-backed Twoslash setup for Cloudflare that loads a pre-generated
twoslash-fsmap.json. - Introduce a build script (and Turbo task dependency) to generate the VFS map before Cloudflare worker builds.
- Update
@node-core/rehype-shiki’s Twoslash transformer to supportoptions.twoslasherviacreateTransformerFactory.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
packages/rehype-shiki/src/transformers/twoslash/index.mjs |
Adds support for passing a custom twoslasher (Cloudflare-compatible path). |
apps/site/mdx/plugins.mjs |
Enables Twoslash on Cloudflare by creating a VFS-based twoslasher and passing it through twoslashOptions. |
apps/site/scripts/twoslash-fsmap/index.mjs |
New script to write the generated VFS map JSON to apps/site/generated/. |
apps/site/scripts/twoslash-fsmap/generate.mjs |
New generator that builds a map of TS lib + @types/node declaration files. |
apps/site/turbo.json |
Adds build:twoslash-fsmap task and makes Cloudflare worker build depend on it. |
apps/site/package.json |
Adds build:twoslash-fsmap script entry. |
.gitignore |
Ignores apps/site/generated/ build artifacts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
dario-piotrowicz
force-pushed
the
dario/two-slash-on-cloudflare
branch
from
a7081c9 to
47d4edb
Compare
dario-piotrowicz
force-pushed
the
dario/two-slash-on-cloudflare
branch
from
47d4edb to
98310c5
Compare
dario-piotrowicz
force-pushed
the
dario/two-slash-on-cloudflare
branch
from
98310c5 to
2e5bac5
Compare
dario-piotrowicz
force-pushed
the
dario/two-slash-on-cloudflare
branch
from
2e5bac5 to
841a898
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 841a898. Configure here.
ovflowd
left a comment
ovflowd
left a comment
There was a problem hiding this comment.
Not fan of this diff/approach. Let's keep Twoslash disabled for now in Cloudflare and discuss our options.
| * generated at build time by `scripts/twoslash-fsmap/index.mjs`. | ||
| */ | ||
| async function createVfsTwoslasher() { | ||
| const [{ createTwoslasher }, ts, fsMapJson] = await Promise.all([ |
There was a problem hiding this comment.
We don't need promise all here, let's simply make one import statement per line and let's not mix then() here.
| * Uses a pre-built JSON map of TypeScript lib declarations and @types/node | ||
| * generated at build time by `scripts/twoslash-fsmap/index.mjs`. | ||
| */ | ||
| async function createVfsTwoslasher() { |
There was a problem hiding this comment.
This is so specific that should probably reside in another file IMO
There was a problem hiding this comment.
I don't think we really need any of this? I'm fairly sue twoslash supports using vfs out of the box? Since they have this CDN thing https://twoslash.netlify.app/packages/cdn (for example)
ovflowd
left a comment
ovflowd
left a comment
There was a problem hiding this comment.
Feel free to eventually remove my block as I'm getting away of the project for a while... but I'd appreciate if you could look into my comments and find ways of improving the fs map...
I'm really not a fan of YET another prebuild step of generating files. We already have the blog-data one which also is only needed because of Cloudflare Workers. Having one more is starting to make me regret that Cloudflare Workers cannot index these things in a practical way. Like it'd be great if this could just be uploaded to the build as for example Next.js's out of the box "outputFileTracingIncludes" supports that.
Description
Note
Disclaimer, This PR has been authored mostly by Claude Opus 4.6 with minor tweaks from me.
It does seem to work and the code makes sense to me.
Enables twoslash to work on Cloudflare (previously it would only work on the Vercel deployment):
Validation
See: https://nodejs-website.dario-test.workers.dev/en
Related Issues
Check List
pnpm formatto ensure the code follows the style guide.pnpm testto check if all tests are passing.pnpm buildto check if the website builds without errors.