From 0a244a39c2465d1afeaf29af3a3dcf3820155081 Mon Sep 17 00:00:00 2001 From: johnnyreilly Date: Sun, 25 Oct 2026 09:48:22 +0000 Subject: [PATCH] chore: tidy comments in src/ and fix typos in API error messages Trim, deduplicate and correct comments under src/: - fix comments that referenced a removed fs/readFile override, wrong files/functions, or stale type names - keep detailed explanations in one place (function docs) with short pointers from types.ts - keep the Windows path / casing gotchas and upstream issue links Also fix doubled wording in two error messages and an info log ("TypeScript TypeScript mode", "TypeScript typeScript API", "typescript typeScript API"). Co-Authored-By: Claude Opus 5.5 --- src/colors.ts | 9 +- src/index.ts | 59 ++----- src/typeScriptApi.ts | 412 +++++++++++++------------------------------ src/types.ts | 81 ++------- 4 files changed, 155 insertions(+), 406 deletions(-) diff --git a/src/colors.ts b/src/colors.ts index 50cdbebab..fd164df9c 100644 --- a/src/colors.ts +++ b/src/colors.ts @@ -39,11 +39,7 @@ function buildColors(base: PicocolorsInstance): Colors { }; } -// `pc` is itself the default picocolors instance, already built once (at -// module load, using auto-detected `isColorSupported`) rather than -// recomputed - reusing it here avoids rebuilding all ~26 of its formatter -// functions on every call when the requested state happens to match it, -// which is the common case (colors enabled on a color-capable terminal). +// Both variants are built once and shared. const colorsEnabled = buildColors(pc); let colorsDisabled: Colors | undefined; @@ -51,8 +47,5 @@ export function createColors(loaderOptionsColors: boolean): Colors { if (loaderOptionsColors && pc.isColorSupported) { return colorsEnabled; } - // Only ever one "disabled" variant needed, so build it once and reuse it - - // there's no `enabled`-dependent state in `LoaderOptions` beyond this - // boolean, so a single memoized instance covers every disabled call. return (colorsDisabled ??= buildColors(pc.createColors(false))); } diff --git a/src/index.ts b/src/index.ts index 67c437269..a8dbbf13e 100644 --- a/src/index.ts +++ b/src/index.ts @@ -155,24 +155,15 @@ function getTypeScriptInstance( loader.addDependency(configFilePath); } - // Captured now rather than read as `loader._compiler` from inside the hook - // callback below: `loader` is this one loader invocation's short-lived - // context, torn down well before a later rebuild's `compile` fires, but - // the `Compiler` it currently points to is the same long-lived instance - // for the whole watch session - only the reference needs capturing early, - // its `modifiedFiles`/`removedFiles` are still read fresh on every fire. + // Captured here: `loader` is short-lived, but the Compiler lives for the + // whole watch session. const compiler = loader._compiler!; - // Only the first file compiled in a given build needs to tell the API - // which files might be stale in its view (see `pendingInvalidation`/ - // `pendingChangedFiles`/`pendingRemovedFiles` in typeScriptApi.ts); - // `compile` fires once per build/watch rebuild in both webpack 4 and 5, so - // tap it to re-arm that ahead of every rebuild. + // Fires once per build/rebuild: tells the next build's first compile what + // may be stale (see `pendingInvalidation`/`pendingChangedFiles` in types.ts). compiler.hooks.compile.tap('ts-loader', () => { - // Webpack 5's watcher already knows exactly which files changed/were - // removed since the last build - both are `undefined` only for the very - // first build (nothing to compare against yet) or on webpack 4 (no - // equivalent API), which is when a full rescan is genuinely needed. + // `modifiedFiles`/`removedFiles` are undefined on the first build and on + // webpack 4 - only then is a full rescan needed. if ( loaderUtils.isWebpack5 && (compiler.modifiedFiles !== undefined || @@ -212,24 +203,15 @@ function getTypeScriptInstance( } }); - // The typeScript API instance holds a live `tsgo` child process (see - // typeScriptApi.ts's createTypeScriptInstance) that nothing else kills: - // it's spawned per instance, not per file, and the instance cache is a - // WeakMap keyed on the compiler, so leaving this untapped leaks one native - // process per compiler for as long as the Node process runs. `watchClose` - // covers a watcher's own teardown; `shutdown` (webpack 5 only - webpack 4 - // has no `compiler.close()`/`shutdown` hook) covers the one-shot - // run/close pattern. + // Each instance holds a live `tsgo` child process that nothing else kills, + // leaking one per compiler otherwise. `watchClose` covers watch mode; + // `shutdown` (webpack 5 only) covers run/close. const disposeTypeScriptApiInstance = () => { try { instance.typeScriptApiInstance.api.close(); } catch { - // The child process may already be gone (crashed, OOM-killed, or - // otherwise reaped) - close() talks to it one last time to shut it - // down cleanly, and that call throws like any other request does once - // the pipe is dead (see updateSnapshot's own callers). There's nothing - // left to preserve at this point; letting this escape would crash the - // whole webpack process on an otherwise-unrelated watcher teardown. + // close() throws if the child process is already dead; don't let that + // crash webpack during teardown. log.logInfo( 'failed to close TypeScript API child process, it may have already been closed', ); @@ -245,9 +227,7 @@ function getTypeScriptInstance( } setTSInstanceInCache(compiler, loaderOptions.instance, instance); - log.logInfo( - `Using ${loaderOptions.compiler} typeScript API with ${configFilePath}`, - ); + log.logInfo(`Using ${loaderOptions.compiler} API with ${configFilePath}`); return instance; } @@ -394,10 +374,8 @@ const validLoaderOptions: ValidLoaderOptions[] = [ 'reportFiles', 'allowTsInNodeModules', 'projectReferences', - // Accepted for backwards compatibility but currently inert: the - // `typescript/sync` (tsgo) API this loader now runs on only - // exposes filesystem-level hooks, not a resolveModuleName-style custom - // resolver hook. See test/execution-tests/3.0.1_resolveModuleName. + // Accepted for backwards compatibility but inert: the TypeScript 7 API has + // no custom resolver hook. See test/execution-tests/3.0.1_resolveModuleName. 'resolveModuleName', 'resolveTypeReferenceDirective', 'useCaseSensitiveFileNames', @@ -495,10 +473,6 @@ function updateFileInCache( let file: TSFile | undefined = instance.files.get(resolvedPath); if (file === undefined) { - // No "don't compile this" step needed for a disallowed node_modules - // file here: the API already skips emit for files it considers part of - // an external library, surfaced instead as makeSourceMapAndFinish's - // "TypeScript emitted no output" error below. file = { fileName: filePath, version: 0 }; instance.files.set(resolvedPath, file); instance.version++; @@ -554,9 +528,8 @@ function makeSourceMapAndFinish( ) { if (outputText === null || outputText === undefined) { setModuleMeta(loaderContext, fileVersion); - // A file under node_modules is the most common cause: the API skips - // emit for a source file it considers part of an external library, - // matching classic ts-loader's guidance for the same situation. + // Most commonly a node_modules file: the API skips emit for external + // library files. const additionalGuidance = !allowTsInNodeModules && filePath.indexOf('node_modules') !== -1 ? ' By default, ts-loader will not compile .ts files in node_modules.\n' + diff --git a/src/typeScriptApi.ts b/src/typeScriptApi.ts index 18d5468f5..ff49dfad9 100644 --- a/src/typeScriptApi.ts +++ b/src/typeScriptApi.ts @@ -45,11 +45,9 @@ export function createTypeScriptInstance( useCaseSensitiveFileNames: boolean, ): TypeScriptInstance { const typeScriptApiModule = loadTypeScriptApiModule(loaderOptions.compiler); - // No `fs` callbacks: the API reads the host filesystem directly, and the - // content webpack hands the loader is layered over it per file instead - - // see layerFileContents. Case sensitivity is passed explicitly so the API - // always agrees with `resolvedFilePathCache` about which paths are the - // same file. + // No `fs` callbacks: the API reads disk directly, with webpack's content + // layered on top per file (see layerFileContents). Case sensitivity is + // passed explicitly so the API agrees with `resolvedFilePathCache`. const api = new typeScriptApiModule.API({ useCaseSensitiveFileNames }); return { @@ -72,15 +70,9 @@ export function getTypeScriptEmit( instance: TSInstance, loaderContext: webpack.LoaderContext, ) { - // See toApiFacingFileName - used for every call into the TypeScript API - // below; `fileName` itself is reserved for what's reported back to webpack - // or shown to the user. + // `apiFileName` for API calls; `fileName` for anything shown to webpack/the user. const apiFileName = toApiFacingFileName(fileName); - // transpileModule/transpileDeclaration are program-less, single-file - // transforms (matching classic ts-loader's transpileOnly behaviour), so - // none of the synthetic-root machinery below - needed only to satisfy - // program.getJavaScriptEmit's root-file requirement - applies here. if (instance.loaderOptions.transpileOnly) { return getTranspileOnlyEmit( fileName, @@ -128,9 +120,7 @@ export function getTypeScriptEmit( : (snapshot.getDefaultProjectForFile(apiFileName) ?? configuredProject); if (!project) { - throw new Error( - `TypeScript TypeScript mode could not resolve project for ${fileName}.`, - ); + throw new Error(`TypeScript could not resolve a project for ${fileName}.`); } const program = project.program; @@ -138,12 +128,8 @@ export function getTypeScriptEmit( const configFileParsingDiagnostics = program.getConfigFileParsingDiagnostics(); if (configFileParsingDiagnostics.length > 0) { - // A broken tsconfig is a hard failure in classic ts-loader too: - // there's nothing left to compile, so report the diagnostic(s) then - // throw, which webpack surfaces as a "Module build failed" error. - // Returned rather than thrown so index.ts's `loader` can reconstruct - // the Error in its own stack frame, matching classic ts-loader's - // single-frame stack trace for this failure. + // Returned rather than thrown so index.ts's `loader` can create the + // Error in its own frame, matching classic ts-loader's stack trace. return { outputText: undefined, sourceMapText: undefined, @@ -199,30 +185,20 @@ export function getTypeScriptEmit( ), ); - // Deferred to processAssets (see reportPendingTypeScriptDiagnostics in - // index.ts) so we can tell whether webpack already recorded its own - // error for this module and avoid double-counting. + // Reported at end of build (reportPendingTypeScriptDiagnostics), once we + // can tell whether webpack already recorded its own error for the module. instance.pendingDiagnostics.set(instance.resolvedFilePathCache(fileName), { fileName, errors, }); - // A dependant's diagnostics can change without its own module - // rebuilding, and webpack has no reason to re-invoke ts-loader for it - - // recorded here and rechecked once for the whole build by - // recheckAllTransitiveDependants (see index.ts's postCompile hook), - // matching classic's afterCompile re-check. + // Its dependants are rechecked once per build by recheckAllTransitiveDependants. typeScriptInstance.changedFilesThisBuild.add(apiFileName); - // Read out synchronously for the same reason as `errors` above; - // emitted as webpack assets later by emitPendingDeclarationFiles. if ( program.getCompilerOptions().declaration || program.getCompilerOptions().composite ) { - // Scans the whole project (e.g. an allowJs .js file never passes - // through ts-loader's own webpack rule), matching classic's - // provideDeclarationFilesToWebpack. recordProjectDeclarationFiles(instance, program); } @@ -230,11 +206,9 @@ export function getTypeScriptEmit( } /** - * transpileOnly's emit path, built on `api.transpileModule` (and, for - * `isolatedDeclarations` projects, `api.transpileDeclaration`) rather than - * `getTypeScriptEmit`'s program-based `program.getJavaScriptEmit`. Both are - * program-less single-file transforms, so only the project's resolved - * compiler options are needed, not its root files. + * transpileOnly's emit path: `api.transpileModule` (plus + * `api.transpileDeclaration` for `isolatedDeclarations`) - program-less + * single-file transforms that need only the project's compiler options. */ function getTranspileOnlyEmit( fileName: string, @@ -261,12 +235,9 @@ function getTranspileOnlyEmit( }; } - // Unlike every other API call in this file, transpileModule/ - // transpileDeclaration build their own virtual filesystem from `fileName` - // verbatim rather than resolving it through a `Project` - an OS-native - // path panics on Windows ("mixed posix and windows paths") once combined - // with the API's forward-slash-normalized internal paths. See - // toForwardSlashPath. + // transpileModule/transpileDeclaration use `fileName` verbatim in their own + // virtual filesystem; an OS-native path panics there on Windows ("mixed + // posix and windows paths"). const transpileFileName = toForwardSlashPath(apiFileName); const compilerOptions = config.options; @@ -313,13 +284,9 @@ function getTranspileOnlyEmit( typeScriptInstance.transpileConfigDiagnosticsReported = true; } - // No type-checking to wait for in transpileOnly mode, so report - // immediately, matching classic ts-loader. + // No type-check to wait for, so report immediately, matching classic. reportTypeScriptErrors(loaderContext, errors); - // transpileDeclaration is the only way transpileOnly can produce .d.ts - // output: the full, type-checked program.getDeclarationEmit path needs a - // checker transpileOnly never runs. if ( (compilerOptions.declaration || compilerOptions.composite) && compilerOptions.isolatedDeclarations @@ -399,10 +366,9 @@ function requiresRootFileDiagnostics(config: ParsedCommandLine): boolean { } /** - * Reports a broken tsconfig's diagnostics - a hard failure in classic - * ts-loader too: there's nothing left to compile, so report the - * diagnostic(s) then throw, which webpack surfaces as a "Module build - * failed" error. Shared by getTypeScriptEmit and getTranspileOnlyEmit. + * Reports a broken tsconfig's diagnostics and returns the message for the + * "Module build failed" error the caller raises - a hard failure, as in + * classic ts-loader. */ function reportConfigFileParsingErrors( instance: TSInstance, @@ -430,20 +396,15 @@ function reportConfigFileParsingErrors( } /** - * Emits a single file's declaration output via `api.transpileDeclaration`. - * Unlike `program.getDeclarationEmit`, this returns raw text rather than a - * named output file, so the output path is computed here. Emitted directly - * via `loaderContext.emitFile` rather than `instance.pendingDeclarationFiles`, - * since transpileOnly instances never register addPostCompileHooks (their - * diagnostics are reported immediately, not deferred). + * transpileDeclaration returns raw text, not a named output file, so the + * output path is computed here. Emitted directly rather than via + * `pendingDeclarationFiles`: transpileOnly never registers addPostCompileHooks. */ function recordTranspileOnlyDeclarationFile( instance: TSInstance, typeScriptInstance: TypeScriptInstance, loaderContext: webpack.LoaderContext, fileName: string, - // Forward-slash-normalized (see transpileFileName's own comment at the - // call site in getTranspileOnlyEmit). transpileFileName: string, contents: string, compilerOptions: CompilerOptions, @@ -546,7 +507,7 @@ function loadTypeScriptApiModule(compilerPackage: string): TypeScriptApiModule { return require(specifier) as TypeScriptApiModule; } catch (error) { throw new Error( - `Could not load TypeScript typeScript API from "${specifier}": ${ + `Could not load the TypeScript API from "${specifier}": ${ error instanceof Error ? error.message : String(error) }`, ); @@ -568,25 +529,16 @@ function updateSnapshot( resolvedFilePathCache: ResolvedFilePathCache, fileName: string, changes?: SnapshotChanges, - // Api-facing names of specific files known to have just changed (see - // `changedFilesThisBuild`) - passed only by recheckAllTransitiveDependants's - // own snapshot refresh. Tells the API to re-read exactly these files rather - // than every file in the project, unlike `pendingInvalidation`'s full - // `invalidateAll` below - see its call site for why a targeted list is both - // sufficient and far cheaper there. + // Only from recheckAllTransitiveDependants: files compiled this build, + // for the API to re-read. changedFileNames?: ReadonlySet, ) { const previousSnapshot = typeScriptInstance.snapshot; - // Priority, highest first: (1) `pendingInvalidation`'s full `invalidateAll` - - // ts-loader has no idea what changed (webpack 4, or the very first build), - // so it has to assume anything could have. (2) `pendingChangedFiles`/ - // `pendingRemovedFiles` - webpack 5's watcher already knows exactly which - // files changed since the last build (see its `compile`-hook wiring in - // index.ts), so listing them is just as sufficient as (1) but far cheaper. - // (3) `changedFileNames` - see its own doc comment above. Only ever one of - // these three is live for a given call: (1) and (2) are set (and consumed - // here) once per build, before any file compiles; (3) is passed explicitly - // by the one caller that needs it, once that's no longer true. + // Which files the API should re-read, most conservative first: + // 1. `pendingInvalidation` - nothing known (first build, webpack 4): rescan all. + // 2. `pendingChangedFiles`/`pendingRemovedFiles` - webpack 5's watcher list. + // 3. `changedFileNames`. + // At most one applies: 1 and 2 are consumed by the first call of a build. const pendingChangedOrRemovedFiles = typeScriptInstance.pendingChangedFiles ?? typeScriptInstance.pendingRemovedFiles; @@ -618,27 +570,15 @@ function updateSnapshot( ), } : undefined; + // directImportsCache: a full rescan could change any file, and a created + // file can satisfy imports that previously failed to resolve, so clear it + // all. Otherwise only the listed files' own entries can be stale (for + // `changedFileNames` they were already rewritten by their own compile - + // see registerResolvedImportDependencies). projectFileNamesCache + // self-validates, so it's never cleared. if (typeScriptInstance.pendingInvalidation || createdFiles.size > 0) { - // A created file can also satisfy imports that other files previously - // failed to resolve, leaving their own entries stale too. - // - // A real rescan can change any file's content, so directImportsCache - // entries (derived from a file's own imports - see - // findTransitiveDependants) can no longer be trusted. A targeted - // `changed`/`deleted` list (either branch below) needs no such clear: - // every *other* file's own entry is unaffected by files it doesn't - // import, and the listed files' own entries are handled explicitly below - // (`pendingChangedFiles`/`pendingRemovedFiles`) or are already fresh, - // written by their own compile earlier this build (`changedFileNames` - - // see registerResolvedImportDependencies). projectFileNamesCache isn't - // cleared here at all - it validates itself against the program's own - // file list on every read (see getProjectFileNames), so it's safe to - // keep across every case here, including a full rescan. typeScriptInstance.directImportsCache.clear(); } else if (pendingChangedOrRemovedFiles) { - // Only these specific files' own direct-imports could have changed (a - // content edit) or stopped mattering (a removal) - every other project - // file's own entry is still derived from its own, unaffected content. for (const rawFileName of [ ...(typeScriptInstance.pendingChangedFiles ?? []), ...(typeScriptInstance.pendingRemovedFiles ?? []), @@ -651,24 +591,20 @@ function updateSnapshot( typeScriptInstance.pendingInvalidation = false; typeScriptInstance.pendingChangedFiles = undefined; typeScriptInstance.pendingRemovedFiles = undefined; - // Every path in a request keeps its real casing, the same as `openProjects` - // (see `configFilePath`): a mismatch - e.g. a lowercased drive letter in - // one and not the other - is enough on its own to make the API unable to - // find a project for the opened file on Windows - see + // Real casing, not resolvedFilePathCache's canonical form: a casing + // mismatch with `openProjects` (e.g. a lowercased drive letter) stops the + // API finding the file's project on Windows - see // https://github.com/microsoft/TypeScript/pull/64204#issuecomment-5791438694. - // resolvedFilePathCache's canonical form is only for ts-loader's own keys: - // the API echoes back whatever casing it's given, into diagnostics too. + // The API also echoes this casing back into diagnostics. const apiFacingFileName = path.resolve(fileName); const params: CreateSnapshotParams = { ...changes, openFiles: [apiFacingFileName], fileNotifications, }; - // The API no longer tracks a "latest snapshot" of its own (see - // https://github.com/microsoft/TypeScript/pull/64204) - ts-loader must - // explicitly layer each new snapshot over the previous one via - // `snapshot.update` to retain its cache, falling back to a fresh - // `createSnapshot` only for the very first snapshot of the instance. + // The API keeps no "latest snapshot" of its own, so each one must be built + // via `update` on the previous to keep its cache + // (https://github.com/microsoft/TypeScript/pull/64204). const snapshot = previousSnapshot ? previousSnapshot.update(params) : typeScriptInstance.api.createSnapshot(params); @@ -718,12 +654,7 @@ function findCreatedFiles( ); } -/** - * Api-facing names (see `toApiFacingFileName`) of a raw webpack file-path - * set, or `undefined` for an absent/empty set - so an unpopulated - * `pendingRemovedFiles` (say) omits `deleted` from `FileNotifications` entirely - * rather than sending an empty array. - */ +/** `undefined` for an empty set, so the field is omitted from the request. */ function mapApiFacingFileNames( fileNames: ReadonlySet | undefined, ): string[] | undefined { @@ -735,11 +666,7 @@ function mapApiFacingFileNames( ); } -/** - * Opens (or reuses) the primary project - the one configured project - * ts-loader resolved via `configFilePath`. Shared by `prepareSnapshotForFile` - * (which may fall back to a synthetic project) and `getTranspileOnlyEmit`. - */ +/** Opens (or reuses) the project for `configFilePath`. */ function openPrimaryProject( typeScriptInstance: TypeScriptInstance, resolvedFilePathCache: ResolvedFilePathCache, @@ -769,16 +696,12 @@ function openPrimaryProject( /** * Layers `contents` - what webpack handed the loader, which can differ from * disk (another loader ran first) or not exist there at all (a `.vue.ts` - * alias) - over the instance's snapshot for the rest of its life, so every - * later snapshot and program sees it too, not just this file's own compile. + * alias) - over the snapshot, so every later snapshot and program sees it. * - * The primary project's snapshot is otherwise refreshed only once per build, - * not per file - see - * https://github.com/microsoft/TypeScript/issues/63875#issuecomment-5513828692 - - * when `pendingInvalidation`/`pendingChangedFiles`/`pendingRemovedFiles` - * indicate something may be stale (same gate as getTranspileConfig). That - * refresh opens `fileName`, so it carries the layer too: opening a file that - * exists only in a layer fails if the layer arrives in a later request. + * The primary project is refreshed only on the first call of a build (see + * https://github.com/microsoft/TypeScript/issues/63875#issuecomment-5513828692). + * That refresh carries the layer in the same request because opening a + * layer-only file fails if the layer arrives later. */ function layerFileContents( typeScriptInstance: TypeScriptInstance, @@ -818,9 +741,8 @@ function prepareSnapshotForFile( contents: string, forceSyntheticRoot: boolean, ): { snapshot: Snapshot; syntheticProgramId?: SyntheticProjectId } { - // Layered before the primary project is consulted below - a layered file - // also shows up in directory listings, so it can change which files the - // project's `include` globs match (e.g. a virtual `.vue.ts` alias). + // Layered first: a layered file appears in directory listings, so it can + // change what the project's `include` globs match. const snapshot = layerFileContents( typeScriptInstance, resolvedFilePathCache, @@ -834,9 +756,7 @@ function prepareSnapshotForFile( return { snapshot }; } - // A tsconfig that fails to parse is a hard failure classic ts-loader - // surfaces directly - falling back to a synthetic program would silently - // paper over it by explicitly listing `fileName`. + // A broken tsconfig must surface; a synthetic program would hide it. if ( primaryProject && primaryProject.program.getConfigFileParsingDiagnostics().length > 0 @@ -855,16 +775,13 @@ function prepareSnapshotForFile( } /** - * The one synthetic program (the API's `createPrograms`) shared by every - * file the primary project doesn't include - matching classic ts-loader's - * single program, whose roots were the tsconfig's files plus every file - * webpack handed it. Its roots are the primary project's own root files plus - * just the orphan files that need to be roots: one is added only when the - * program doesn't already contain it (most orphans arrive via an import), or - * always under `forceRoot` (the API skips emit for a node_modules file that - * isn't one). Being a superset of the primary project, it also gives the - * transitive-dependant recheck the full import graph - see - * recheckAllTransitiveDependants. + * The one synthetic program shared by every "orphan" file the primary + * project doesn't include - like classic ts-loader's single program, whose + * roots were the tsconfig's files plus every file webpack handed it. Roots: + * the primary project's roots, plus each orphan the program doesn't already + * contain via an import (or always, under `forceRoot` - the API skips emit + * for a node_modules file that isn't a root). As a superset of the primary + * project it's also the full import graph for recheckAllTransitiveDependants. */ function ensureOrphanProgram( typeScriptInstance: TypeScriptInstance, @@ -967,15 +884,12 @@ function orphanProgramParams( } /** - * The identity used for every TypeScript-API-facing call, as opposed to - * `fileName` itself, which is what's reported to webpack/the user. A file - * with an unrecognized extension (e.g. a Vue SFC's extracted `