fix(compression): lazy-load typescript in RTK codeStripper so prod-lean deploys don't break (#7096) (#7164)

Co-authored-by: Diego Rodrigues de Sa e Souza <diegosouza.pw@gmail.com>
Co-authored-by: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com>
This commit is contained in:
Ronaldo Davi
2026-07-17 02:39:40 -03:00
committed by GitHub
parent 197f726c62
commit 7fcfbcd8fd
2 changed files with 145 additions and 4 deletions

View File

@@ -1,4 +1,57 @@
import ts from "typescript";
import { createRequire } from "node:module";
// Type-only import: erased at build time, so it never forces the `typescript`
// package to be present at runtime. The value handle is resolved lazily below.
import type * as TypeScriptApi from "typescript";
type TypeScriptModule = typeof import("typescript");
// `typescript` is a devDependency used only for opt-in AST-based comment
// stripping. A production-lean deploy (`npm run build && npm prune --omit=dev`,
// recommended in Discussion #6956) removes it, so importing it eagerly at module
// top level broke *every* Compression Context page (#7096). Resolve it lazily on
// first use and degrade to a no-op when it is unavailable.
let typeScriptModule: TypeScriptModule | null | undefined;
let warnedMissingTypeScript = false;
let loadTypeScriptModule: () => TypeScriptModule | null = defaultLoadTypeScriptModule;
function defaultLoadTypeScriptModule(): TypeScriptModule | null {
try {
const requireFromHere = createRequire(import.meta.url);
return requireFromHere("typescript") as TypeScriptModule;
} catch {
return null;
}
}
function resolveTypeScript(): TypeScriptModule | null {
if (typeScriptModule === undefined) {
typeScriptModule = loadTypeScriptModule();
if (!typeScriptModule && !warnedMissingTypeScript) {
warnedMissingTypeScript = true;
// One-time warning: compression still works, just without AST-based
// code-comment stripping (which is opt-in and off by default anyway).
console.warn(
"[compression/rtk] optional dependency 'typescript' is not installed; " +
"skipping AST-based code-comment stripping (compression still works). " +
"Install 'typescript' to re-enable it."
);
}
}
return typeScriptModule;
}
/**
* @internal Test seam — override the lazy TypeScript loader (pass `null` to
* restore the default) and reset the cache so graceful degradation can be
* exercised without uninstalling the package. Not part of the public API.
*/
export function __setTypeScriptModuleLoaderForTests(
loader: (() => TypeScriptModule | null) | null
): void {
loadTypeScriptModule = loader ?? defaultLoadTypeScriptModule;
typeScriptModule = undefined;
warnedMissingTypeScript = false;
}
export type CodeLanguage =
| "javascript"
@@ -60,6 +113,12 @@ export function detectCodeLanguage(text: string): CodeLanguage {
* JSX expression-container comments are never corrupted.
*/
function stripJsTsComments(text: string, preserveDocstrings: boolean): string {
const ts = resolveTypeScript();
// Graceful degradation: when `typescript` is unavailable (e.g. after
// `npm prune --omit=dev`), skip AST-based comment stripping and leave the
// code untouched rather than crashing (#7096).
if (!ts) return text;
const source = ts.createSourceFile(
"snippet.tsx",
text,
@@ -69,7 +128,7 @@ function stripJsTsComments(text: string, preserveDocstrings: boolean): string {
);
let hasJsx = false;
const detectJsx = (node: ts.Node): void => {
const detectJsx = (node: TypeScriptApi.Node): void => {
if (ts.isJsxElement(node) || ts.isJsxSelfClosingElement(node) || ts.isJsxFragment(node)) {
hasJsx = true;
return;
@@ -79,8 +138,8 @@ function stripJsTsComments(text: string, preserveDocstrings: boolean): string {
detectJsx(source);
if (hasJsx) return text;
const ranges = new Map<number, ts.CommentRange>();
const collect = (node: ts.Node): void => {
const ranges = new Map<number, TypeScriptApi.CommentRange>();
const collect = (node: TypeScriptApi.Node): void => {
for (const range of ts.getLeadingCommentRanges(text, node.getFullStart()) ?? []) {
ranges.set(range.pos, range);
}

View File

@@ -0,0 +1,82 @@
/**
* Regression for #7096 — the RTK code stripper imported the `typescript`
* package eagerly at module top level (`import ts from "typescript"`), but
* `typescript` lives in devDependencies. After a production-lean deploy
* (`npm run build && npm prune --omit=dev`, recommended in Discussion #6956)
* the package is gone, so merely importing `codeStripper.ts` — which every
* Compression Context page (Lite/Aggressive/Ultra/CCR) pulls in — threw a
* module-not-found error and broke the whole feature.
*
* The fix resolves `typescript` lazily and only when AST-based comment
* stripping is actually requested (opt-in, default off), degrading to a no-op
* when the package is unavailable instead of crashing at import time.
*
* Run: node --import tsx/esm --test tests/unit/compression/codestripper-lazy-ts-7096.test.ts
*/
import { describe, it, afterEach } from "node:test";
import assert from "node:assert/strict";
import fs from "node:fs";
import path from "node:path";
import { fileURLToPath } from "node:url";
// Namespace import so a missing named export does not crash the whole test
// module at load — it simply shows up as `undefined` (clean granular red).
import * as codeStripper from "../../../open-sse/services/compression/engines/rtk/codeStripper.ts";
const CODE_STRIPPER_SOURCE = fileURLToPath(
new URL("../../../open-sse/services/compression/engines/rtk/codeStripper.ts", import.meta.url)
);
describe("RTK codeStripper — lazy TypeScript loading (#7096)", () => {
afterEach(() => {
// Always restore the default loader if the seam exists.
codeStripper.__setTypeScriptModuleLoaderForTests?.(null);
});
it("does not import the `typescript` package eagerly at module top level", () => {
const source = fs.readFileSync(CODE_STRIPPER_SOURCE, "utf8");
// A top-level *value* import of typescript (`import ts from "typescript"`)
// is what broke every compression page after `npm prune --omit=dev`.
// Type-only imports (`import type ... from "typescript"`) are erased at
// build time and are fine.
const eagerValueImport =
/^\s*import\s+(?!type\b)[^;\n]*\bfrom\s+["']typescript["']/m.test(source);
assert.equal(
eagerValueImport,
false,
"codeStripper.ts must not import `typescript` eagerly at module top level (use a lazy require instead)"
);
});
it("still strips comments when `typescript` is available (opt-in)", () => {
const code = [
"const x = 1; // inline note",
"// full line comment",
"const y = 2;",
].join("\n");
const out = codeStripper.stripCode(code, "typescript", { removeComments: true });
assert.ok(!out.text.includes("inline note"), "line comment should be removed");
assert.ok(!out.text.includes("full line comment"), "full-line comment should be removed");
assert.ok(out.text.includes("const x = 1"), "code should survive");
assert.ok(out.text.includes("const y = 2"), "code should survive");
});
it("degrades to a no-op (no throw) when `typescript` cannot be resolved", () => {
assert.equal(
typeof codeStripper.__setTypeScriptModuleLoaderForTests,
"function",
"codeStripper must expose a lazy TypeScript loader seam so it can degrade gracefully"
);
// Simulate `npm prune --omit=dev`: typescript is not resolvable.
codeStripper.__setTypeScriptModuleLoaderForTests(() => null);
const code = ["const x = 1; // keep me", "const y = 2;"].join("\n");
let out: ReturnType<typeof codeStripper.stripCode>;
assert.doesNotThrow(() => {
out = codeStripper.stripCode(code, "typescript", { removeComments: true });
}, "stripCode must not throw when typescript is unavailable");
// Comment stripping is skipped, but the code passes through intact.
assert.ok(out!.text.includes("const x = 1"), "code passes through");
assert.ok(out!.text.includes("keep me"), "comment left intact under graceful degradation");
});
});