-
Notifications
You must be signed in to change notification settings - Fork 122
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
perf: prevent requests from being sent to the sesrver when outside An…
…gular context The Angular Language Service extension of vscode has several pieces to it: * The client, which communicates directly with vscode * The server, which handles LSP requests over an lsp connection * @angular/language-service which provides concrete answers to LS queries We added an optimization the @angular/language-service which exits early when a request is outside the Angular context. This prevents unnecessary Angular analysis of a file when we know from the start that there are no Angular-specific results at a location. This commit provides an additional optimization by adding a similar preventing short-circuit from the client side. This prevents requests from even being sent to the server when we know there is no Angular information at a location. This optimization is a necessary addition because the server can be blocked from processing requests if another one is taking a while to respond. We found that requests for diagnostics, which are necessary when opening a new file, block other server requests from being processed. This commit would prevent that block from happening by never making the request to the server in the first place. fixes #1176
- Loading branch information
Showing
9 changed files
with
321 additions
and
18 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| /** | ||
| * @license | ||
| * Copyright Google Inc. All Rights Reserved. | ||
| * | ||
| * Use of this source code is governed by an MIT-style license that can be | ||
| * found in the LICENSE file at https://angular.io/license | ||
| */ | ||
| import * as ts from 'typescript'; | ||
| import * as vscode from 'vscode'; | ||
|
|
||
| /** Determines if the position is inside an inline template. */ | ||
| export function isInsideInlineTemplateRegion( | ||
| document: vscode.TextDocument, position: vscode.Position): boolean { | ||
| if (document.languageId !== 'typescript') { | ||
| return true; | ||
| } | ||
| return isPropertyAssignmentToStringOrStringInArray( | ||
| document.getText(), document.offsetAt(position), ['template']); | ||
| } | ||
|
|
||
| /** Determines if the position is inside an inline template, templateUrl, or string in styleUrls. */ | ||
| export function isInsideComponentDecorator( | ||
| document: vscode.TextDocument, position: vscode.Position): boolean { | ||
| if (document.languageId !== 'typescript') { | ||
| return true; | ||
| } | ||
| return isPropertyAssignmentToStringOrStringInArray( | ||
| document.getText(), document.offsetAt(position), ['template', 'templateUrl', 'styleUrls']); | ||
| } | ||
|
|
||
| /** | ||
| * Basic scanner to determine if we're inside a string of a property with one of the given names. | ||
| * | ||
| * This scanner is not currently robust or perfect but provides us with an accurate answer _most_ of | ||
| * the time. | ||
| * | ||
| * False positives are OK here. Though this will give some false positives for determining if a | ||
| * position is within an Angular context, i.e. an object like `{template: ''}` that is not inside an | ||
| * `@Component` or `{styleUrls: [someFunction('stringL¦iteral')]}`, the @angular/language-service | ||
| * will always give us the correct answer. This helper gives us a quick win for optimizing the | ||
| * number of requests we send to the server. | ||
| */ | ||
| function isPropertyAssignmentToStringOrStringInArray( | ||
| documentText: string, offset: number, propertyAssignmentNames: string[]): boolean { | ||
| const scanner = ts.createScanner(ts.ScriptTarget.ESNext, true /* skipTrivia */); | ||
| scanner.setText(documentText); | ||
|
|
||
| let token: ts.SyntaxKind = scanner.scan(); | ||
| let lastToken: ts.SyntaxKind|undefined; | ||
| let lastTokenText: string|undefined; | ||
| let unclosedBraces = 0; | ||
| let unclosedBrackets = 0; | ||
| let propertyAssignmentContext = false; | ||
| while (token !== ts.SyntaxKind.EndOfFileToken && scanner.getStartPos() < offset) { | ||
| if (lastToken === ts.SyntaxKind.Identifier && lastTokenText !== undefined && | ||
| propertyAssignmentNames.includes(lastTokenText) && token === ts.SyntaxKind.ColonToken) { | ||
| propertyAssignmentContext = true; | ||
| token = scanner.scan(); | ||
| continue; | ||
| } | ||
| if (unclosedBraces === 0 && unclosedBrackets === 0 && isPropertyAssignmentTerminator(token)) { | ||
| propertyAssignmentContext = false; | ||
| } | ||
|
|
||
| if (token === ts.SyntaxKind.OpenBracketToken) { | ||
| unclosedBrackets++; | ||
| } else if (token === ts.SyntaxKind.OpenBraceToken) { | ||
| unclosedBraces++; | ||
| } else if (token === ts.SyntaxKind.CloseBracketToken) { | ||
| unclosedBrackets--; | ||
| } else if (token === ts.SyntaxKind.CloseBraceToken) { | ||
| unclosedBraces--; | ||
| } | ||
|
|
||
| const isStringToken = token === ts.SyntaxKind.StringLiteral || | ||
| token === ts.SyntaxKind.NoSubstitutionTemplateLiteral; | ||
| const isCursorInToken = scanner.getStartPos() <= offset && | ||
| scanner.getStartPos() + scanner.getTokenText().length >= offset; | ||
| if (propertyAssignmentContext && isCursorInToken && isStringToken) { | ||
| return true; | ||
| } | ||
|
|
||
| lastTokenText = scanner.getTokenText(); | ||
| lastToken = token; | ||
| token = scanner.scan(); | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| function isPropertyAssignmentTerminator(token: ts.SyntaxKind) { | ||
| return token === ts.SyntaxKind.EndOfFileToken || token === ts.SyntaxKind.CommaToken || | ||
| token === ts.SyntaxKind.SemicolonToken || token === ts.SyntaxKind.CloseBraceToken; | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,115 @@ | ||
| /** | ||
| * @license | ||
| * Copyright Google Inc. All Rights Reserved. | ||
| * | ||
| * Use of this source code is governed by an MIT-style license that can be | ||
| * found in the LICENSE file at https://angular.io/license | ||
| */ | ||
| import * as vscode from 'vscode'; | ||
| import {DocumentUri, TextDocument} from 'vscode-languageserver-textdocument'; | ||
|
|
||
| import {isInsideComponentDecorator, isInsideInlineTemplateRegion} from '../embedded_support'; | ||
|
|
||
| describe('embedded language support', () => { | ||
| describe('isInsideInlineTemplateRegion', () => { | ||
| it('empty file', () => { | ||
| test('¦', isInsideInlineTemplateRegion, false); | ||
| }); | ||
|
|
||
| it('just after template', () => { | ||
| test(`template: '<div></div>'¦`, isInsideInlineTemplateRegion, false); | ||
| }); | ||
|
|
||
| it('just before template', () => { | ||
| // Note that while it seems that this should be `false`, we should still consider this inside | ||
| // the string because the visual mode of vim appears to have a position on top of the open | ||
| // quote while the cursor position is before it. | ||
| test(`template: ¦'<div></div>'`, isInsideInlineTemplateRegion, true); | ||
| }); | ||
|
|
||
| it('two spaces before template', () => { | ||
| test(`template:¦ '<div></div>'`, isInsideInlineTemplateRegion, false); | ||
| }); | ||
|
|
||
| it('at beginning of template', () => { | ||
| test(`template: '¦<div></div>'`, isInsideInlineTemplateRegion, true); | ||
| }); | ||
|
|
||
| it('at end of template', () => { | ||
| test(`template: '<div></div>¦'`, isInsideInlineTemplateRegion, true); | ||
| }); | ||
| }); | ||
|
|
||
| describe('isInsideAngularContext', () => { | ||
| it('empty file', () => { | ||
| test('¦', isInsideComponentDecorator, false); | ||
| }); | ||
|
|
||
| it('just after template', () => { | ||
| test(`template: '<div></div>'¦`, isInsideComponentDecorator, false); | ||
| }); | ||
|
|
||
| it('inside template', () => { | ||
| test(`template: '<div>¦</div>'`, isInsideComponentDecorator, true); | ||
| }); | ||
|
|
||
| it('just after templateUrl', () => { | ||
| test(`templateUrl: './abc.html'¦`, isInsideComponentDecorator, false); | ||
| }); | ||
|
|
||
| it('inside templateUrl', () => { | ||
| test(`templateUrl: './abc¦.html'`, isInsideComponentDecorator, true); | ||
| }); | ||
|
|
||
| it('just after styleUrls', () => { | ||
| test(`styleUrls: ['./abc.css']¦`, isInsideComponentDecorator, false); | ||
| }); | ||
|
|
||
| it('inside first item of styleUrls', () => { | ||
| test(`styleUrls: ['./abc.c¦ss', 'def.css']`, isInsideComponentDecorator, true); | ||
| }); | ||
|
|
||
| it('inside second item of styleUrls', () => { | ||
| test(`styleUrls: ['./abc.css', 'def¦.css']`, isInsideComponentDecorator, true); | ||
| }); | ||
|
|
||
| it('inside second item of styleUrls, when first is complicated function', () => { | ||
| test( | ||
| `styleUrls: [getCss({strict: true, dirs: ['apple', 'banana']}), 'def¦.css']`, | ||
| isInsideComponentDecorator, true); | ||
| }); | ||
|
|
||
| it('inside non-string item of styleUrls', () => { | ||
| test( | ||
| `styleUrls: [getCss({strict: true¦, dirs: ['apple', 'banana']}), 'def.css']`, | ||
| isInsideComponentDecorator, false); | ||
| }); | ||
| }); | ||
| }); | ||
|
|
||
| function test( | ||
| fileWithCursor: string, | ||
| testFn: (doc: vscode.TextDocument, position: vscode.Position) => boolean, | ||
| expectation: boolean): void { | ||
| const {cursor, text} = extractCursorInfo(fileWithCursor); | ||
| const vdoc = TextDocument.create('test' as DocumentUri, 'typescript', 0, text) as {} as | ||
| vscode.TextDocument; | ||
| const actual = testFn(vdoc, vdoc.positionAt(cursor)); | ||
| expect(actual).toBe(expectation); | ||
| } | ||
|
|
||
| /** | ||
| * Given a text snippet which contains exactly one cursor symbol ('¦'), extract both the offset of | ||
| * that cursor within the text as well as the text snippet without the cursor. | ||
| */ | ||
| function extractCursorInfo(textWithCursor: string): {cursor: number, text: string} { | ||
| const cursor = textWithCursor.indexOf('¦'); | ||
| if (cursor === -1 || textWithCursor.indexOf('¦', cursor + 1) !== -1) { | ||
| throw new Error(`Expected to find exactly one cursor symbol '¦'`); | ||
| } | ||
|
|
||
| return { | ||
| cursor, | ||
| text: textWithCursor.substr(0, cursor) + textWithCursor.substr(cursor + 1), | ||
| }; | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| { | ||
| "extends": "../../../tsconfig.json", | ||
| "compilerOptions": { | ||
| "outDir": "../../../dist/client/tests" | ||
| }, | ||
| "references": [ | ||
| { | ||
| "path": "../../tsconfig.json" | ||
| } | ||
| ], | ||
| "include": [ | ||
| "*.ts" | ||
| ] | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,23 @@ | ||
| { | ||
| "extends": "../tsconfig.json", | ||
| "compilerOptions": { | ||
| "composite": true, | ||
| "outDir": "../dist/client", | ||
| "rootDirs": [".", "../dist"] | ||
| "rootDir": "src", | ||
| "rootDirs": [ | ||
| ".", | ||
| "../dist" | ||
| ] | ||
| }, | ||
| "references": [ | ||
| {"path": "../common"} | ||
| { | ||
| "path": "../common" | ||
| } | ||
| ], | ||
| "include": ["src"], | ||
| "exclude": ["node_modules"] | ||
| } | ||
| "include": [ | ||
| "src" | ||
| ], | ||
| "exclude": [ | ||
| "node_modules" | ||
| ] | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| { | ||
| "spec_dir": "dist", | ||
| "spec_files": [ | ||
| "client/tests/*_spec.js", | ||
| "server/tests/*_spec.js" | ||
| ] | ||
| } |
Oops, something went wrong.