Skip to content

Commit

Permalink
fix test and some module cache invalidation issues
Browse files Browse the repository at this point in the history
  • Loading branch information
jasonlyu123 committed Sep 9, 2023
1 parent 9d20957 commit 742fec2
Show file tree
Hide file tree
Showing 5 changed files with 140 additions and 47 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,10 @@ export class LSAndTSDocResolver {
this.docManager.releaseDocument(uri);
}

async deleteModuleResolutionCache(filePath: string) {
await forAllServices((service) => service.invalidateModuleCache(filePath));
}

/**
* Updates project files in all existing ts services
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -510,14 +510,21 @@ export class TypeScriptPlugin
continue;
}

if (changeType === FileChangeType.Created && !doneUpdateProjectFiles) {
doneUpdateProjectFiles = true;
await this.lsAndTsDocResolver.updateProjectFiles();
} else if (changeType === FileChangeType.Deleted) {
if (changeType === FileChangeType.Deleted) {
await this.lsAndTsDocResolver.deleteSnapshot(fileName);
} else {
await this.lsAndTsDocResolver.updateExistingTsOrJsFile(fileName);
continue;
}

if (changeType === FileChangeType.Created) {
if (!doneUpdateProjectFiles) {
doneUpdateProjectFiles = true;
await this.lsAndTsDocResolver.updateProjectFiles();
}
await this.lsAndTsDocResolver.deleteModuleResolutionCache(fileName);
continue;
}

await this.lsAndTsDocResolver.updateExistingTsOrJsFile(fileName);
}
}

Expand Down
54 changes: 24 additions & 30 deletions packages/language-server/src/plugins/typescript/module-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -230,8 +230,15 @@ export function createSvelteModuleLoader(
containingSourceFile,
index
);
moduleCache.set(moduleName, containingFile, resolvedModule);
return resolvedModule;

resolvedModule?.failedLookupLocations?.forEach((failedLocation) => {
const failedPaths = failedPathToContainingFile.get(failedLocation) ?? new FileSet();
failedPaths.add(containingFile);
failedPathToContainingFile.set(failedLocation, failedPaths);
});

moduleCache.set(moduleName, containingFile, resolvedModule?.resolvedModule);
return resolvedModule?.resolvedModule;
});
}

Expand All @@ -240,7 +247,7 @@ export function createSvelteModuleLoader(
containingFile: string,
containingSourceFile: ts.SourceFile | undefined,
index: number
): ts.ResolvedModule | undefined {
): ts.ResolvedModuleWithFailedLookupLocations {
const mode = impliedNodeFormatResolver.resolve(
name,
index,
Expand All @@ -250,33 +257,37 @@ export function createSvelteModuleLoader(
// Delegate to the TS resolver first.
// If that does not bring up anything, try the Svelte Module loader
// which is able to deal with .svelte files.
const tsResolvedModule = tsModule.resolveModuleName(
const tsResolvedModuleWithFailedLookup = tsModule.resolveModuleName(
name,
containingFile,
compilerOptions,
wrapSystemWithMonitor(ts.sys, containingFile),
ts.sys,
tsModuleCache,
undefined,
mode
).resolvedModule;
);

const tsResolvedModule = tsResolvedModuleWithFailedLookup.resolvedModule;
if (tsResolvedModule && !isVirtualSvelteFilePath(tsResolvedModule.resolvedFileName)) {
return tsResolvedModule;
return tsResolvedModuleWithFailedLookup;
}

const svelteResolvedModule = tsModule.resolveModuleName(
const svelteResolvedModuleWithFailedLookup = tsModule.resolveModuleName(
name,
containingFile,
compilerOptions,
wrapSystemWithMonitor(svelteSys, containingFile),
svelteSys,
undefined,
undefined,
mode
).resolvedModule;
);

const svelteResolvedModule = svelteResolvedModuleWithFailedLookup.resolvedModule;
if (
!svelteResolvedModule ||
!isVirtualSvelteFilePath(svelteResolvedModule.resolvedFileName)
) {
return svelteResolvedModule;
return svelteResolvedModuleWithFailedLookup;
}

const resolvedFileName = ensureRealSvelteFilePath(svelteResolvedModule.resolvedFileName);
Expand All @@ -287,26 +298,9 @@ export function createSvelteModuleLoader(
resolvedFileName,
isExternalLibraryImport: svelteResolvedModule.isExternalLibraryImport
};
return resolvedSvelteModule;
}

function wrapSystemWithMonitor(sys: ts.System, containingFile: string): ts.System {
return {
...sys,
fileExists(path) {
path = ensureRealSvelteFilePath(path);
const exists = sys.fileExists(path);

if (exists) {
return true;
}

const set = failedPathToContainingFile.get(path) ?? new FileSet();
set.add(containingFile);
failedPathToContainingFile.set(path, set);

return false;
}
...svelteResolvedModuleWithFailedLookup,
resolvedModule: resolvedSvelteModule
};
}

Expand Down
73 changes: 63 additions & 10 deletions packages/language-server/src/plugins/typescript/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export interface LanguageServiceContainer {
getService(): ts.LanguageService;
updateSnapshot(documentOrFilePath: Document | string): DocumentSnapshot;
deleteSnapshot(filePath: string): void;
invalidateModuleCache(filePath: string): void;
updateProjectFiles(): void;
updateTsOrJsFile(fileName: string, changes?: TextDocumentContentChangeEvent[]): void;
/**
Expand All @@ -54,6 +55,21 @@ declare module 'typescript' {
interface LanguageServiceHost {
/** @internal */ hasInvalidatedResolutions?: (sourceFile: string) => boolean;
}

interface ResolvedModuleWithFailedLookupLocations {
/** @internal */
failedLookupLocations?: string[];
/** @internal */
affectingLocations?: string[];
/** @internal */
resolutionDiagnostics?: ts.Diagnostic[];
/**
* @internal
* Used to issue a diagnostic if typings for a non-relative import couldn't be found
* while respecting package.json `exports`, but were found when disabling `exports`.
*/
node10Result?: string;
}
}

const maxProgramSizeForNonTsFiles = 20 * 1024 * 1024; // 20 MB
Expand All @@ -66,6 +82,11 @@ const configFileModifiedTime = new FileMap<Date | undefined>();
const configFileForOpenFiles = new FileMap<string>();
const pendingReloads = new FileSet();

process.on('exit', () => {
// so that the schedule update doesn't keep the process open
forAllServices((service) => service.dispose());
});

/**
* For testing only: Reset the cache for services.
* Try to refactor this some day so that this file provides
Expand Down Expand Up @@ -254,8 +275,9 @@ async function createLanguageService(
log: (message) => Logger.debug(`[ts] ${message}`),
getCompilationSettings: () => compilerOptions,
getScriptFileNames,
getScriptVersion: (fileName: string) => getSnapshot(fileName).version.toString(),
getScriptSnapshot: getSnapshot,
getScriptVersion: (fileName: string) =>
getSnapshotIfExists(fileName)?.version.toString() || '',
getScriptSnapshot: getSnapshotIfExists,
getCurrentDirectory: () => workspacePath,
getDefaultLibFileName: ts.getDefaultLibFilePath,
fileExists: svelteModuleLoader.fileExists,
Expand All @@ -269,7 +291,7 @@ async function createLanguageService(
getNewLine: () => tsSystem.newLine,
resolveTypeReferenceDirectiveReferences:
svelteModuleLoader.resolveTypeReferenceDirectiveReferences,
hasInvalidatedResolutions: svelteModuleLoader.mightHaveInvalidatedResolutions,
hasInvalidatedResolutions: svelteModuleLoader.mightHaveInvalidatedResolutions
};

let languageService = ts.createLanguageService(host);
Expand All @@ -278,12 +300,7 @@ async function createLanguageService(
typingsNamespace: raw?.svelteOptions?.namespace || 'svelteHTML'
};

const onSnapshotChange = () => {
projectVersion++;
dirty = true;
scheduleUpdate();
};
docContext.globalSnapshotsManager.onChange(onSnapshotChange);
docContext.globalSnapshotsManager.onChange(scheduleUpdate);

reduceLanguageServiceCapabilityIfFileSizeTooBig();
updateExtendedConfigDependents();
Expand All @@ -302,6 +319,7 @@ async function createLanguageService(
fileBelongsToProject,
snapshotManager,
updateIfDirty,
invalidateModuleCache,
dispose
};

Expand All @@ -311,6 +329,13 @@ async function createLanguageService(
configFileForOpenFiles.delete(filePath);
}

function invalidateModuleCache(filePath: string) {
svelteModuleLoader.deleteFromModuleCache(filePath);
svelteModuleLoader.deleteUnresolvedResolutionsFromCache(filePath);

scheduleUpdate();
}

function updateSnapshot(documentOrFilePath: Document | string): DocumentSnapshot {
return typeof documentOrFilePath === 'string'
? updateSnapshotFromFilePath(documentOrFilePath)
Expand Down Expand Up @@ -357,6 +382,26 @@ async function createLanguageService(
return newSnapshot;
}

/**
* Deleted files will still be requested during the program update.
* Don't create snapshots for them.
* Otherwise, deleteUnresolvedResolutionsFromCache won't be called when the file is created again *
*/
function getSnapshotIfExists(fileName: string): DocumentSnapshot | undefined {
fileName = ensureRealSvelteFilePath(fileName);

let doc = snapshotManager.get(fileName);
if (doc) {
return doc;
}

if (!svelteModuleLoader.fileExists(fileName)) {
return undefined;
}

return createSnapshot(fileName, doc);
}

function getSnapshot(fileName: string): DocumentSnapshot {
fileName = ensureRealSvelteFilePath(fileName);

Expand All @@ -365,6 +410,10 @@ async function createLanguageService(
return doc;
}

return createSnapshot(fileName, doc);
}

function createSnapshot(fileName: string, doc: DocumentSnapshot | undefined) {
svelteModuleLoader.deleteUnresolvedResolutionsFromCache(fileName);
doc = DocumentSnapshot.fromFilePath(
fileName,
Expand Down Expand Up @@ -581,12 +630,13 @@ async function createLanguageService(
}

function dispose() {
clearTimeout(timer);
languageService.dispose();
snapshotManager.dispose();
configWatchers.get(tsconfigPath)?.close();
configWatchers.delete(tsconfigPath);
configFileForOpenFiles.clear();
docContext.globalSnapshotsManager.removeChangeListener(onSnapshotChange);
docContext.globalSnapshotsManager.removeChangeListener(scheduleUpdate);
}

function updateExtendedConfigDependents() {
Expand Down Expand Up @@ -669,6 +719,9 @@ async function createLanguageService(
}

function scheduleUpdate() {
projectVersion++;
dirty = true;

if (timer) {
clearTimeout(timer);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ describe('DiagnosticsProvider', function () {
const newFilePath = normalizePath(path.join(testDir, 'doesntexistyet.js')) || '';
writeFileSync(newFilePath, 'export default function foo() {}');
assert.ok(existsSync(newFilePath));
await lsAndTsDocResolver.getSnapshot(newFilePath);
await lsAndTsDocResolver.deleteModuleResolutionCache(newFilePath);

try {
const diagnostics2 = await plugin.getDiagnostics(document);
Expand All @@ -57,6 +57,41 @@ describe('DiagnosticsProvider', function () {
assert.deepStrictEqual(diagnostics3.length, 1);
}).timeout(5000);

it('notices changes of module resolution because of new file', async () => {
const { plugin, document, lsAndTsDocResolver } = setup('unresolvedimport.svelte');

const diagnostics1 = await plugin.getDiagnostics(document);
assert.deepStrictEqual(diagnostics1.length, 1);

// back-and-forth-conversion normalizes slashes
const newFilePath = normalizePath(path.join(testDir, 'doesntexistyet.js')) || '';
const newTsFilePath = normalizePath(path.join(testDir, 'doesntexistyet.ts')) || '';
writeFileSync(newFilePath, 'export function foo() {}');
assert.ok(existsSync(newFilePath));
await lsAndTsDocResolver.deleteModuleResolutionCache(newFilePath);

try {
const diagnostics2 = await plugin.getDiagnostics(document);
assert.deepStrictEqual(diagnostics2[0].code, 2613);
} catch (e) {
unlinkSync(newFilePath);
throw e;
}

writeFileSync(newTsFilePath, 'export default function foo() {}');
assert.ok(existsSync(newTsFilePath));
await lsAndTsDocResolver.deleteModuleResolutionCache(newTsFilePath);

try {
const diagnostics3 = await plugin.getDiagnostics(document);
assert.deepStrictEqual(diagnostics3.length, 1);
await lsAndTsDocResolver.deleteSnapshot(newTsFilePath);
} finally {
unlinkSync(newTsFilePath);
unlinkSync(newFilePath);
}
}).timeout(5000);

it('notices update of imported module', async () => {
const { plugin, document, lsAndTsDocResolver } = setup(
'diagnostics-imported-js-update.svelte'
Expand Down

0 comments on commit 742fec2

Please sign in to comment.