Skip to content

Commit 87b119a

Browse files
author
Keen Yee Liau
committed
feat: Bundle @angular/language-service with server
The initial idea of the new extension is to always load `@angular/language-service` from an Angular project that has the package installed. That is why it does not bundle the package with the server. However, `@angular/language-service` is not always available, due to a few reasons: 1. The initial directory opened is not the root of an Angular project. 2. When spawned as pure LSP server, the process CWD is not where @angular/langauge-service is installed. Besides that, users have no control over which @angular/language-service version they want the extension to use. (TypeScript extension provides this feature). It is also surprising to users that the extension does nothing if @angular/language-service is not found. For the reasons above, it is better that the server bundles a fallback version of @angular/language-service, but still provides users with an option to specify the location via `--pluginProbeLocation` cmd line flag. PR closes #369
1 parent c30ff7b commit 87b119a

File tree

7 files changed

+147
-9
lines changed

7 files changed

+147
-9
lines changed

client/src/extension.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ export function activate(context: vscode.ExtensionContext) {
1717
// Log file does not yet exist on disk. It is up to the server to create the
1818
// file.
1919
const logFile = path.join(context.logPath, 'nglangsvc.log');
20+
const pluginProbeLocation = context.asAbsolutePath('server');
2021

2122
// If the extension is launched in debug mode then the debug server options are used
2223
// Otherwise the run options are used
@@ -25,8 +26,11 @@ export function activate(context: vscode.ExtensionContext) {
2526
module: context.asAbsolutePath(path.join('server', 'server.js')),
2627
transport: lsp.TransportKind.ipc,
2728
args: [
28-
'--logFile', logFile,
29+
'--logFile',
30+
logFile,
2931
// TODO: Might want to turn off logging completely.
32+
'--pluginProbeLocation',
33+
pluginProbeLocation,
3034
],
3135
options: {
3236
env: {
@@ -43,6 +47,8 @@ export function activate(context: vscode.ExtensionContext) {
4347
logFile,
4448
'--logVerbosity',
4549
'verbose',
50+
'--pluginProbeLocation',
51+
pluginProbeLocation,
4652
],
4753
options: {
4854
env: {

server/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
"node": "*"
1010
},
1111
"dependencies": {
12+
"@angular/language-service": "^9.0.0-next.7",
1213
"vscode-languageserver": "~5.2.1",
1314
"vscode-uri": "^2.0.3"
1415
}

server/src/project_service.ts

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import * as lsp from 'vscode-languageserver';
1212
import {tsDiagnosticToLspDiagnostic} from './diagnostic';
1313
import {projectLoadingNotification} from './protocol';
1414
import {filePathToUri} from './utils';
15+
import {NgVersionProvider} from './version_provider';
1516

1617
// NOTE:
1718
// There are three types of `project`:
@@ -43,9 +44,7 @@ export class ProjectService {
4344
private readonly connection: lsp.IConnection,
4445
options: Map<string, string>,
4546
) {
46-
const pluginProbeLocation =
47-
options.get('pluginProbeLocation') || serverHost.getCurrentDirectory();
48-
connection.console.info(`Angular LS probe location: ${pluginProbeLocation}`);
47+
const pluginProbeLocation = options.get('pluginProbeLocation');
4948
// TODO: Should load TypeScript from workspace.
5049
this.tsProjSvc = new ts.server.ProjectService({
5150
host: serverHost,
@@ -57,7 +56,7 @@ export class ProjectService {
5756
suppressDiagnosticEvents: false,
5857
eventHandler: (e) => this.handleProjectServiceEvent(e),
5958
globalPlugins: ['@angular/language-service'],
60-
pluginProbeLocations: [pluginProbeLocation],
59+
pluginProbeLocations: this.getPluginProbeLocations(pluginProbeLocation),
6160
allowLocalPluginLoads: false, // do not load plugins from tsconfig.json
6261
});
6362

@@ -172,4 +171,33 @@ export class ProjectService {
172171
});
173172
}
174173
}
174+
175+
/**
176+
* Determine valid directories where `@angular/language-service` could be
177+
* found.
178+
* @param probeLocation
179+
*/
180+
private getPluginProbeLocations(probeLocation?: string): string[] {
181+
const versionProvider = new NgVersionProvider(probeLocation);
182+
const pluginProbeLocations = [];
183+
const localVersion = versionProvider.localVersion;
184+
if (localVersion) {
185+
pluginProbeLocations.push(localVersion.dirName);
186+
}
187+
const bundledVersion = versionProvider.bundledVersion;
188+
if (bundledVersion) {
189+
pluginProbeLocations.push(bundledVersion.dirName);
190+
}
191+
if (pluginProbeLocations.length) {
192+
this.connection.console.info(
193+
`Angular LS probe locations: [${pluginProbeLocations.join(', ')}]`);
194+
} else {
195+
let msg = 'Failed to find @angular/language service';
196+
if (probeLocation) {
197+
msg += ` in ${probeLocation}`;
198+
}
199+
this.connection.console.error(msg);
200+
}
201+
return pluginProbeLocations;
202+
}
175203
}

server/src/server.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,13 @@ enum LanguageId {
2323
// Parse startup options
2424
const options = new Map<string, string>();
2525
for (let i = 0; i < process.argv.length; ++i) {
26-
const argv = process.argv[i];
27-
if (argv === '--logFile') {
26+
const arg = process.argv[i];
27+
if (arg === '--logFile') {
2828
options.set('logFile', process.argv[i + 1]);
29-
}
30-
if (argv === '--logVerbosity') {
29+
} else if (arg === '--logVerbosity') {
3130
options.set('logVerbosity', process.argv[i + 1]);
31+
} else if (arg === '--pluginProbeLocation') {
32+
options.set('pluginProbeLocation', process.argv[i + 1]);
3233
}
3334
}
3435

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/**
2+
* @license
3+
* Copyright Google Inc. All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
import {NgVersionProvider} from '../version_provider';
10+
11+
describe('NgVersionProvider', () => {
12+
const probeLocation = __dirname;
13+
14+
it('should find bundled version', () => {
15+
const provider = new NgVersionProvider(probeLocation);
16+
const bundledVersion = provider.bundledVersion;
17+
expect(bundledVersion).toBeDefined();
18+
const {dirName, version} = bundledVersion!;
19+
expect(dirName).toMatch(/@angular\/language-service$/);
20+
expect(version).toBeTruthy();
21+
});
22+
23+
it('should not find local version', () => {
24+
const provider = new NgVersionProvider(probeLocation);
25+
const localVersion = provider.localVersion;
26+
// Don't expect to find `@angular/language-service` in current directory.
27+
expect(localVersion).toBeUndefined();
28+
});
29+
});

server/src/version_provider.ts

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
/**
2+
* @license
3+
* Copyright Google Inc. All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
import * as path from 'path';
10+
11+
/**
12+
* Represents a valid directory where `@angular/language-service` could be
13+
* found, as well as its version.
14+
*/
15+
interface NgVersion {
16+
dirName: string;
17+
version?: string;
18+
}
19+
20+
const NGLANGSVC = `@angular/language-service/package.json`;
21+
22+
export class NgVersionProvider {
23+
constructor(private readonly probeLocation?: string) {}
24+
25+
/**
26+
* Return the version that is found via the probe location, if provided.
27+
*/
28+
get bundledVersion(): NgVersion|undefined {
29+
if (!this.probeLocation) {
30+
return;
31+
}
32+
return this.resolve(this.probeLocation);
33+
}
34+
35+
/**
36+
* Return the version that is found via current directory, if any.
37+
*/
38+
get localVersion(): NgVersion|undefined {
39+
return this.resolve(process.cwd());
40+
}
41+
42+
private resolve(dirName: string): NgVersion|undefined {
43+
// Here, use native NodeJS require instead of ServerHost.require because
44+
// we want the full path of the resolution provided by native
45+
// `require.resolve()`, which ServerHost does not provide.
46+
let result: NgVersion|undefined;
47+
try {
48+
// require.resolve() throws if module resolution fails.
49+
const resolutionPath = require.resolve(NGLANGSVC, {
50+
paths: [dirName],
51+
});
52+
if (!resolutionPath) {
53+
return;
54+
}
55+
result = {
56+
dirName: path.dirname(resolutionPath),
57+
version: undefined,
58+
};
59+
// require would throw if package.json is not strict JSON
60+
const packageJson = require(resolutionPath);
61+
if (packageJson && packageJson.version) {
62+
result.version = packageJson.version;
63+
}
64+
} finally {
65+
return result;
66+
}
67+
}
68+
}

server/yarn.lock

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,11 @@
22
# yarn lockfile v1
33

44

5+
"@angular/language-service@^9.0.0-next.7":
6+
version "9.0.0-next.7"
7+
resolved "https://registry.yarnpkg.com/@angular/language-service/-/language-service-9.0.0-next.7.tgz#101820e30d7f16f9ca68004aa5c9fed5b8d66a1c"
8+
integrity sha512-tw/zJBxJJjKWoDoF/Bm7HZGP9IQppf7Ettajhn+JSbbVh+IN1vh3eA9HPkSt4mQotdHUxdMHZ1ZV1E7W5XqgTA==
9+
510
vscode-jsonrpc@^4.0.0:
611
version "4.0.0"
712
resolved "https://registry.yarnpkg.com/vscode-jsonrpc/-/vscode-jsonrpc-4.0.0.tgz#a7bf74ef3254d0a0c272fab15c82128e378b3be9"

0 commit comments

Comments
 (0)