fix: Switch to didChangeConfiguration, always use local filesystem

Also add 'node_modules' to resolvedPath
Don't give up when resolveJavaScriptModule fails

Author: tomv564

src/plugins.ts

@@ -1,7 +1,10 @@
+import * as fs from 'mz/fs';
+import * as path from 'path';
 import * as ts from 'typescript';
 import { Logger, NoopLogger } from './logging';
 import { combinePaths } from './match-files';
-import { InitializationOptions } from './request-type';
+import { PluginSettings } from './request-type';
+import { toUnixPath } from './util';
 
 // Based on types and logic from TypeScript server/project.ts @
 // https://github.com/Microsoft/TypeScript/blob/711e890e59e10aa05a43cb938474a3d9c2270429/src/server/project.ts
@@ -58,11 +61,17 @@ export class PluginLoader {
 	private globalPlugins: string[] = [];
 	private pluginProbeLocations: string[] = [];
 
-	constructor(private rootFilePath: string, private fs: ts.ModuleResolutionHost, initializationOptions?: InitializationOptions, private logger = new NoopLogger(), private requireModule: (moduleName: string) => any = require) {
-		if (initializationOptions) {
-			this.allowLocalPluginLoads = initializationOptions.allowLocalPluginLoads || false;
-			this.globalPlugins = initializationOptions.globalPlugins || [];
-			this.pluginProbeLocations = initializationOptions.pluginProbeLocations || [];
+	constructor(
+		private rootFilePath: string,
+		private fs: ts.ModuleResolutionHost,
+		pluginSettings?: PluginSettings,
+		private logger = new NoopLogger(),
+		private resolutionHost = new LocalModuleResolutionHost(),
+		private requireModule: (moduleName: string) => any = require) {
+		if (pluginSettings) {
+			this.allowLocalPluginLoads = pluginSettings.allowLocalPluginLoads || false;
+			this.globalPlugins = pluginSettings.globalPlugins || [];
+			this.pluginProbeLocations = pluginSettings.pluginProbeLocations || [];
 		}
 	}
 
@@ -117,7 +126,7 @@ export class PluginLoader {
 				return;
 			}
 		}
-		this.logger.info(`Couldn't find ${pluginConfigEntry.name} anywhere in paths: ${searchPaths.join(',')}`);
+		this.logger.error(`Couldn't find ${pluginConfigEntry.name} anywhere in paths: ${searchPaths.join(',')}`);
 	}
 
 	/**
@@ -126,10 +135,11 @@ export class PluginLoader {
 	 * @param initialDir
 	 */
 	private resolveModule(moduleName: string, initialDir: string): {} | undefined {
-		this.logger.info(`Loading ${moduleName} from ${initialDir}`);
-		const result = this.requirePlugin(initialDir, moduleName);
+		const resolvedPath = toUnixPath(path.resolve(combinePaths(initialDir, 'node_modules')));
+		this.logger.info(`Loading ${moduleName} from ${initialDir} (resolved to ${resolvedPath})`);
+		const result = this.requirePlugin(resolvedPath, moduleName);
 		if (result.error) {
-			this.logger.info(`Failed to load module: ${JSON.stringify(result.error)}`);
+			this.logger.error(`Failed to load module: ${JSON.stringify(result.error)}`);
 			return undefined;
 		}
 		return result.module;
@@ -141,8 +151,8 @@ export class PluginLoader {
 	 * @param moduleName
 	 */
 	private requirePlugin(initialDir: string, moduleName: string): RequireResult {
-		const modulePath = this.resolveJavaScriptModule(moduleName, initialDir, this.fs);
 		try {
+			const modulePath = this.resolveJavaScriptModule(moduleName, initialDir, this.fs);
 			return { module: this.requireModule(modulePath), error: undefined };
 		} catch (error) {
 			return { module: undefined, error };
@@ -158,11 +168,23 @@ export class PluginLoader {
 	private resolveJavaScriptModule(moduleName: string, initialDir: string, host: ts.ModuleResolutionHost): string {
 		// TODO: this should set jsOnly=true to the internal resolver, but this parameter is not exposed on a public api.
 		const result =
-			ts.nodeModuleNameResolver(moduleName, /* containingFile */ initialDir.replace('\\', '/') + '/package.json', { moduleResolution: ts.ModuleResolutionKind.NodeJs, allowJs: true }, this.fs, undefined);
+			ts.nodeModuleNameResolver(moduleName, /* containingFile */ initialDir.replace('\\', '/') + '/package.json', { moduleResolution: ts.ModuleResolutionKind.NodeJs, allowJs: true }, this.resolutionHost, undefined);
 		if (!result.resolvedModule) {
 			// this.logger.error(result.failedLookupLocations);
 			throw new Error(`Could not resolve JS module ${moduleName} starting at ${initialDir}.`);
 		}
 		return result.resolvedModule.resolvedFileName;
 	}
 }
+
+/**
+ * A local filesystem-based ModuleResolutionHost for plugin loading.
+ */
+export class LocalModuleResolutionHost implements ts.ModuleResolutionHost {
+	fileExists(fileName: string): boolean {
+		return fs.existsSync(fileName);
+	}
+	readFile(fileName: string): string {
+		return fs.readFileSync(fileName, 'utf8');
+	}
+}

src/project-manager.ts

@@ -10,7 +10,7 @@ import { FileSystemUpdater } from './fs';
 import { Logger, NoopLogger } from './logging';
 import { InMemoryFileSystem } from './memfs';
 import { PluginCreateInfo, PluginLoader, PluginModuleFactory } from './plugins';
-import { InitializationOptions } from './request-type';
+import { PluginSettings } from './request-type';
 import { traceObservable, traceSync } from './tracing';
 import {
 	isConfigFile,
@@ -105,7 +105,7 @@ export class ProjectManager implements Disposable {
 	/**
 	 * Options passed to the language server at startup
 	 */
-	private initializationOptions?: InitializationOptions;
+	private pluginSettings?: PluginSettings;
 
 	/**
 	 * @param rootPath root path as passed to `initialize`
@@ -118,14 +118,14 @@ export class ProjectManager implements Disposable {
 		inMemoryFileSystem: InMemoryFileSystem,
 		updater: FileSystemUpdater,
 		traceModuleResolution?: boolean,
-		initializationOptions?: InitializationOptions,
+		pluginSettings?: PluginSettings,
 		protected logger: Logger = new NoopLogger()
 	) {
 		this.rootPath = rootPath;
 		this.updater = updater;
 		this.inMemoryFs = inMemoryFileSystem;
 		this.versions = new Map<string, number>();
-		this.initializationOptions = initializationOptions;
+		this.pluginSettings = pluginSettings;
 		this.traceModuleResolution = traceModuleResolution || false;
 
 		// Share DocumentRegistry between all ProjectConfigurations
@@ -153,7 +153,7 @@ export class ProjectManager implements Disposable {
 				'',
 				tsConfig,
 				this.traceModuleResolution,
-				this.initializationOptions,
+				this.pluginSettings,
 				this.logger
 			);
 			configs.set(trimmedRootPath, config);
@@ -183,7 +183,7 @@ export class ProjectManager implements Disposable {
 						filePath,
 						undefined,
 						this.traceModuleResolution,
-						this.initializationOptions,
+						this.pluginSettings,
 						this.logger
 					));
 					// Remove catch-all config (if exists)
@@ -813,7 +813,7 @@ export class ProjectConfiguration {
 		configFilePath: string,
 		configContent?: any,
 		traceModuleResolution?: boolean,
-		private initializationOptions?: InitializationOptions,
+		private pluginSettings?: PluginSettings,
 		private logger: Logger = new NoopLogger()
 	) {
 		this.fs = fs;
@@ -922,8 +922,8 @@ export class ProjectConfiguration {
 			this.logger
 		);
 		this.service = ts.createLanguageService(this.host, this.documentRegistry);
-		const pluginLoader = new PluginLoader(this.rootFilePath, this.fs, this.initializationOptions, this.logger);
-		pluginLoader.loadPlugins(options, (factory, config) => this.enableProxy(factory, config));
+		const pluginLoader = new PluginLoader(this.rootFilePath, this.fs, this.pluginSettings, this.logger);
+		pluginLoader.loadPlugins(options, this.enableProxy.bind(this) /* (factory, config) => this.enableProxy(factory, config)) */);
 		this.initialized = true;
 	}
 

src/request-type.ts

@@ -3,10 +3,12 @@ import * as vscode from 'vscode-languageserver';
 
 export interface InitializeParams extends vscode.InitializeParams {
 	capabilities: ClientCapabilities;
-	initializationOptions?: InitializationOptions;
 }
 
-export interface InitializationOptions {
+/**
+ * Settings to enable plugin loading
+ */
+export interface PluginSettings {
 	allowLocalPluginLoads: boolean;
 	globalPlugins: string[];
 	pluginProbeLocations: string[];

src/test/plugins.test.ts

@@ -3,7 +3,7 @@ import * as sinon from 'sinon';
 import * as ts from 'typescript';
 import {InMemoryFileSystem} from '../memfs';
 import {PluginLoader, PluginModule, PluginModuleFactory} from '../plugins';
-import {InitializationOptions} from '../request-type';
+import {PluginSettings} from '../request-type';
 import { path2uri } from '../util';
 
 describe('plugins', () => {
@@ -24,14 +24,14 @@ describe('plugins', () => {
 			const peerPackagesUri = path2uri(peerPackagesPath);
 			memfs.add(peerPackagesUri + '/node_modules/some-plugin/package.json', '{ "name": "some-plugin", "version": "0.1.1", "main": "plugin.js"}');
 			memfs.add(peerPackagesUri + '/node_modules/some-plugin/plugin.js', '');
-			const initializationOptions: InitializationOptions = {
+			const pluginSettings: PluginSettings = {
 				globalPlugins: ['some-plugin'],
 				allowLocalPluginLoads: false,
 				pluginProbeLocations: []
 			};
 			const pluginFactoryFunc = (modules: any) => 5;
 			const fakeRequire = (path: string) => pluginFactoryFunc;
-			const loader = new PluginLoader('/', memfs, initializationOptions, undefined, fakeRequire);
+			const loader = new PluginLoader('/', memfs, pluginSettings, undefined, memfs, fakeRequire);
 			const compilerOptions: ts.CompilerOptions = {};
 			const applyProxy = sinon.spy();
 			loader.loadPlugins(compilerOptions, applyProxy);
@@ -43,14 +43,14 @@ describe('plugins', () => {
 			const memfs = new InMemoryFileSystem('/some-project');
 			memfs.add('file:///some-project/node_modules/some-plugin/package.json', '{ "name": "some-plugin", "version": "0.1.1", "main": "plugin.js"}');
 			memfs.add('file:///some-project/node_modules/some-plugin/plugin.js', '');
-			const initializationOptions: InitializationOptions = {
+			const pluginSettings: PluginSettings = {
 				globalPlugins: [],
 				allowLocalPluginLoads: true,
 				pluginProbeLocations: []
 			};
 			const pluginFactoryFunc = (modules: any) => 5;
 			const fakeRequire = (path: string) => pluginFactoryFunc;
-			const loader = new PluginLoader('/some-project', memfs, initializationOptions, undefined, fakeRequire);
+			const loader = new PluginLoader('/some-project', memfs, pluginSettings, undefined, memfs, fakeRequire);
 			const pluginOption: ts.PluginImport = {
 				name: 'some-plugin'
 			};

src/typescript-service.ts

@@ -49,6 +49,7 @@ import {
 	InitializeResult,
 	PackageDescriptor,
 	PackageInformation,
+	PluginSettings,
 	ReferenceInformation,
 	SymbolDescriptor,
 	SymbolLocationInformation,
@@ -86,9 +87,9 @@ export type TypeScriptServiceFactory = (client: LanguageClient, options?: TypeSc
 /**
  * Settings synced through `didChangeConfiguration`
  */
-export interface Settings {
-	format: ts.FormatCodeSettings;
-}
+export type Settings = {
+	format: ts.FormatCodeSettings
+} & PluginSettings;
 
 /**
  * Handles incoming requests and return responses. There is a one-to-one-to-one
@@ -171,7 +172,10 @@ export class TypeScriptService {
 			insertSpaceBeforeFunctionParenthesis: false,
 			placeOpenBraceOnNewLineForFunctions: false,
 			placeOpenBraceOnNewLineForControlBlocks: false
-		}
+		},
+		allowLocalPluginLoads: false,
+		globalPlugins: [],
+		pluginProbeLocations: []
 	};
 
 	/**
@@ -223,7 +227,7 @@ export class TypeScriptService {
 				this.inMemoryFileSystem,
 				this.updater,
 				this.traceModuleResolution,
-				params.initializationOptions,
+				this.settings,
 				this.logger
 			);
 			this.packageManager = new PackageManager(this.updater, this.inMemoryFileSystem, this.logger);