capnweb-validate: ship typescript as a capped dependency - #240

Merged
teamchong merged 1 commit into
cloudflare:mainfrom
Maximo-Guk:validate-own-compiler
Aug 12, 2026
Merged

capnweb-validate: ship typescript as a capped dependency#240
teamchong merged 1 commit into
cloudflare:mainfrom
Maximo-Guk:validate-own-compiler

Conversation

@Maximo-Guk

@Maximo-GukMaximo-Guk commented Aug 12, 2026

Copy link
Copy Markdown
Member

capnweb-validate's build-time transform needs the JS compiler API (transpileModule, createProgram, ts.sys), which TypeScript 7 (tsgo) no longer ships. The current peer is uncapped (>=5.7.0), so in a workspace whose typescript is 7.x the transform resolves a metadata-only package and dies at build time (ts.sys / ts.findConfigFile undefined). Working around it consumer-side takes a .pnpmfile.cjs hook, because pnpm resolves peers from the importer's context and neither overrides nor packageExtensions can move one.

This swaps the peer for a real dependency capped at <7: under a TS 7 workspace the transform gets its own JS-based compiler (transform compiler and check compiler are independent - checking with tsgo still works fine), and consumers on 5.7–6.x dedupe to their existing install. The 6.0.3 compiler is validated against the transform in a real consumer.

@changeset-bot

changeset-botBot commented Aug 12, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: af74f4d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
NameType
capnweb-validatePatch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new

pkg-pr-newBot commented Aug 12, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/cloudflare/capnweb@240

commit: af74f4d

The transform needs the JS compiler API, which TypeScript 7 (tsgo)
no longer ships. The uncapped peer (>=5.7.0) resolved to whatever the
consumer's workspace has, so under a TS 7 workspace the transform got
a metadata-only package and crashed at build time; consumers had to
work around it with a .pnpmfile.cjs rewriting the peer.
A real dependency capped at <7 gives the transform its own JS-based
compiler under TS 7 workspaces (transform compiler and check compiler
are independent), and dedupes against the consumer's install on
5.7-6.x. The lockfile diff is hand-minimized because npm 11.9 strips
the libc fields a newer npm wrote; npm ci validates it.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Maximo-Guk
Maximo-Gukforce-pushed the validate-own-compiler branch from 380aa93 to af74f4dCompareAugust 12, 2026 19:40
@Maximo-Guk
Maximo-Guk marked this pull request as ready for review August 12, 2026 19:42
@teamchong

Copy link
Copy Markdown
Collaborator

/bonk review this

@ask-bonkask-bonkBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: ship typescript as a capped dependency

The change is sound and correctly targeted. The root cause analysis is accurate: capnweb-validate's transform imports the typescript compiler API at build time (import ts from "typescript" in src/transform/{run,context,transform-module}.ts), and those entry points call ts.sys / ts.findConfigFile / transpileModule / createProgram — none of which exist in the metadata-only TS 7 (tsgo) package. Moving typescript from an uncapped peer to a >=5.7.0 <7 direct dependency guarantees a real JS compiler is present regardless of the workspace's TS version, and consumers on 5.7–6.x will dedupe.

Consistency check (all good)

  • package.json and the lockfile agree: dependencies.typescript = ">=5.7.0 <7", peer removed, devDep removed.
  • Resolved lockfile version 5.9.3 satisfies the new range and is no longer marked dev.
  • Root workspace still declares typescript: ^5.9.3, so in-repo build/test are unaffected.
  • Changeset wording matches the actual range.

Points worth confirming before merge

  1. Trade-off is a real duplicate install for TS 7 consumers. Under a TS 7 workspace, <7 forces a second, JS-based TypeScript (6.x) to be installed alongside the consumer's tsgo. That's the intended behavior and is stated in the PR body, but it's worth flagging explicitly for reviewers: this is a deliberate on-disk cost, not a bug.

  2. import type ts from "typescript" in type-introspector.ts is type-only and would have been erased — so the runtime need is genuinely the three value-importing modules. No concern, just confirming the peer->dep move is necessary rather than cosmetic.

  3. Making typescript a hard dependencies rather than an optional peer changes install semantics for all consumers, including any who only use the runtime ./internal exports and never the build-time transform. Those consumers will now always pull a TypeScript copy. If there is a runtime-only consumption path that never touches the compiler, consider whether typescript could instead be a capped peer (>=5.7.0 <7) so it stays out of pure-runtime installs. If the package is only ever consumed as a build-time transform, the direct dependency is the right call and this is a non-issue.

Nothing blocking from a correctness standpoint. Items 1 and 3 are design confirmations rather than defects.

@ask-bonk

ask-bonkBot commented Aug 12, 2026

Copy link
Copy Markdown

Review posted successfully. Summary of my assessment of PR #240:

Verdict: sound and correctly targeted, no blocking issues.

The change moves typescript from an uncapped peer dependency to a capped direct dependency (>=5.7.0 <7) on capnweb-validate.

What I verified:

  • The root cause is real — the transform value-imports the TS compiler API (ts.sys, findConfigFile, transpileModule, createProgram) in src/transform/{run,context,transform-module}.ts, which TS 7 (tsgo) doesn't ship.
  • package.json and package-lock.json are internally consistent (dep added, peer removed, devDep removed, dev: true flag dropped, resolved 5.9.3 satisfies the range).
  • Root workspace still has typescript: ^5.9.3, so in-repo build/test are unaffected.
  • Changeset wording matches the actual range.

Points I flagged for the maintainer (design confirmations, not defects):

  1. The <7 cap deliberately forces a duplicate JS-based TS install for TS 7 consumers — intended, but worth noting.
  2. type-introspector.ts uses a type-only import, confirming the runtime need is genuine.
  3. A hard dependencies entry means even runtime-only (./internal) consumers now always pull TypeScript; if such a path exists, a capped peer might be preferable — otherwise the direct dependency is correct.

github run

@teamchong
teamchong merged commit f7f7fa8 into cloudflare:mainAug 12, 2026
5 checks passed
@github-actionsgithub-actionsBot mentioned this pull request Aug 12, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@Maximo-Guk@teamchong
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

capnweb-validate: ship typescript as a capped dependency - #240

Merged
teamchong merged 1 commit into
cloudflare:mainfrom
Maximo-Guk:validate-own-compiler
Aug 12, 2026
Merged

capnweb-validate: ship typescript as a capped dependency#240
teamchong merged 1 commit into
cloudflare:mainfrom
Maximo-Guk:validate-own-compiler

Conversation

@Maximo-Guk

@Maximo-GukMaximo-Guk commented Aug 12, 2026

Copy link
Copy Markdown
Member

capnweb-validate's build-time transform needs the JS compiler API (transpileModule, createProgram, ts.sys), which TypeScript 7 (tsgo) no longer ships. The current peer is uncapped (>=5.7.0), so in a workspace whose typescript is 7.x the transform resolves a metadata-only package and dies at build time (ts.sys / ts.findConfigFile undefined). Working around it consumer-side takes a .pnpmfile.cjs hook, because pnpm resolves peers from the importer's context and neither overrides nor packageExtensions can move one.

This swaps the peer for a real dependency capped at <7: under a TS 7 workspace the transform gets its own JS-based compiler (transform compiler and check compiler are independent - checking with tsgo still works fine), and consumers on 5.7–6.x dedupe to their existing install. The 6.0.3 compiler is validated against the transform in a real consumer.

@changeset-bot

changeset-botBot commented Aug 12, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: af74f4d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
NameType
capnweb-validatePatch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new

pkg-pr-newBot commented Aug 12, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/cloudflare/capnweb@240

commit: af74f4d

The transform needs the JS compiler API, which TypeScript 7 (tsgo)
no longer ships. The uncapped peer (>=5.7.0) resolved to whatever the
consumer's workspace has, so under a TS 7 workspace the transform got
a metadata-only package and crashed at build time; consumers had to
work around it with a .pnpmfile.cjs rewriting the peer.
A real dependency capped at <7 gives the transform its own JS-based
compiler under TS 7 workspaces (transform compiler and check compiler
are independent), and dedupes against the consumer's install on
5.7-6.x. The lockfile diff is hand-minimized because npm 11.9 strips
the libc fields a newer npm wrote; npm ci validates it.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Maximo-Guk
Maximo-Gukforce-pushed the validate-own-compiler branch from 380aa93 to af74f4dCompareAugust 12, 2026 19:40
@Maximo-Guk
Maximo-Guk marked this pull request as ready for review August 12, 2026 19:42
@teamchong

Copy link
Copy Markdown
Collaborator

/bonk review this

@ask-bonkask-bonkBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: ship typescript as a capped dependency

The change is sound and correctly targeted. The root cause analysis is accurate: capnweb-validate's transform imports the typescript compiler API at build time (import ts from "typescript" in src/transform/{run,context,transform-module}.ts), and those entry points call ts.sys / ts.findConfigFile / transpileModule / createProgram — none of which exist in the metadata-only TS 7 (tsgo) package. Moving typescript from an uncapped peer to a >=5.7.0 <7 direct dependency guarantees a real JS compiler is present regardless of the workspace's TS version, and consumers on 5.7–6.x will dedupe.

Consistency check (all good)

  • package.json and the lockfile agree: dependencies.typescript = ">=5.7.0 <7", peer removed, devDep removed.
  • Resolved lockfile version 5.9.3 satisfies the new range and is no longer marked dev.
  • Root workspace still declares typescript: ^5.9.3, so in-repo build/test are unaffected.
  • Changeset wording matches the actual range.

Points worth confirming before merge

  1. Trade-off is a real duplicate install for TS 7 consumers. Under a TS 7 workspace, <7 forces a second, JS-based TypeScript (6.x) to be installed alongside the consumer's tsgo. That's the intended behavior and is stated in the PR body, but it's worth flagging explicitly for reviewers: this is a deliberate on-disk cost, not a bug.

  2. import type ts from "typescript" in type-introspector.ts is type-only and would have been erased — so the runtime need is genuinely the three value-importing modules. No concern, just confirming the peer->dep move is necessary rather than cosmetic.

  3. Making typescript a hard dependencies rather than an optional peer changes install semantics for all consumers, including any who only use the runtime ./internal exports and never the build-time transform. Those consumers will now always pull a TypeScript copy. If there is a runtime-only consumption path that never touches the compiler, consider whether typescript could instead be a capped peer (>=5.7.0 <7) so it stays out of pure-runtime installs. If the package is only ever consumed as a build-time transform, the direct dependency is the right call and this is a non-issue.

Nothing blocking from a correctness standpoint. Items 1 and 3 are design confirmations rather than defects.

@ask-bonk

ask-bonkBot commented Aug 12, 2026

Copy link
Copy Markdown

Review posted successfully. Summary of my assessment of PR #240:

Verdict: sound and correctly targeted, no blocking issues.

The change moves typescript from an uncapped peer dependency to a capped direct dependency (>=5.7.0 <7) on capnweb-validate.

What I verified:

  • The root cause is real — the transform value-imports the TS compiler API (ts.sys, findConfigFile, transpileModule, createProgram) in src/transform/{run,context,transform-module}.ts, which TS 7 (tsgo) doesn't ship.
  • package.json and package-lock.json are internally consistent (dep added, peer removed, devDep removed, dev: true flag dropped, resolved 5.9.3 satisfies the range).
  • Root workspace still has typescript: ^5.9.3, so in-repo build/test are unaffected.
  • Changeset wording matches the actual range.

Points I flagged for the maintainer (design confirmations, not defects):

  1. The <7 cap deliberately forces a duplicate JS-based TS install for TS 7 consumers — intended, but worth noting.
  2. type-introspector.ts uses a type-only import, confirming the runtime need is genuine.
  3. A hard dependencies entry means even runtime-only (./internal) consumers now always pull TypeScript; if such a path exists, a capped peer might be preferable — otherwise the direct dependency is correct.

github run

@teamchong
teamchong merged commit f7f7fa8 into cloudflare:mainAug 12, 2026
5 checks passed
@github-actionsgithub-actionsBot mentioned this pull request Aug 12, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@Maximo-Guk@teamchong
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

capnweb-validate: ship typescript as a capped dependency - #240

Merged
teamchong merged 1 commit into
cloudflare:mainfrom
Maximo-Guk:validate-own-compiler
Aug 12, 2026
Merged

capnweb-validate: ship typescript as a capped dependency#240
teamchong merged 1 commit into
cloudflare:mainfrom
Maximo-Guk:validate-own-compiler

Conversation

@Maximo-Guk

@Maximo-GukMaximo-Guk commented Aug 12, 2026

Copy link
Copy Markdown
Member

capnweb-validate's build-time transform needs the JS compiler API (transpileModule, createProgram, ts.sys), which TypeScript 7 (tsgo) no longer ships. The current peer is uncapped (>=5.7.0), so in a workspace whose typescript is 7.x the transform resolves a metadata-only package and dies at build time (ts.sys / ts.findConfigFile undefined). Working around it consumer-side takes a .pnpmfile.cjs hook, because pnpm resolves peers from the importer's context and neither overrides nor packageExtensions can move one.

This swaps the peer for a real dependency capped at <7: under a TS 7 workspace the transform gets its own JS-based compiler (transform compiler and check compiler are independent - checking with tsgo still works fine), and consumers on 5.7–6.x dedupe to their existing install. The 6.0.3 compiler is validated against the transform in a real consumer.

@changeset-bot

changeset-botBot commented Aug 12, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: af74f4d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
NameType
capnweb-validatePatch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new

pkg-pr-newBot commented Aug 12, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/cloudflare/capnweb@240

commit: af74f4d

The transform needs the JS compiler API, which TypeScript 7 (tsgo)
no longer ships. The uncapped peer (>=5.7.0) resolved to whatever the
consumer's workspace has, so under a TS 7 workspace the transform got
a metadata-only package and crashed at build time; consumers had to
work around it with a .pnpmfile.cjs rewriting the peer.
A real dependency capped at <7 gives the transform its own JS-based
compiler under TS 7 workspaces (transform compiler and check compiler
are independent), and dedupes against the consumer's install on
5.7-6.x. The lockfile diff is hand-minimized because npm 11.9 strips
the libc fields a newer npm wrote; npm ci validates it.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Maximo-Guk
Maximo-Gukforce-pushed the validate-own-compiler branch from 380aa93 to af74f4dCompareAugust 12, 2026 19:40
@Maximo-Guk
Maximo-Guk marked this pull request as ready for review August 12, 2026 19:42
@teamchong

Copy link
Copy Markdown
Collaborator

/bonk review this

@ask-bonkask-bonkBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: ship typescript as a capped dependency

The change is sound and correctly targeted. The root cause analysis is accurate: capnweb-validate's transform imports the typescript compiler API at build time (import ts from "typescript" in src/transform/{run,context,transform-module}.ts), and those entry points call ts.sys / ts.findConfigFile / transpileModule / createProgram — none of which exist in the metadata-only TS 7 (tsgo) package. Moving typescript from an uncapped peer to a >=5.7.0 <7 direct dependency guarantees a real JS compiler is present regardless of the workspace's TS version, and consumers on 5.7–6.x will dedupe.

Consistency check (all good)

  • package.json and the lockfile agree: dependencies.typescript = ">=5.7.0 <7", peer removed, devDep removed.
  • Resolved lockfile version 5.9.3 satisfies the new range and is no longer marked dev.
  • Root workspace still declares typescript: ^5.9.3, so in-repo build/test are unaffected.
  • Changeset wording matches the actual range.

Points worth confirming before merge

  1. Trade-off is a real duplicate install for TS 7 consumers. Under a TS 7 workspace, <7 forces a second, JS-based TypeScript (6.x) to be installed alongside the consumer's tsgo. That's the intended behavior and is stated in the PR body, but it's worth flagging explicitly for reviewers: this is a deliberate on-disk cost, not a bug.

  2. import type ts from "typescript" in type-introspector.ts is type-only and would have been erased — so the runtime need is genuinely the three value-importing modules. No concern, just confirming the peer->dep move is necessary rather than cosmetic.

  3. Making typescript a hard dependencies rather than an optional peer changes install semantics for all consumers, including any who only use the runtime ./internal exports and never the build-time transform. Those consumers will now always pull a TypeScript copy. If there is a runtime-only consumption path that never touches the compiler, consider whether typescript could instead be a capped peer (>=5.7.0 <7) so it stays out of pure-runtime installs. If the package is only ever consumed as a build-time transform, the direct dependency is the right call and this is a non-issue.

Nothing blocking from a correctness standpoint. Items 1 and 3 are design confirmations rather than defects.

@ask-bonk

ask-bonkBot commented Aug 12, 2026

Copy link
Copy Markdown

Review posted successfully. Summary of my assessment of PR #240:

Verdict: sound and correctly targeted, no blocking issues.

The change moves typescript from an uncapped peer dependency to a capped direct dependency (>=5.7.0 <7) on capnweb-validate.

What I verified:

  • The root cause is real — the transform value-imports the TS compiler API (ts.sys, findConfigFile, transpileModule, createProgram) in src/transform/{run,context,transform-module}.ts, which TS 7 (tsgo) doesn't ship.
  • package.json and package-lock.json are internally consistent (dep added, peer removed, devDep removed, dev: true flag dropped, resolved 5.9.3 satisfies the range).
  • Root workspace still has typescript: ^5.9.3, so in-repo build/test are unaffected.
  • Changeset wording matches the actual range.

Points I flagged for the maintainer (design confirmations, not defects):

  1. The <7 cap deliberately forces a duplicate JS-based TS install for TS 7 consumers — intended, but worth noting.
  2. type-introspector.ts uses a type-only import, confirming the runtime need is genuine.
  3. A hard dependencies entry means even runtime-only (./internal) consumers now always pull TypeScript; if such a path exists, a capped peer might be preferable — otherwise the direct dependency is correct.

github run

@teamchong
teamchong merged commit f7f7fa8 into cloudflare:mainAug 12, 2026
5 checks passed
@github-actionsgithub-actionsBot mentioned this pull request Aug 12, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@Maximo-Guk@teamchong
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

capnweb-validate: ship typescript as a capped dependency - #240

Merged
teamchong merged 1 commit into
cloudflare:mainfrom
Maximo-Guk:validate-own-compiler
Aug 12, 2026
Merged

capnweb-validate: ship typescript as a capped dependency#240
teamchong merged 1 commit into
cloudflare:mainfrom
Maximo-Guk:validate-own-compiler

Conversation

@Maximo-Guk

@Maximo-GukMaximo-Guk commented Aug 12, 2026

Copy link
Copy Markdown
Member

capnweb-validate's build-time transform needs the JS compiler API (transpileModule, createProgram, ts.sys), which TypeScript 7 (tsgo) no longer ships. The current peer is uncapped (>=5.7.0), so in a workspace whose typescript is 7.x the transform resolves a metadata-only package and dies at build time (ts.sys / ts.findConfigFile undefined). Working around it consumer-side takes a .pnpmfile.cjs hook, because pnpm resolves peers from the importer's context and neither overrides nor packageExtensions can move one.

This swaps the peer for a real dependency capped at <7: under a TS 7 workspace the transform gets its own JS-based compiler (transform compiler and check compiler are independent - checking with tsgo still works fine), and consumers on 5.7–6.x dedupe to their existing install. The 6.0.3 compiler is validated against the transform in a real consumer.

@changeset-bot

changeset-botBot commented Aug 12, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: af74f4d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
NameType
capnweb-validatePatch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new

pkg-pr-newBot commented Aug 12, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/cloudflare/capnweb@240

commit: af74f4d

The transform needs the JS compiler API, which TypeScript 7 (tsgo)
no longer ships. The uncapped peer (>=5.7.0) resolved to whatever the
consumer's workspace has, so under a TS 7 workspace the transform got
a metadata-only package and crashed at build time; consumers had to
work around it with a .pnpmfile.cjs rewriting the peer.
A real dependency capped at <7 gives the transform its own JS-based
compiler under TS 7 workspaces (transform compiler and check compiler
are independent), and dedupes against the consumer's install on
5.7-6.x. The lockfile diff is hand-minimized because npm 11.9 strips
the libc fields a newer npm wrote; npm ci validates it.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Maximo-Guk
Maximo-Gukforce-pushed the validate-own-compiler branch from 380aa93 to af74f4dCompareAugust 12, 2026 19:40
@Maximo-Guk
Maximo-Guk marked this pull request as ready for review August 12, 2026 19:42
@teamchong

Copy link
Copy Markdown
Collaborator

/bonk review this

@ask-bonkask-bonkBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: ship typescript as a capped dependency

The change is sound and correctly targeted. The root cause analysis is accurate: capnweb-validate's transform imports the typescript compiler API at build time (import ts from "typescript" in src/transform/{run,context,transform-module}.ts), and those entry points call ts.sys / ts.findConfigFile / transpileModule / createProgram — none of which exist in the metadata-only TS 7 (tsgo) package. Moving typescript from an uncapped peer to a >=5.7.0 <7 direct dependency guarantees a real JS compiler is present regardless of the workspace's TS version, and consumers on 5.7–6.x will dedupe.

Consistency check (all good)

  • package.json and the lockfile agree: dependencies.typescript = ">=5.7.0 <7", peer removed, devDep removed.
  • Resolved lockfile version 5.9.3 satisfies the new range and is no longer marked dev.
  • Root workspace still declares typescript: ^5.9.3, so in-repo build/test are unaffected.
  • Changeset wording matches the actual range.

Points worth confirming before merge

  1. Trade-off is a real duplicate install for TS 7 consumers. Under a TS 7 workspace, <7 forces a second, JS-based TypeScript (6.x) to be installed alongside the consumer's tsgo. That's the intended behavior and is stated in the PR body, but it's worth flagging explicitly for reviewers: this is a deliberate on-disk cost, not a bug.

  2. import type ts from "typescript" in type-introspector.ts is type-only and would have been erased — so the runtime need is genuinely the three value-importing modules. No concern, just confirming the peer->dep move is necessary rather than cosmetic.

  3. Making typescript a hard dependencies rather than an optional peer changes install semantics for all consumers, including any who only use the runtime ./internal exports and never the build-time transform. Those consumers will now always pull a TypeScript copy. If there is a runtime-only consumption path that never touches the compiler, consider whether typescript could instead be a capped peer (>=5.7.0 <7) so it stays out of pure-runtime installs. If the package is only ever consumed as a build-time transform, the direct dependency is the right call and this is a non-issue.

Nothing blocking from a correctness standpoint. Items 1 and 3 are design confirmations rather than defects.

@ask-bonk

ask-bonkBot commented Aug 12, 2026

Copy link
Copy Markdown

Review posted successfully. Summary of my assessment of PR #240:

Verdict: sound and correctly targeted, no blocking issues.

The change moves typescript from an uncapped peer dependency to a capped direct dependency (>=5.7.0 <7) on capnweb-validate.

What I verified:

  • The root cause is real — the transform value-imports the TS compiler API (ts.sys, findConfigFile, transpileModule, createProgram) in src/transform/{run,context,transform-module}.ts, which TS 7 (tsgo) doesn't ship.
  • package.json and package-lock.json are internally consistent (dep added, peer removed, devDep removed, dev: true flag dropped, resolved 5.9.3 satisfies the range).
  • Root workspace still has typescript: ^5.9.3, so in-repo build/test are unaffected.
  • Changeset wording matches the actual range.

Points I flagged for the maintainer (design confirmations, not defects):

  1. The <7 cap deliberately forces a duplicate JS-based TS install for TS 7 consumers — intended, but worth noting.
  2. type-introspector.ts uses a type-only import, confirming the runtime need is genuine.
  3. A hard dependencies entry means even runtime-only (./internal) consumers now always pull TypeScript; if such a path exists, a capped peer might be preferable — otherwise the direct dependency is correct.

github run

@teamchong
teamchong merged commit f7f7fa8 into cloudflare:mainAug 12, 2026
5 checks passed
@github-actionsgithub-actionsBot mentioned this pull request Aug 12, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@Maximo-Guk@teamchong
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

capnweb-validate: ship typescript as a capped dependency - #240

Merged
teamchong merged 1 commit into
cloudflare:mainfrom
Maximo-Guk:validate-own-compiler
Aug 12, 2026
Merged

capnweb-validate: ship typescript as a capped dependency#240
teamchong merged 1 commit into
cloudflare:mainfrom
Maximo-Guk:validate-own-compiler

Conversation

@Maximo-Guk

@Maximo-GukMaximo-Guk commented Aug 12, 2026

Copy link
Copy Markdown
Member

capnweb-validate's build-time transform needs the JS compiler API (transpileModule, createProgram, ts.sys), which TypeScript 7 (tsgo) no longer ships. The current peer is uncapped (>=5.7.0), so in a workspace whose typescript is 7.x the transform resolves a metadata-only package and dies at build time (ts.sys / ts.findConfigFile undefined). Working around it consumer-side takes a .pnpmfile.cjs hook, because pnpm resolves peers from the importer's context and neither overrides nor packageExtensions can move one.

This swaps the peer for a real dependency capped at <7: under a TS 7 workspace the transform gets its own JS-based compiler (transform compiler and check compiler are independent - checking with tsgo still works fine), and consumers on 5.7–6.x dedupe to their existing install. The 6.0.3 compiler is validated against the transform in a real consumer.

@changeset-bot

changeset-botBot commented Aug 12, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: af74f4d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
NameType
capnweb-validatePatch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new

pkg-pr-newBot commented Aug 12, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/cloudflare/capnweb@240

commit: af74f4d

The transform needs the JS compiler API, which TypeScript 7 (tsgo)
no longer ships. The uncapped peer (>=5.7.0) resolved to whatever the
consumer's workspace has, so under a TS 7 workspace the transform got
a metadata-only package and crashed at build time; consumers had to
work around it with a .pnpmfile.cjs rewriting the peer.
A real dependency capped at <7 gives the transform its own JS-based
compiler under TS 7 workspaces (transform compiler and check compiler
are independent), and dedupes against the consumer's install on
5.7-6.x. The lockfile diff is hand-minimized because npm 11.9 strips
the libc fields a newer npm wrote; npm ci validates it.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Maximo-Guk
Maximo-Gukforce-pushed the validate-own-compiler branch from 380aa93 to af74f4dCompareAugust 12, 2026 19:40
@Maximo-Guk
Maximo-Guk marked this pull request as ready for review August 12, 2026 19:42
@teamchong

Copy link
Copy Markdown
Collaborator

/bonk review this

@ask-bonkask-bonkBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: ship typescript as a capped dependency

The change is sound and correctly targeted. The root cause analysis is accurate: capnweb-validate's transform imports the typescript compiler API at build time (import ts from "typescript" in src/transform/{run,context,transform-module}.ts), and those entry points call ts.sys / ts.findConfigFile / transpileModule / createProgram — none of which exist in the metadata-only TS 7 (tsgo) package. Moving typescript from an uncapped peer to a >=5.7.0 <7 direct dependency guarantees a real JS compiler is present regardless of the workspace's TS version, and consumers on 5.7–6.x will dedupe.

Consistency check (all good)

  • package.json and the lockfile agree: dependencies.typescript = ">=5.7.0 <7", peer removed, devDep removed.
  • Resolved lockfile version 5.9.3 satisfies the new range and is no longer marked dev.
  • Root workspace still declares typescript: ^5.9.3, so in-repo build/test are unaffected.
  • Changeset wording matches the actual range.

Points worth confirming before merge

  1. Trade-off is a real duplicate install for TS 7 consumers. Under a TS 7 workspace, <7 forces a second, JS-based TypeScript (6.x) to be installed alongside the consumer's tsgo. That's the intended behavior and is stated in the PR body, but it's worth flagging explicitly for reviewers: this is a deliberate on-disk cost, not a bug.

  2. import type ts from "typescript" in type-introspector.ts is type-only and would have been erased — so the runtime need is genuinely the three value-importing modules. No concern, just confirming the peer->dep move is necessary rather than cosmetic.

  3. Making typescript a hard dependencies rather than an optional peer changes install semantics for all consumers, including any who only use the runtime ./internal exports and never the build-time transform. Those consumers will now always pull a TypeScript copy. If there is a runtime-only consumption path that never touches the compiler, consider whether typescript could instead be a capped peer (>=5.7.0 <7) so it stays out of pure-runtime installs. If the package is only ever consumed as a build-time transform, the direct dependency is the right call and this is a non-issue.

Nothing blocking from a correctness standpoint. Items 1 and 3 are design confirmations rather than defects.

@ask-bonk

ask-bonkBot commented Aug 12, 2026

Copy link
Copy Markdown

Review posted successfully. Summary of my assessment of PR #240:

Verdict: sound and correctly targeted, no blocking issues.

The change moves typescript from an uncapped peer dependency to a capped direct dependency (>=5.7.0 <7) on capnweb-validate.

What I verified:

  • The root cause is real — the transform value-imports the TS compiler API (ts.sys, findConfigFile, transpileModule, createProgram) in src/transform/{run,context,transform-module}.ts, which TS 7 (tsgo) doesn't ship.
  • package.json and package-lock.json are internally consistent (dep added, peer removed, devDep removed, dev: true flag dropped, resolved 5.9.3 satisfies the range).
  • Root workspace still has typescript: ^5.9.3, so in-repo build/test are unaffected.
  • Changeset wording matches the actual range.

Points I flagged for the maintainer (design confirmations, not defects):

  1. The <7 cap deliberately forces a duplicate JS-based TS install for TS 7 consumers — intended, but worth noting.
  2. type-introspector.ts uses a type-only import, confirming the runtime need is genuine.
  3. A hard dependencies entry means even runtime-only (./internal) consumers now always pull TypeScript; if such a path exists, a capped peer might be preferable — otherwise the direct dependency is correct.

github run

@teamchong
teamchong merged commit f7f7fa8 into cloudflare:mainAug 12, 2026
5 checks passed
@github-actionsgithub-actionsBot mentioned this pull request Aug 12, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@Maximo-Guk@teamchong
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

capnweb-validate: ship typescript as a capped dependency - #240

Merged
teamchong merged 1 commit into
cloudflare:mainfrom
Maximo-Guk:validate-own-compiler
Aug 12, 2026
Merged

capnweb-validate: ship typescript as a capped dependency#240
teamchong merged 1 commit into
cloudflare:mainfrom
Maximo-Guk:validate-own-compiler

Conversation

@Maximo-Guk

@Maximo-GukMaximo-Guk commented Aug 12, 2026

Copy link
Copy Markdown
Member

capnweb-validate's build-time transform needs the JS compiler API (transpileModule, createProgram, ts.sys), which TypeScript 7 (tsgo) no longer ships. The current peer is uncapped (>=5.7.0), so in a workspace whose typescript is 7.x the transform resolves a metadata-only package and dies at build time (ts.sys / ts.findConfigFile undefined). Working around it consumer-side takes a .pnpmfile.cjs hook, because pnpm resolves peers from the importer's context and neither overrides nor packageExtensions can move one.

This swaps the peer for a real dependency capped at <7: under a TS 7 workspace the transform gets its own JS-based compiler (transform compiler and check compiler are independent - checking with tsgo still works fine), and consumers on 5.7–6.x dedupe to their existing install. The 6.0.3 compiler is validated against the transform in a real consumer.

@changeset-bot

changeset-botBot commented Aug 12, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: af74f4d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
NameType
capnweb-validatePatch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new

pkg-pr-newBot commented Aug 12, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/cloudflare/capnweb@240

commit: af74f4d

The transform needs the JS compiler API, which TypeScript 7 (tsgo)
no longer ships. The uncapped peer (>=5.7.0) resolved to whatever the
consumer's workspace has, so under a TS 7 workspace the transform got
a metadata-only package and crashed at build time; consumers had to
work around it with a .pnpmfile.cjs rewriting the peer.
A real dependency capped at <7 gives the transform its own JS-based
compiler under TS 7 workspaces (transform compiler and check compiler
are independent), and dedupes against the consumer's install on
5.7-6.x. The lockfile diff is hand-minimized because npm 11.9 strips
the libc fields a newer npm wrote; npm ci validates it.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Maximo-Guk
Maximo-Gukforce-pushed the validate-own-compiler branch from 380aa93 to af74f4dCompareAugust 12, 2026 19:40
@Maximo-Guk
Maximo-Guk marked this pull request as ready for review August 12, 2026 19:42
@teamchong

Copy link
Copy Markdown
Collaborator

/bonk review this

@ask-bonkask-bonkBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: ship typescript as a capped dependency

The change is sound and correctly targeted. The root cause analysis is accurate: capnweb-validate's transform imports the typescript compiler API at build time (import ts from "typescript" in src/transform/{run,context,transform-module}.ts), and those entry points call ts.sys / ts.findConfigFile / transpileModule / createProgram — none of which exist in the metadata-only TS 7 (tsgo) package. Moving typescript from an uncapped peer to a >=5.7.0 <7 direct dependency guarantees a real JS compiler is present regardless of the workspace's TS version, and consumers on 5.7–6.x will dedupe.

Consistency check (all good)

  • package.json and the lockfile agree: dependencies.typescript = ">=5.7.0 <7", peer removed, devDep removed.
  • Resolved lockfile version 5.9.3 satisfies the new range and is no longer marked dev.
  • Root workspace still declares typescript: ^5.9.3, so in-repo build/test are unaffected.
  • Changeset wording matches the actual range.

Points worth confirming before merge

  1. Trade-off is a real duplicate install for TS 7 consumers. Under a TS 7 workspace, <7 forces a second, JS-based TypeScript (6.x) to be installed alongside the consumer's tsgo. That's the intended behavior and is stated in the PR body, but it's worth flagging explicitly for reviewers: this is a deliberate on-disk cost, not a bug.

  2. import type ts from "typescript" in type-introspector.ts is type-only and would have been erased — so the runtime need is genuinely the three value-importing modules. No concern, just confirming the peer->dep move is necessary rather than cosmetic.

  3. Making typescript a hard dependencies rather than an optional peer changes install semantics for all consumers, including any who only use the runtime ./internal exports and never the build-time transform. Those consumers will now always pull a TypeScript copy. If there is a runtime-only consumption path that never touches the compiler, consider whether typescript could instead be a capped peer (>=5.7.0 <7) so it stays out of pure-runtime installs. If the package is only ever consumed as a build-time transform, the direct dependency is the right call and this is a non-issue.

Nothing blocking from a correctness standpoint. Items 1 and 3 are design confirmations rather than defects.

@ask-bonk

ask-bonkBot commented Aug 12, 2026

Copy link
Copy Markdown

Review posted successfully. Summary of my assessment of PR #240:

Verdict: sound and correctly targeted, no blocking issues.

The change moves typescript from an uncapped peer dependency to a capped direct dependency (>=5.7.0 <7) on capnweb-validate.

What I verified:

  • The root cause is real — the transform value-imports the TS compiler API (ts.sys, findConfigFile, transpileModule, createProgram) in src/transform/{run,context,transform-module}.ts, which TS 7 (tsgo) doesn't ship.
  • package.json and package-lock.json are internally consistent (dep added, peer removed, devDep removed, dev: true flag dropped, resolved 5.9.3 satisfies the range).
  • Root workspace still has typescript: ^5.9.3, so in-repo build/test are unaffected.
  • Changeset wording matches the actual range.

Points I flagged for the maintainer (design confirmations, not defects):

  1. The <7 cap deliberately forces a duplicate JS-based TS install for TS 7 consumers — intended, but worth noting.
  2. type-introspector.ts uses a type-only import, confirming the runtime need is genuine.
  3. A hard dependencies entry means even runtime-only (./internal) consumers now always pull TypeScript; if such a path exists, a capped peer might be preferable — otherwise the direct dependency is correct.

github run

@teamchong
teamchong merged commit f7f7fa8 into cloudflare:mainAug 12, 2026
5 checks passed
@github-actionsgithub-actionsBot mentioned this pull request Aug 12, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@Maximo-Guk@teamchong
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

capnweb-validate: ship typescript as a capped dependency - #240

Merged
teamchong merged 1 commit into
cloudflare:mainfrom
Maximo-Guk:validate-own-compiler
Aug 12, 2026
Merged

capnweb-validate: ship typescript as a capped dependency#240
teamchong merged 1 commit into
cloudflare:mainfrom
Maximo-Guk:validate-own-compiler

Conversation

@Maximo-Guk

@Maximo-GukMaximo-Guk commented Aug 12, 2026

Copy link
Copy Markdown
Member

capnweb-validate's build-time transform needs the JS compiler API (transpileModule, createProgram, ts.sys), which TypeScript 7 (tsgo) no longer ships. The current peer is uncapped (>=5.7.0), so in a workspace whose typescript is 7.x the transform resolves a metadata-only package and dies at build time (ts.sys / ts.findConfigFile undefined). Working around it consumer-side takes a .pnpmfile.cjs hook, because pnpm resolves peers from the importer's context and neither overrides nor packageExtensions can move one.

This swaps the peer for a real dependency capped at <7: under a TS 7 workspace the transform gets its own JS-based compiler (transform compiler and check compiler are independent - checking with tsgo still works fine), and consumers on 5.7–6.x dedupe to their existing install. The 6.0.3 compiler is validated against the transform in a real consumer.

@changeset-bot

changeset-botBot commented Aug 12, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: af74f4d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
NameType
capnweb-validatePatch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new

pkg-pr-newBot commented Aug 12, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/cloudflare/capnweb@240

commit: af74f4d

The transform needs the JS compiler API, which TypeScript 7 (tsgo)
no longer ships. The uncapped peer (>=5.7.0) resolved to whatever the
consumer's workspace has, so under a TS 7 workspace the transform got
a metadata-only package and crashed at build time; consumers had to
work around it with a .pnpmfile.cjs rewriting the peer.
A real dependency capped at <7 gives the transform its own JS-based
compiler under TS 7 workspaces (transform compiler and check compiler
are independent), and dedupes against the consumer's install on
5.7-6.x. The lockfile diff is hand-minimized because npm 11.9 strips
the libc fields a newer npm wrote; npm ci validates it.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Maximo-Guk
Maximo-Gukforce-pushed the validate-own-compiler branch from 380aa93 to af74f4dCompareAugust 12, 2026 19:40
@Maximo-Guk
Maximo-Guk marked this pull request as ready for review August 12, 2026 19:42
@teamchong

Copy link
Copy Markdown
Collaborator

/bonk review this

@ask-bonkask-bonkBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: ship typescript as a capped dependency

The change is sound and correctly targeted. The root cause analysis is accurate: capnweb-validate's transform imports the typescript compiler API at build time (import ts from "typescript" in src/transform/{run,context,transform-module}.ts), and those entry points call ts.sys / ts.findConfigFile / transpileModule / createProgram — none of which exist in the metadata-only TS 7 (tsgo) package. Moving typescript from an uncapped peer to a >=5.7.0 <7 direct dependency guarantees a real JS compiler is present regardless of the workspace's TS version, and consumers on 5.7–6.x will dedupe.

Consistency check (all good)

  • package.json and the lockfile agree: dependencies.typescript = ">=5.7.0 <7", peer removed, devDep removed.
  • Resolved lockfile version 5.9.3 satisfies the new range and is no longer marked dev.
  • Root workspace still declares typescript: ^5.9.3, so in-repo build/test are unaffected.
  • Changeset wording matches the actual range.

Points worth confirming before merge

  1. Trade-off is a real duplicate install for TS 7 consumers. Under a TS 7 workspace, <7 forces a second, JS-based TypeScript (6.x) to be installed alongside the consumer's tsgo. That's the intended behavior and is stated in the PR body, but it's worth flagging explicitly for reviewers: this is a deliberate on-disk cost, not a bug.

  2. import type ts from "typescript" in type-introspector.ts is type-only and would have been erased — so the runtime need is genuinely the three value-importing modules. No concern, just confirming the peer->dep move is necessary rather than cosmetic.

  3. Making typescript a hard dependencies rather than an optional peer changes install semantics for all consumers, including any who only use the runtime ./internal exports and never the build-time transform. Those consumers will now always pull a TypeScript copy. If there is a runtime-only consumption path that never touches the compiler, consider whether typescript could instead be a capped peer (>=5.7.0 <7) so it stays out of pure-runtime installs. If the package is only ever consumed as a build-time transform, the direct dependency is the right call and this is a non-issue.

Nothing blocking from a correctness standpoint. Items 1 and 3 are design confirmations rather than defects.

@ask-bonk

ask-bonkBot commented Aug 12, 2026

Copy link
Copy Markdown

Review posted successfully. Summary of my assessment of PR #240:

Verdict: sound and correctly targeted, no blocking issues.

The change moves typescript from an uncapped peer dependency to a capped direct dependency (>=5.7.0 <7) on capnweb-validate.

What I verified:

  • The root cause is real — the transform value-imports the TS compiler API (ts.sys, findConfigFile, transpileModule, createProgram) in src/transform/{run,context,transform-module}.ts, which TS 7 (tsgo) doesn't ship.
  • package.json and package-lock.json are internally consistent (dep added, peer removed, devDep removed, dev: true flag dropped, resolved 5.9.3 satisfies the range).
  • Root workspace still has typescript: ^5.9.3, so in-repo build/test are unaffected.
  • Changeset wording matches the actual range.

Points I flagged for the maintainer (design confirmations, not defects):

  1. The <7 cap deliberately forces a duplicate JS-based TS install for TS 7 consumers — intended, but worth noting.
  2. type-introspector.ts uses a type-only import, confirming the runtime need is genuine.
  3. A hard dependencies entry means even runtime-only (./internal) consumers now always pull TypeScript; if such a path exists, a capped peer might be preferable — otherwise the direct dependency is correct.

github run

@teamchong
teamchong merged commit f7f7fa8 into cloudflare:mainAug 12, 2026
5 checks passed
@github-actionsgithub-actionsBot mentioned this pull request Aug 12, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@Maximo-Guk@teamchong
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

capnweb-validate: ship typescript as a capped dependency - #240

Merged
teamchong merged 1 commit into
cloudflare:mainfrom
Maximo-Guk:validate-own-compiler
Aug 12, 2026
Merged

capnweb-validate: ship typescript as a capped dependency#240
teamchong merged 1 commit into
cloudflare:mainfrom
Maximo-Guk:validate-own-compiler

Conversation

@Maximo-Guk

@Maximo-GukMaximo-Guk commented Aug 12, 2026

Copy link
Copy Markdown
Member

capnweb-validate's build-time transform needs the JS compiler API (transpileModule, createProgram, ts.sys), which TypeScript 7 (tsgo) no longer ships. The current peer is uncapped (>=5.7.0), so in a workspace whose typescript is 7.x the transform resolves a metadata-only package and dies at build time (ts.sys / ts.findConfigFile undefined). Working around it consumer-side takes a .pnpmfile.cjs hook, because pnpm resolves peers from the importer's context and neither overrides nor packageExtensions can move one.

This swaps the peer for a real dependency capped at <7: under a TS 7 workspace the transform gets its own JS-based compiler (transform compiler and check compiler are independent - checking with tsgo still works fine), and consumers on 5.7–6.x dedupe to their existing install. The 6.0.3 compiler is validated against the transform in a real consumer.

@changeset-bot

changeset-botBot commented Aug 12, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: af74f4d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
NameType
capnweb-validatePatch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new

pkg-pr-newBot commented Aug 12, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/cloudflare/capnweb@240

commit: af74f4d

The transform needs the JS compiler API, which TypeScript 7 (tsgo)
no longer ships. The uncapped peer (>=5.7.0) resolved to whatever the
consumer's workspace has, so under a TS 7 workspace the transform got
a metadata-only package and crashed at build time; consumers had to
work around it with a .pnpmfile.cjs rewriting the peer.
A real dependency capped at <7 gives the transform its own JS-based
compiler under TS 7 workspaces (transform compiler and check compiler
are independent), and dedupes against the consumer's install on
5.7-6.x. The lockfile diff is hand-minimized because npm 11.9 strips
the libc fields a newer npm wrote; npm ci validates it.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Maximo-Guk
Maximo-Gukforce-pushed the validate-own-compiler branch from 380aa93 to af74f4dCompareAugust 12, 2026 19:40
@Maximo-Guk
Maximo-Guk marked this pull request as ready for review August 12, 2026 19:42
@teamchong

Copy link
Copy Markdown
Collaborator

/bonk review this

@ask-bonkask-bonkBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: ship typescript as a capped dependency

The change is sound and correctly targeted. The root cause analysis is accurate: capnweb-validate's transform imports the typescript compiler API at build time (import ts from "typescript" in src/transform/{run,context,transform-module}.ts), and those entry points call ts.sys / ts.findConfigFile / transpileModule / createProgram — none of which exist in the metadata-only TS 7 (tsgo) package. Moving typescript from an uncapped peer to a >=5.7.0 <7 direct dependency guarantees a real JS compiler is present regardless of the workspace's TS version, and consumers on 5.7–6.x will dedupe.

Consistency check (all good)

  • package.json and the lockfile agree: dependencies.typescript = ">=5.7.0 <7", peer removed, devDep removed.
  • Resolved lockfile version 5.9.3 satisfies the new range and is no longer marked dev.
  • Root workspace still declares typescript: ^5.9.3, so in-repo build/test are unaffected.
  • Changeset wording matches the actual range.

Points worth confirming before merge

  1. Trade-off is a real duplicate install for TS 7 consumers. Under a TS 7 workspace, <7 forces a second, JS-based TypeScript (6.x) to be installed alongside the consumer's tsgo. That's the intended behavior and is stated in the PR body, but it's worth flagging explicitly for reviewers: this is a deliberate on-disk cost, not a bug.

  2. import type ts from "typescript" in type-introspector.ts is type-only and would have been erased — so the runtime need is genuinely the three value-importing modules. No concern, just confirming the peer->dep move is necessary rather than cosmetic.

  3. Making typescript a hard dependencies rather than an optional peer changes install semantics for all consumers, including any who only use the runtime ./internal exports and never the build-time transform. Those consumers will now always pull a TypeScript copy. If there is a runtime-only consumption path that never touches the compiler, consider whether typescript could instead be a capped peer (>=5.7.0 <7) so it stays out of pure-runtime installs. If the package is only ever consumed as a build-time transform, the direct dependency is the right call and this is a non-issue.

Nothing blocking from a correctness standpoint. Items 1 and 3 are design confirmations rather than defects.

@ask-bonk

ask-bonkBot commented Aug 12, 2026

Copy link
Copy Markdown

Review posted successfully. Summary of my assessment of PR #240:

Verdict: sound and correctly targeted, no blocking issues.

The change moves typescript from an uncapped peer dependency to a capped direct dependency (>=5.7.0 <7) on capnweb-validate.

What I verified:

  • The root cause is real — the transform value-imports the TS compiler API (ts.sys, findConfigFile, transpileModule, createProgram) in src/transform/{run,context,transform-module}.ts, which TS 7 (tsgo) doesn't ship.
  • package.json and package-lock.json are internally consistent (dep added, peer removed, devDep removed, dev: true flag dropped, resolved 5.9.3 satisfies the range).
  • Root workspace still has typescript: ^5.9.3, so in-repo build/test are unaffected.
  • Changeset wording matches the actual range.

Points I flagged for the maintainer (design confirmations, not defects):

  1. The <7 cap deliberately forces a duplicate JS-based TS install for TS 7 consumers — intended, but worth noting.
  2. type-introspector.ts uses a type-only import, confirming the runtime need is genuine.
  3. A hard dependencies entry means even runtime-only (./internal) consumers now always pull TypeScript; if such a path exists, a capped peer might be preferable — otherwise the direct dependency is correct.

github run

@teamchong
teamchong merged commit f7f7fa8 into cloudflare:mainAug 12, 2026
5 checks passed
@github-actionsgithub-actionsBot mentioned this pull request Aug 12, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@Maximo-Guk@teamchong