Skip to content

Commit

Permalink
(fix) (perf) getSnapshot fixes (sveltejs#142)
Browse files Browse the repository at this point in the history 
- svelte-sys readFile invoked getSvelteSnapshot with file contents which is just wrong
- optimization: save non-svelte-snapshots to manager, too, which greatly diminishes IO/file reads
  • Loading branch information
@dummdidumm
dummdidumm committed May 31, 2020
1 parent 2ece595 commit 1177fa2
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 74 deletions.
8 changes: 4 additions & 4 deletions packages/language-server/src/plugins/typescript/module-loader.ts
View file
Expand Up @@ -58,14 +58,14 @@ class ModuleResolutionCache {
* so it assumes it's a normal typescript file and searches for files like `../Component.svelte.ts`, which is wrong.
* In order to fix this, we need to wrap typescript's module resolution and reroute all `.svelte.ts` file lookups to .svelte.
*
* @param getSvelteSnapshot A function which returns a fully preprocessed typescript/javascript snapshot
* @param getSnapshot A function which returns a (in case of svelte file fully preprocessed) typescript/javascript snapshot
* @param compilerOptions The typescript compiler options
*/
export function createSvelteModuleLoader(
getSvelteSnapshot: (fileName: string) => DocumentSnapshot | undefined,
getSnapshot: (fileName: string) => DocumentSnapshot,
compilerOptions: ts.CompilerOptions,
) {
const svelteSys = createSvelteSys(getSvelteSnapshot);
const svelteSys = createSvelteSys(getSnapshot);
const moduleCache = new ModuleResolutionCache();

return {
Expand Down Expand Up @@ -113,7 +113,7 @@ export function createSvelteModuleLoader(
}

const resolvedFileName = ensureRealSvelteFilePath(tsResolvedModule.resolvedFileName);
const snapshot = getSvelteSnapshot(resolvedFileName);
const snapshot = getSnapshot(resolvedFileName);

const resolvedSvelteModule: ts.ResolvedModuleFull = {
extension: getExtensionFromScriptKind(snapshot && snapshot.scriptKind),
Expand Down
58 changes: 17 additions & 41 deletions packages/language-server/src/plugins/typescript/service.ts
View file
@@ -1,17 +1,12 @@
import { dirname, resolve } from 'path';
import ts from 'typescript';
import { DocumentSnapshot, INITIAL_VERSION } from './DocumentSnapshot';
import { createSvelteModuleLoader } from './module-loader';
import {
ensureRealSvelteFilePath,
getScriptKindFromFileName,
isSvelteFilePath,
findTsConfigPath,
} from './utils';
import { SnapshotManager } from './SnapshotManager';
import { Document } from '../../lib/documents';
import { getPackageInfo } from '../importPackage';
import { Logger } from '../../logger';
import { getPackageInfo } from '../importPackage';
import { DocumentSnapshot } from './DocumentSnapshot';
import { createSvelteModuleLoader } from './module-loader';
import { SnapshotManager } from './SnapshotManager';
import { ensureRealSvelteFilePath, findTsConfigPath, isSvelteFilePath } from './utils';

export interface LanguageServiceContainer {
getService(): ts.LanguageService;
Expand Down Expand Up @@ -66,7 +61,7 @@ export function createLanguageService(

const { compilerOptions, files } = getCompilerOptionsAndRootFiles();

const svelteModuleLoader = createSvelteModuleLoader(getSvelteSnapshot, compilerOptions);
const svelteModuleLoader = createSvelteModuleLoader(getSnapshot, compilerOptions);

const svelteTsPath = dirname(require.resolve('svelte2tsx'));
const svelteTsxFiles = ['./svelte-shims.d.ts', './svelte-jsx.d.ts'].map((f) =>
Expand All @@ -77,18 +72,8 @@ export function createLanguageService(
getCompilationSettings: () => compilerOptions,
getScriptFileNames: () =>
Array.from(new Set([...files, ...snapshotManager.getFileNames(), ...svelteTsxFiles])),
getScriptVersion(fileName: string) {
const doc = getScriptSnapshot(fileName);
return doc ? String(doc.version) : INITIAL_VERSION.toString();
},
getScriptSnapshot(fileName: string): ts.IScriptSnapshot | undefined {
const doc = getScriptSnapshot(fileName);
if (doc) {
return doc;
}

return ts.ScriptSnapshot.fromString(this.readFile!(fileName) || '');
},
getScriptVersion: (fileName: string) => getSnapshot(fileName).version.toString(),
getScriptSnapshot: getSnapshot,
getCurrentDirectory: () => workspacePath,
getDefaultLibFileName: ts.getDefaultLibFilePath,
fileExists: svelteModuleLoader.fileExists,
Expand All @@ -98,14 +83,7 @@ export function createLanguageService(
getDirectories: ts.sys.getDirectories,
// vscode's uri is all lowercase
useCaseSensitiveFileNames: () => false,
getScriptKind: (fileName: string) => {
const doc = getSvelteSnapshot(fileName);
if (doc) {
return doc.scriptKind;
}

return getScriptKindFromFileName(fileName);
},
getScriptKind: (fileName: string) => getSnapshot(fileName).scriptKind,
};
let languageService = ts.createLanguageService(host);

Expand Down Expand Up @@ -139,23 +117,21 @@ export function createLanguageService(
return languageService;
}

function getScriptSnapshot(fileName: string): DocumentSnapshot | undefined {
return getSvelteSnapshot(fileName) ?? snapshotManager.get(fileName);
}

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

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

const file = ts.sys.readFile(fileName) || '';
doc = DocumentSnapshot.fromDocument(createDocument(fileName, file));
if (isSvelteFilePath(fileName)) {
const file = ts.sys.readFile(fileName) || '';
doc = DocumentSnapshot.fromDocument(createDocument(fileName, file));
} else {
doc = DocumentSnapshot.fromFilePath(fileName);
}

snapshotManager.set(fileName, doc);
return doc;
}
Expand Down
17 changes: 5 additions & 12 deletions packages/language-server/src/plugins/typescript/svelte-sys.ts
View file
Expand Up @@ -5,28 +5,21 @@ import { ensureRealSvelteFilePath, isVirtualSvelteFilePath, toRealSvelteFilePath
/**
* This should only be accessed by TS svelte module resolution.
*/
export function createSvelteSys(
getSvelteSnapshot: (fileName: string) => DocumentSnapshot | undefined,
) {
export function createSvelteSys(getSnapshot: (fileName: string) => DocumentSnapshot) {
const svelteSys: ts.System = {
...ts.sys,
fileExists(path: string) {
return ts.sys.fileExists(ensureRealSvelteFilePath(path));
},
readFile(path, encoding) {
if (isVirtualSvelteFilePath(path)) {
const fileText = ts.sys.readFile(toRealSvelteFilePath(path), encoding);
const snapshot = getSvelteSnapshot(fileText!);
return fileText ? snapshot?.getText(0, snapshot.getLength()) : fileText;
}
const fileText = ts.sys.readFile(path, encoding);
return fileText;
readFile(path: string) {
const snapshot = getSnapshot(path);
return snapshot.getText(0, snapshot.getLength());
},
};

if (ts.sys.realpath) {
const realpath = ts.sys.realpath;
svelteSys.realpath = function(path) {
svelteSys.realpath = function (path) {
if (isVirtualSvelteFilePath(path)) {
return realpath(toRealSvelteFilePath(path)) + '.ts';
}
Expand Down
33 changes: 16 additions & 17 deletions packages/language-server/test/plugins/typescript/svelte-sys.test.ts
View file
Expand Up @@ -14,22 +14,23 @@ describe('Svelte Sys', () => {
const svelteFile = 'const a = "svelte file";';

const fileExistsStub = sinon.stub().returns(true);
const readFileStub = sinon.stub().returns(tsFile);
const getSvelteSnapshotStub = sinon.stub().returns(<Partial<DocumentSnapshot>>{
getText: () => svelteFile,
getLength: () => svelteFile.length,
});
const getSnapshotStub = sinon.stub().callsFake(
(path: string) =>
<Partial<DocumentSnapshot>>{
getText: () => (path.endsWith('.svelte.ts') ? svelteFile : tsFile),
getLength: () =>
path.endsWith('.svelte.ts') ? svelteFile.length : tsFile.length,
},
);

sinon.replace(ts.sys, 'fileExists', fileExistsStub);
sinon.replace(ts.sys, 'readFile', readFileStub);
const loader = createSvelteSys(getSvelteSnapshotStub);
const loader = createSvelteSys(getSnapshotStub);

return {
tsFile,
svelteFile,
fileExistsStub,
readFileStub,
getSvelteSnapshotStub,
getSnapshotStub,
loader,
};
}
Expand All @@ -51,21 +52,19 @@ describe('Svelte Sys', () => {
});

describe('#readFile', () => {
it('should delegate read to ts.sys for files with no .svelte.ts-ending as is', async () => {
const { loader, readFileStub, getSvelteSnapshotStub, tsFile } = setupLoader();
it('should invoke getSnapshot for ts/js files', async () => {
const { loader, getSnapshotStub, tsFile } = setupLoader();
const code = loader.readFile('../file.ts')!;

assert.strictEqual(readFileStub.getCall(0).args[0], '../file.ts');
assert.strictEqual(getSvelteSnapshotStub.called, false);
assert.strictEqual(getSnapshotStub.called, true);
assert.strictEqual(code, tsFile);
});

it('should convert .svelte.ts-endings and invoke getSvelteSnapshot', async () => {
const { loader, readFileStub, getSvelteSnapshotStub, svelteFile } = setupLoader();
it('should invoke getSnapshot for svelte files', async () => {
const { loader, getSnapshotStub, svelteFile } = setupLoader();
const code = loader.readFile('../file.svelte.ts')!;

assert.strictEqual(readFileStub.getCall(0).args[0], '../file.svelte');
assert.strictEqual(getSvelteSnapshotStub.called, true);
assert.strictEqual(getSnapshotStub.called, true);
assert.strictEqual(code, svelteFile);
});
});
Expand Down

0 comments on commit 1177fa2

Please sign in to comment.