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 step ( Reviewed by Cursor Bugbot for commit a7081c9. 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.84% -0.06%
==========================================
Files 105 105
Lines 8889 8889
Branches 326 327 +1
==========================================
- Hits 6569 6564 -5
- Misses 2319 2324 +5
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.
| import { writeFileSync } from 'node:fs'; | ||
|
|
||
| import generateTwoslashFsMap from './generate.mjs'; | ||
|
|
||
| const fsMap = generateTwoslashFsMap(); | ||
|
|
||
| writeFileSync( | ||
| new URL('../../generated/twoslash-fsmap.json', import.meta.url), |
There was a problem hiding this comment.
writeFileSync will throw in a clean checkout because apps/site/generated/ is not created anywhere before writing twoslash-fsmap.json. Ensure the target directory exists (e.g., mkdirSync(..., { recursive: true })) before writing the file, or write to an existing output directory.
| import { writeFileSync } from 'node:fs'; | |
| import generateTwoslashFsMap from './generate.mjs'; | |
| const fsMap = generateTwoslashFsMap(); | |
| writeFileSync( | |
| new URL('../../generated/twoslash-fsmap.json', import.meta.url), | |
| import { mkdirSync, writeFileSync } from 'node:fs'; | |
| import generateTwoslashFsMap from './generate.mjs'; | |
| const fsMap = generateTwoslashFsMap(); | |
| const outputFile = new URL('../../generated/twoslash-fsmap.json', import.meta.url); | |
| mkdirSync(new URL('../../generated/', import.meta.url), { recursive: true }); | |
| writeFileSync( | |
| outputFile, |
| ] | ||
| }, | ||
| "build:twoslash-fsmap": { | ||
| "inputs": [], |
There was a problem hiding this comment.
Turbo task build:twoslash-fsmap is configured with inputs: [], which makes its cache key effectively constant and can cause stale generated/twoslash-fsmap.json to be reused even when the generator script or dependency versions change. Add appropriate inputs (e.g., scripts/twoslash-fsmap/**, and the relevant package.json/lockfile entries) so the task reruns when its true inputs change.
| "inputs": [], | |
| "inputs": [ | |
| "scripts/twoslash-fsmap/**", | |
| "package.json", | |
| "../../package.json", | |
| "../../pnpm-lock.yaml", | |
| "../../package-lock.json", | |
| "../../yarn.lock", | |
| "../../bun.lockb", | |
| "../../bun.lock" | |
| ], |
| compilerOptions: { | ||
| moduleResolution: 100, | ||
| // Explicitly include @types/node so that the VFS resolves Node.js | ||
| // globals and `node:*` module imports from the bundled declarations. | ||
| types: ['node'], | ||
| }, |
There was a problem hiding this comment.
moduleResolution: 100 is a hard-coded enum value (Bundler) and is brittle across TypeScript versions/readability. Since ts is available here, prefer moduleResolution: ts.ModuleResolutionKind.Bundler (or the intended enum) to make the intent explicit and avoid relying on numeric constants.
| * `twoslash`. This is needed for environments like Cloudflare Workers where | ||
| * the filesystem-backed default twoslasher cannot be used. | ||
| * | ||
| * @param {import('@shikijs/twoslash').TransformerTwoslashOptions} [options] |
There was a problem hiding this comment.
The JSDoc option type here (TransformerTwoslashOptions) is inconsistent with the type used by the caller (TransformerTwoslashIndexOptions in packages/rehype-shiki/src/index.mjs). Align these so editor/TS inference matches the actual options object supported by this helper.
| * @param {import('@shikijs/twoslash').TransformerTwoslashOptions} [options] | |
| * @param {import('../../index.mjs').TransformerTwoslashIndexOptions} [options] |
| export const twoslash = (options = {}) => { | ||
| if (options.twoslasher) { | ||
| return createTransformerFactory( | ||
| options.twoslasher, | ||
| rendererRich(rendererOptions) | ||
| )({ ...transformerOptions, ...options }); | ||
| } | ||
|
|
||
| return transformerTwoslash({ ...transformerOptions, ...options }); | ||
| }; |
There was a problem hiding this comment.
This change adds a new execution path when options.twoslasher is provided (switching from transformerTwoslash to createTransformerFactory). There are existing unit tests for the rehype-shiki package but none covering this branching behavior; add a small unit test that verifies the factory path is used when twoslasher is set (and the default path otherwise) to prevent regressions.
| const tsLibFiles = readdirSync(tsLibDir).filter( | ||
| f => f.startsWith('lib.') && /\.d\.([^.]+\.)?[cm]?ts$/i.test(f) | ||
| ); | ||
|
|
||
| for (const file of tsLibFiles) { | ||
| fsMap[`/${file}`] = readFileSync(join(tsLibDir, file), 'utf8'); | ||
| } |
There was a problem hiding this comment.
readdirSync() ordering is filesystem-dependent, so the generated fsMap (and resulting JSON) can be non-deterministic across environments. To keep builds reproducible and Turbo caching stable, sort tsLibFiles (and ideally directory entries in collectDtsFiles) before adding them to fsMap.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
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 a7081c9. Configure here.
| new URL('../../generated/twoslash-fsmap.json', import.meta.url), | ||
| JSON.stringify(fsMap), | ||
| 'utf8' | ||
| ); |
There was a problem hiding this comment.
Missing directory creation causes build script failure
High Severity
The writeFileSync call writes to ../../generated/twoslash-fsmap.json, but the generated directory is added to .gitignore and contains no tracked files, so it won't exist on a fresh clone. Node.js's writeFileSync does not create parent directories automatically, meaning the build:twoslash-fsmap script will crash with an ENOENT error. Since cloudflare:build:worker depends on this task, all Cloudflare builds from clean checkouts (including CI) will fail.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit a7081c9. Configure here.
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
Description
Note
Disclaimer, I simply asked open-code to enable twoslashe on Cloudflare and this is what it came with.
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.