perf: load TypeScript through a single require path - #3255
perf: load TypeScript through a single require path#3255JoshuaKGoldberg wants to merge 1 commit into
Conversation
Routes every internal value import of `typescript` and `ts-api-utils` through `createRequire` shims in `@flint.fyi/typescript-language`, so Node no longer runs cjs-module-lexer across the whole TypeScript bundle to build an ESM namespace for it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
✅ Deploy Preview for flint-fyi ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
And it automatically switches? Is this running in a sandboxed cloud environment or is it just stingy? |
|
And personally I still think it's better to do this in the build process |
SASSAFRAS
PR Checklist
status: accepting prsOverview
typescriptis CommonJS, so importing it from ESM makes Node run cjs-module-lexer across all ~9 MB oftypescript.jsto enumerate the named exports for an ESM namespace — workenableCompileCache()does not cover.packages/ts-patch/src/install-patch.tsalready loads TypeScript withrequire, so the same library was being loaded through both a CJS and an ESM path in one process.This adds two tiny shims to
@flint.fyi/typescript-languageand points internal value imports at them:@flint.fyi/typescript-language/typescript—createRequire(import.meta.url)("typescript"), re-exported as a default plus the enums and helpers used across the repo.@flint.fyi/typescript-language/ts-api-utils—createRequire(import.meta.url)("ts-api-utils"), which picks up that package's CJS build.The second shim is what makes the first one pay off.
ts-api-utils's ESM build doesimport ts from "typescript", so as long as anything in the graph reaches it, the lexer pass happens anyway; its CJS buildrequires TypeScript and reuses the instance already in the require cache. With both in place,cjs-module-lexerdrops from ~68 ms to ~3 ms of a two-file run's CPU profile, andtypescript.jsis loaded exactly once, byinstall-patch.The convention after this change is: types come from
"typescript"directly (import typeis erased, so it costs nothing), values come from the shim. In the 70 files that usedimport ts from "typescript"for both,tsnow names the type-only namespace andtypescriptnames the runtime module.Two supporting changes:
packages/ts-patch/src/install-patch-hooks.tsonly patched TypeScript's source whenloadhanded it aBuffer.registerHooksalso interceptsrequire(), where Node hands it a string — so under Vitest the patched build was silently skipped once TypeScript came in throughrequire. It now handles both, which is what keeps.astro/.vueextension support (_flintGetExtraSupportedExtensions) working.ts-api-utilsis no longer referenced frompackages/performance, so its dependency entry is removed (with the matchingpnpm-lock.yamlimporter entry). This is the one change I could not verify with an install — my localpnpmis v10 and this repo needs v11, so the lockfile edit is by hand: the three-line importer entry forpackages/performanceis dropped and nothing else. Worth apnpm install --lockfile-onlybefore merging.Measurements
node packages/flint/bin/index.js --cache-ignore --skip-formatting --skip-language-reports, hyperfine, 3 warmups × 10 runs, alternating branches within a single session (the machine drifts by 10–20% over tens of minutes, so back-to-back single runs are not comparable):Startup only, no linting — loading
install-patchplus the@flint.fyi/tsplugin graph and exiting:The
--versionbenchmark from the issue investigation is flat, and deliberately reported as such:--versionshort-circuits before any plugin is imported, so it never reaches a value import oftypescript— it measuresinstall-patchand the CLI shell only. It is not a useful isolation for this change; the module-graph row above is.Not covered
@flint.fyi/ts'spublishConfigpoints atdist/index.mjs, a single ~918 KB bundle, so an installed Flint does not load 301 separate rule modules. The 425 ms → 224 ms gap in the issue is specific to running from source, which is whatpackages/flint/bin/index.jsdoes and what these benchmarks measure. Closing it would mean making the CLI preferdistoversrc, i.e. making a fresh clone require a build beforeflintruns — a build-system change rather than a perf change, so I left it alone.typescript/lib/tsserverlibraryis a non-issue on TypeScript 6.@typescript-eslint/project-servicestill requires it, but in 6.0.3 that file is 1 KB and its whole body ismodule.exports = require("./typescript.js"), so it costs nothing and does not produce a second copy of TypeScript.e2etest failures on this branch (cspell,directives,no-reports,typescript) reproduce identically onmainat 116a836 and are unrelated. Thepackages/sitelint errors (missingastro:contenttypes) likewise reproduce onmain.