Skip to content

Commit da3fbc7

Browse files
atscottatscott
committed
perf: prevent requests from being sent to the server when outside Angular context
This reverts commit fcbdf93 (revert of the revert).
1 parent 1223b3a commit da3fbc7

File tree

9 files changed

+306
-16
lines changed

9 files changed

+306
-16
lines changed

‎.vscode/launch.json

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,10 @@
99
"runtimeExecutable": "${execPath}",
1010
"args": [
1111
"--disable-extensions",
12-
"--extensionDevelopmentPath=${workspaceRoot}"
12+
"--extensionDevelopmentPath=${workspaceFolder}"
1313
],
1414
"outFiles": [
15-
"${workspaceRoot}/dist/client/*.js"
15+
"${workspaceFolder}/dist/client/*.js"
1616
],
1717
"preLaunchTask": {
1818
"type": "npm",
@@ -26,9 +26,19 @@
2626
"port": 6009,
2727
"restart": true,
2828
"outFiles": [
29-
"${workspaceRoot}/dist/server/*.js"
29+
"${workspaceFolder}/dist/server/*.js"
3030
]
3131
},
32+
{
33+
"type": "node",
34+
"request": "attach",
35+
"name": "Attach to Jasmine",
36+
"port": 9229,
37+
"restart": true,
38+
"outFiles": [
39+
"${workspaceFolder}/dist/**/*.js"
40+
],
41+
},
3242
{
3343
"name": "Integration test: Attach to server",
3444
"port": 9330,
@@ -37,7 +47,7 @@
3747
"<node_internals>/**"
3848
],
3949
"outFiles": [
40-
"${workspaceRoot}/dist/integration/lsp/*.js"
50+
"${workspaceFolder}/dist/integration/lsp/*.js"
4151
],
4252
"type": "node"
4353
},
@@ -47,12 +57,12 @@
4757
"request": "launch",
4858
"runtimeExecutable": "${execPath}",
4959
"args": [
50-
"--extensionDevelopmentPath=${workspaceRoot}",
51-
"--extensionTestsPath=${workspaceRoot}/dist/integration/e2e",
52-
"${workspaceRoot}/integration/project"
60+
"--extensionDevelopmentPath=${workspaceFolder}",
61+
"--extensionTestsPath=${workspaceFolder}/dist/integration/e2e",
62+
"${workspaceFolder}/integration/project"
5363
],
5464
"outFiles": [
55-
"${workspaceRoot}/dist/integration/e2e/*.js"
65+
"${workspaceFolder}/dist/integration/e2e/*.js"
5666
],
5767
"preLaunchTask": {
5868
"type": "npm",

‎client/src/client.ts

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {ProjectLoadingFinish, ProjectLoadingStart, SuggestIvyLanguageService, Su
1515
import {NgccProgress, NgccProgressToken, NgccProgressType} from '../common/progress';
1616
import {GetTcbRequest} from '../common/requests';
1717

18+
import {isInsideComponentDecorator, isInsideInlineTemplateRegion} from './embedded_support';
1819
import {ProgressReporter} from './progress-reporter';
1920

2021
interface GetTcbResponse {
@@ -50,6 +51,37 @@ export class AngularLanguageClient implements vscode.Disposable {
5051
// Don't let our output console pop open
5152
revealOutputChannelOn: lsp.RevealOutputChannelOn.Never,
5253
outputChannel: this.outputChannel,
54+
middleware: {
55+
provideDefinition: async (
56+
document: vscode.TextDocument, position: vscode.Position,
57+
token: vscode.CancellationToken, next: lsp.ProvideDefinitionSignature) => {
58+
if (isInsideComponentDecorator(document, position)) {
59+
return next(document, position, token);
60+
}
61+
},
62+
provideTypeDefinition: async (
63+
document: vscode.TextDocument, position: vscode.Position,
64+
token: vscode.CancellationToken, next) => {
65+
if (isInsideInlineTemplateRegion(document, position)) {
66+
return next(document, position, token);
67+
}
68+
},
69+
provideHover: async (
70+
document: vscode.TextDocument, position: vscode.Position,
71+
token: vscode.CancellationToken, next: lsp.ProvideHoverSignature) => {
72+
if (isInsideInlineTemplateRegion(document, position)) {
73+
return next(document, position, token);
74+
}
75+
},
76+
provideCompletionItem: async (
77+
document: vscode.TextDocument, position: vscode.Position,
78+
context: vscode.CompletionContext, token: vscode.CancellationToken,
79+
next: lsp.ProvideCompletionItemsSignature) => {
80+
if (isInsideInlineTemplateRegion(document, position)) {
81+
return next(document, position, context, token);
82+
}
83+
}
84+
}
5385
};
5486
}
5587

@@ -312,4 +344,4 @@ function getServerOptions(ctx: vscode.ExtensionContext, debug: boolean): lsp.Nod
312344
execArgv: debug ? devExecArgv : prodExecArgv,
313345
},
314346
};
315-
}
347+
}

‎client/src/embedded_support.ts

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
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+
import * as ts from 'typescript';
9+
import * as vscode from 'vscode';
10+
11+
/** Determines if the position is inside an inline template. */
12+
export function isInsideInlineTemplateRegion(
13+
document: vscode.TextDocument, position: vscode.Position): boolean {
14+
if (document.languageId !== 'typescript') {
15+
return true;
16+
}
17+
return isPropertyAssignmentToStringOrStringInArray(
18+
document.getText(), document.offsetAt(position), ['template']);
19+
}
20+
21+
/** Determines if the position is inside an inline template, templateUrl, or string in styleUrls. */
22+
export function isInsideComponentDecorator(
23+
document: vscode.TextDocument, position: vscode.Position): boolean {
24+
if (document.languageId !== 'typescript') {
25+
return true;
26+
}
27+
return isPropertyAssignmentToStringOrStringInArray(
28+
document.getText(), document.offsetAt(position), ['template', 'templateUrl', 'styleUrls']);
29+
}
30+
31+
/**
32+
* Basic scanner to determine if we're inside a string of a property with one of the given names.
33+
*
34+
* This scanner is not currently robust or perfect but provides us with an accurate answer _most_ of
35+
* the time.
36+
*
37+
* False positives are OK here. Though this will give some false positives for determining if a
38+
* position is within an Angular context, i.e. an object like `{template: ''}` that is not inside an
39+
* `@Component` or `{styleUrls: [someFunction('stringL¦iteral')]}`, the @angular/language-service
40+
* will always give us the correct answer. This helper gives us a quick win for optimizing the
41+
* number of requests we send to the server.
42+
*/
43+
function isPropertyAssignmentToStringOrStringInArray(
44+
documentText: string, offset: number, propertyAssignmentNames: string[]): boolean {
45+
const scanner = ts.createScanner(ts.ScriptTarget.ESNext, true /* skipTrivia */);
46+
scanner.setText(documentText);
47+
48+
let token: ts.SyntaxKind = scanner.scan();
49+
let lastToken: ts.SyntaxKind|undefined;
50+
let lastTokenText: string|undefined;
51+
let unclosedBraces = 0;
52+
let unclosedBrackets = 0;
53+
let propertyAssignmentContext = false;
54+
while (token !== ts.SyntaxKind.EndOfFileToken && scanner.getStartPos() < offset) {
55+
if (lastToken === ts.SyntaxKind.Identifier && lastTokenText !== undefined &&
56+
propertyAssignmentNames.includes(lastTokenText) && token === ts.SyntaxKind.ColonToken) {
57+
propertyAssignmentContext = true;
58+
token = scanner.scan();
59+
continue;
60+
}
61+
if (unclosedBraces === 0 && unclosedBrackets === 0 && isPropertyAssignmentTerminator(token)) {
62+
propertyAssignmentContext = false;
63+
}
64+
65+
if (token === ts.SyntaxKind.OpenBracketToken) {
66+
unclosedBrackets++;
67+
} else if (token === ts.SyntaxKind.OpenBraceToken) {
68+
unclosedBraces++;
69+
} else if (token === ts.SyntaxKind.CloseBracketToken) {
70+
unclosedBrackets--;
71+
} else if (token === ts.SyntaxKind.CloseBraceToken) {
72+
unclosedBraces--;
73+
}
74+
75+
const isStringToken = token === ts.SyntaxKind.StringLiteral ||
76+
token === ts.SyntaxKind.NoSubstitutionTemplateLiteral;
77+
const isCursorInToken = scanner.getStartPos() <= offset &&
78+
scanner.getStartPos() + scanner.getTokenText().length >= offset;
79+
if (propertyAssignmentContext && isCursorInToken && isStringToken) {
80+
return true;
81+
}
82+
83+
lastTokenText = scanner.getTokenText();
84+
lastToken = token;
85+
token = scanner.scan();
86+
}
87+
88+
return false;
89+
}
90+
91+
function isPropertyAssignmentTerminator(token: ts.SyntaxKind) {
92+
return token === ts.SyntaxKind.EndOfFileToken || token === ts.SyntaxKind.CommaToken ||
93+
token === ts.SyntaxKind.SemicolonToken || token === ts.SyntaxKind.CloseBraceToken;
94+
}

‎client/src/tests/embedded_support_spec.ts

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
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+
import * as vscode from 'vscode';
9+
import {DocumentUri, TextDocument} from 'vscode-languageserver-textdocument';
10+
11+
import {isInsideComponentDecorator, isInsideInlineTemplateRegion} from '../embedded_support';
12+
13+
describe('embedded language support', () => {
14+
describe('isInsideInlineTemplateRegion', () => {
15+
it('empty file', () => {
16+
test('¦', isInsideInlineTemplateRegion, false);
17+
});
18+
19+
it('just after template', () => {
20+
test(`template: '<div></div>'¦`, isInsideInlineTemplateRegion, false);
21+
});
22+
23+
it('just before template', () => {
24+
// Note that while it seems that this should be `false`, we should still consider this inside
25+
// the string because the visual mode of vim appears to have a position on top of the open
26+
// quote while the cursor position is before it.
27+
test(`template: ¦'<div></div>'`, isInsideInlineTemplateRegion, true);
28+
});
29+
30+
it('two spaces before template', () => {
31+
test(`template:¦ '<div></div>'`, isInsideInlineTemplateRegion, false);
32+
});
33+
34+
it('at beginning of template', () => {
35+
test(`template: '¦<div></div>'`, isInsideInlineTemplateRegion, true);
36+
});
37+
38+
it('at end of template', () => {
39+
test(`template: '<div></div>¦'`, isInsideInlineTemplateRegion, true);
40+
});
41+
});
42+
43+
describe('isInsideAngularContext', () => {
44+
it('empty file', () => {
45+
test('¦', isInsideComponentDecorator, false);
46+
});
47+
48+
it('just after template', () => {
49+
test(`template: '<div></div>'¦`, isInsideComponentDecorator, false);
50+
});
51+
52+
it('inside template', () => {
53+
test(`template: '<div>¦</div>'`, isInsideComponentDecorator, true);
54+
});
55+
56+
it('just after templateUrl', () => {
57+
test(`templateUrl: './abc.html'¦`, isInsideComponentDecorator, false);
58+
});
59+
60+
it('inside templateUrl', () => {
61+
test(`templateUrl: './abc¦.html'`, isInsideComponentDecorator, true);
62+
});
63+
64+
it('just after styleUrls', () => {
65+
test(`styleUrls: ['./abc.css']¦`, isInsideComponentDecorator, false);
66+
});
67+
68+
it('inside first item of styleUrls', () => {
69+
test(`styleUrls: ['./abc.c¦ss', 'def.css']`, isInsideComponentDecorator, true);
70+
});
71+
72+
it('inside second item of styleUrls', () => {
73+
test(`styleUrls: ['./abc.css', 'def¦.css']`, isInsideComponentDecorator, true);
74+
});
75+
76+
it('inside second item of styleUrls, when first is complicated function', () => {
77+
test(
78+
`styleUrls: [getCss({strict: true, dirs: ['apple', 'banana']}), 'def¦.css']`,
79+
isInsideComponentDecorator, true);
80+
});
81+
82+
it('inside non-string item of styleUrls', () => {
83+
test(
84+
`styleUrls: [getCss({strict: true¦, dirs: ['apple', 'banana']}), 'def.css']`,
85+
isInsideComponentDecorator, false);
86+
});
87+
});
88+
});
89+
90+
function test(
91+
fileWithCursor: string,
92+
testFn: (doc: vscode.TextDocument, position: vscode.Position) => boolean,
93+
expectation: boolean): void {
94+
const {cursor, text} = extractCursorInfo(fileWithCursor);
95+
const vdoc = TextDocument.create('test' as DocumentUri, 'typescript', 0, text) as {} as
96+
vscode.TextDocument;
97+
const actual = testFn(vdoc, vdoc.positionAt(cursor));
98+
expect(actual).toBe(expectation);
99+
}
100+
101+
/**
102+
* Given a text snippet which contains exactly one cursor symbol ('¦'), extract both the offset of
103+
* that cursor within the text as well as the text snippet without the cursor.
104+
*/
105+
function extractCursorInfo(textWithCursor: string): {cursor: number, text: string} {
106+
const cursor = textWithCursor.indexOf('¦');
107+
if (cursor === -1 || textWithCursor.indexOf('¦', cursor + 1) !== -1) {
108+
throw new Error(`Expected to find exactly one cursor symbol '¦'`);
109+
}
110+
111+
return {
112+
cursor,
113+
text: textWithCursor.substr(0, cursor) + textWithCursor.substr(cursor + 1),
114+
};
115+
}

‎client/src/tests/tsconfig.json

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
{
2+
"extends": "../../../tsconfig.json",
3+
"compilerOptions": {
4+
"outDir": "../../../dist/client/tests"
5+
},
6+
"references": [
7+
{
8+
"path": "../../tsconfig.json"
9+
}
10+
],
11+
"include": [
12+
"*.ts"
13+
]
14+
}

‎client/tsconfig.json

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,23 @@
11
{
22
"extends": "../tsconfig.json",
33
"compilerOptions": {
4+
"composite": true,
45
"outDir": "../dist/client",
5-
"rootDirs": [".", "../dist"]
6+
"rootDir": "src",
7+
"rootDirs": [
8+
".",
9+
"../dist"
10+
]
611
},
712
"references": [
8-
{"path": "../common"}
13+
{
14+
"path": "../common"
15+
}
916
],
10-
"include": ["src"],
11-
"exclude": ["node_modules"]
12-
}
17+
"include": [
18+
"src"
19+
],
20+
"exclude": [
21+
"node_modules"
22+
]
23+
}

‎jasmine.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"spec_dir": "dist",
3+
"spec_files": [
4+
"client/tests/*_spec.js",
5+
"server/tests/*_spec.js"
6+
]
7+
}

0 commit comments

Comments
 (0)