Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attempt to fix #20 #22

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/classify-module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,13 @@ const readPJType = cachedMtime(
})
)

export const classifyModule = (fileName: string): PackageJsonType | 'json' => {
export const classifyModule = (
fileName: string
): PackageJsonType | 'json' => {
if (fileName.endsWith('.json')) {
return 'json'
} if (fileName.endsWith('.cts') || fileName.endsWith('.cjs')) {
}
if (fileName.endsWith('.cts') || fileName.endsWith('.cjs')) {
return 'commonjs'
} else if (fileName.endsWith('.mts') || fileName.endsWith('.mjs')) {
return 'module'
Expand Down
27 changes: 26 additions & 1 deletion src/service/get-output-typecheck.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
// Get output with full type-checking from the LanguageService.

import { relative } from 'path'
import type ts from 'typescript'
import ts from 'typescript'
import { info, warn } from '../debug.js'
import { readFile } from '../ts-sys-cached.js'
import { addRootFile, updateFileVersion } from './file-versions.js'
import { getLanguageService } from './language-service.js'
import { markFileNameInternal } from './resolve-module-name-literals.js'
Expand All @@ -27,6 +28,30 @@ export const getOutputTypeCheck = (
markFileNameInternal(fileName)
updateFileVersion(fileName, code)

// TODO: It's not clear which fields on dependenciesInfo are relevant
// TODO: Ensure that preProcessFile looks for dependencies recursively
// TODO: Skip the step of putting the fileNames into a Set if it's
// guaranteed that a given file won't be listed multiple times
const {
referencedFiles,
typeReferenceDirectives,
libReferenceDirectives,
importedFiles,
} = ts.preProcessFile(code)
const dependencyFileNames = new Set<string>()
for (const file of [
...referencedFiles,
...typeReferenceDirectives,
...libReferenceDirectives,
...importedFiles,
]) {
dependencyFileNames.add(file.fileName)
}

for (const dependencyFileName of dependencyFileNames) {
updateFileVersion(dependencyFileName, readFile(fileName) || '')
}

// if we can't get the source file, then return the code un-compiled.
// Eg, loading a JS file if allowJs is not set.
const programBefore = service.getProgram()
Expand Down
57 changes: 37 additions & 20 deletions src/service/load.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import { cachedMtime } from '@isaacs/cached'
import { mkdirSync, writeFileSync } from 'fs'
import { relative, resolve } from 'path'
import { ParsedCommandLine } from 'typescript'
import { Diagnostic, ParsedCommandLine } from 'typescript'
import { info } from '../debug.js'
import { getOutputFile } from '../get-output-file.js'
import {
Expand Down Expand Up @@ -31,30 +31,40 @@ export const load = (
const config = tsconfig()

if (lastConfig && config !== lastConfig) {
compileTypeCheck.cache.clear()
compileTranspileOnly.cache.clear()
compileTypeCheck.mtimeCache.clear()
compileTranspileOnly.mtimeCache.clear()
}
lastConfig = config

// compile to a file on disk, but only if the source has changed.
const compile = typeCheck ? compileTypeCheck : compileTranspileOnly
const cachedMtime = compile.mtimeCache.get(fileName)?.[0]
const newMtime = compile.getMtime(fileName)
let compile: (fileName: string) => {
outputText: string | undefined
diagnostics: Diagnostic[]
}
const outFile = getOutputFile(fileName)
const cachedResult = compile.cache.get(fileName)

if (
cachedMtime &&
cachedMtime === newMtime &&
fileExists(outFile) &&
cachedResult
) {
// saw this one, and have previous build available
return {
fileName: outFile,
diagnostics: reportAll(cachedResult.diagnostics, pretty),
// TODO: Re-enable caching of type-checked results
if (typeCheck) {
compile = compileTypeCheck
} else {
let cachedCompile = compileTranspileOnly
compile = cachedCompile

// Skip compiling if the source has not changed
const cachedMtime = cachedCompile.mtimeCache.get(fileName)?.[0]
const newMtime = cachedCompile.getMtime(fileName)
const cachedResult = cachedCompile.cache.get(fileName)

if (
cachedMtime &&
cachedMtime === newMtime &&
fileExists(outFile) &&
cachedResult
) {
// saw this one, and have previous build available
return {
fileName: outFile,
diagnostics: reportAll(cachedResult.diagnostics, pretty),
}
}
}

Expand Down Expand Up @@ -82,12 +92,19 @@ export const load = (
}
}

const compileTypeCheck = cachedMtime((fileName: string) => {
// It's fine to cache the results of transpilation, as compileTranspileOnly does,
// but it's not safe for this function to perform caching based purely on the
// mtime of a single source file. A dependency may have (for example)
// deleted something that the file was referencing.
// TODO: Cache compile results based on the mtime of all affected files,
// not just the one we're currently getting the results for
// (update cachedMtime() to accept an array of paths)
const compileTypeCheck = (fileName: string) => {
const normalizedFileName: string = normalizeSlashes(fileName)
/* c8 ignore next */
const content = readFile(fileName) || ''
return compile(content, normalizedFileName, true)
})
}

const compileTranspileOnly = cachedMtime((fileName: string) => {
const normalizedFileName: string = normalizeSlashes(fileName)
Expand Down