Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Syntax only server creates inferred project with all the open files w…#38561

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
sheetalkamat merged 8 commits intomasterfromsyntaxAsSemanticServer
Jun 16, 2020

Conversation

sheetalkamat
Copy link
Member

…ith noResolve and can handle semantic operations

Fixes #

…ith noResolve and can handle semantic operations
@sheetalkamat
Copy link
MemberAuthor

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commentedMay 13, 2020
edited
Loading

Heya@sheetalkamat, I've started to run the tarball bundle task on this PR at5726838. You can monitor the buildhere.

@sheetalkamat
Copy link
MemberAuthor

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commentedMay 13, 2020
edited
Loading

Heya@sheetalkamat, I've started to run the tarball bundle task on this PR at90d8a96. You can monitor the buildhere.

@typescript-bot
Copy link
Collaborator

typescript-bot commentedMay 14, 2020
edited
Loading

Hey@sheetalkamat, I've packed this intoan installable tgz. You can install it for testing by referencing it in yourpackage.json like so:

{    "devDependencies": {        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/73860/artifacts?artifactName=tgz&fileId=2BC1D5FE49C2AD5CCA135D57CEE4CA308A5E08ADDD7B5A264EDC56EE6D487BE002&fileName=/typescript-4.0.0-insiders.20200514.tgz"    }}

and then runningnpm install.


There is also a playgroundfor this build.

@sheetalkamat
Copy link
MemberAuthor

@mjbvz@minestarks@uniqueiniquity@amcasey@DanielRosenwasser@RyanCavanaugh
#38561 (comment) is build where syntaxOnly server can accept semantic commands.. ( i havent added any custom changes for any semantic commands nor have i limited any semantic operations as part of this prototype)

#38564 is PR that is on top of this and makes normal tsserver behave like syntaxOnly (one with semantic operations allowed)

DanielRosenwasser reacted with heart emoji

@sheetalkamatsheetalkamat mentioned this pull requestMay 14, 2020
@amcasey
Copy link
Member

I'll be interested to see what this does to startup perf on the syntax server. Looks pretty cool though.

@amcasey
Copy link
Member

I've run into a pretty big issue on the VS side: semantic operations tend to be triggered for Roslyn Documents and those don't exist until the project is loaded. So I see operations during project loadin already open documents, but not in the newly opened document (i.e. the one that triggered the project load). Obviously, this has nothing to do with the tsserver change.

@DanielRosenwasser
Copy link
Member

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commentedJun 9, 2020
edited
Loading

Heya@DanielRosenwasser, I've started to run the tarball bundle task on this PR at90d8a96. You can monitor the buildhere.

@typescript-bot
Copy link
Collaborator

typescript-bot commentedJun 9, 2020
edited
Loading

Hey@DanielRosenwasser, I've packed this intoan installable tgz. You can install it for testing by referencing it in yourpackage.json like so:

{    "devDependencies": {        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/75966/artifacts?artifactName=tgz&fileId=BB849547A1DF11A54E5245D89DF5DAFE50EAC3E3C08A813981103A387395A0B802&fileName=/typescript-4.0.0-insiders.20200609.tgz"    }}

and then runningnpm install.


There is also a playgroundfor this build.

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commentedJun 9, 2020
edited
Loading

I'm not able to see things working better with

@DanielRosenwasser
Copy link
Member

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commentedJun 11, 2020
edited
Loading

Heya@DanielRosenwasser, I've started to run the tarball bundle task on this PR at90d8a96. You can monitor the buildhere.

@typescript-bot
Copy link
Collaborator

typescript-bot commentedJun 11, 2020
edited
Loading

Hey@DanielRosenwasser, I've packed this intoan installable tgz. You can install it for testing by referencing it in yourpackage.json like so:

{    "devDependencies": {        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/76193/artifacts?artifactName=tgz&fileId=0AD85EA4EF3D7253C964378CA736CCDB891DD80E981648EE08731F97511241AD02&fileName=/typescript-4.0.0-insiders.20200611.tgz"    }}

and then runningnpm install.


There is also a playgroundfor this build.

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commentedJun 11, 2020
edited
Loading

I'm finally trying this out with VS Code Insiders! Pretty cool! A couple of thoughts and ideas

From the editor side:

  • the startup time still isn't instantaneous for the syntax server, so I'm really never sure whether I can start editing reliably until I try quick info over and over. I think this is something we'll really need to provide a visual indication for before we make it broadly available.

From the TypeScript side:

  • I think cross-file go-to-definition was one of the key scenarios we'd like to get to users, so I think we should really try to get that to light up, either here or in a follow-up PR.
  • the fact that we showany when things aren't resolved in this mode is kind of confusing. For type display purposes, we might want to consider displaying the error type as something like aloading type.

@CyrusNajmabadi, how does Roslyn work when types haven't been resolved yet in things like quick info and signature help?

For example we have this signature

image

butCodeActionsState.State can't be resolved, so signature help looks like this:

Untitled

@CyrusNajmabadi
Copy link
Contributor

@CyrusNajmabadi, how does Roslyn work when types haven't been resolved yet in things like quick info and signature help?

We don't have that concept :)

DanielRosenwasser reacted with thumbs up emoji

@sheetalkamatsheetalkamat marked this pull request as ready for reviewJune 13, 2020 00:06
CommandNames.CompileOnSaveEmitFile,
CommandNames.CompilerOptionsDiagnosticsFull,
CommandNames.EncodedSemanticClassificationsFull,
CommandNames.SemanticDiagnosticsSync,
Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

SyntacticDiagnosticsSync: This is somewhat project setting dependent ? Atleast at some point parsing errors use to be different based on target .. eg target determines what unicode is considered identifier start... So i am not sure if this should be enabled.. But if we do enable we i was wondering if GetErr should only do syntax checks and skip semantic and suggetion diagnostics on syntax server

Some questionable which i have disabled for now
Reload
ReloadProjects
PrepareCallHierarchy
ProvideCallHierarchyIncomingCalls
ProvideCallHierarchyOutgoingCalls

@sheetalkamat
Copy link
MemberAuthor

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commentedJun 13, 2020
edited
Loading

Heya@sheetalkamat, I've started to run the tarball bundle task on this PR at89127a5. You can monitor the buildhere.

@typescript-bot
Copy link
Collaborator

typescript-bot commentedJun 13, 2020
edited
Loading

Hey@sheetalkamat, I've packed this intoan installable tgz. You can install it for testing by referencing it in yourpackage.json like so:

{    "devDependencies": {        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/76345/artifacts?artifactName=tgz&fileId=CD3C20B0D73291C13F028F1A072CA740FDD866AD700C026ACA443AFA3B0B0C3002&fileName=/typescript-4.0.0-insiders.20200613.tgz"    }}

and then runningnpm install.


There is also a playgroundfor this build.

Copy link
Member

@amcaseyamcasey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Some comments, but LGTM

@@ -1171,6 +1171,26 @@ namespace ts {
}
}

const invalidOperationsOnSyntaxOnly: readonly (keyof LanguageService)[] = [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

How does this relate to the list in session.ts? Do they need to stay in sync? Is one redundant?

if (syntaxOnly) {
invalidOperationsOnSyntaxOnly.forEach(key =>
ls[key] = (...args: any[]) => {
throw new Error(`LanguageService Operation: ${key} not allowed on syntaxServer:: arguments::${JSON.stringify(args)}`);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

As above, we probably don't need the arguments.

@DanielRosenwasser
Copy link
Member

Speaking of logs- do editors log from the syntax server?

@amcasey
Copy link
Member

VS logging is opt-in (via env var or regkey), but, yes, it applies to both.

VS Code only has a command to show the semantic log (AFAIK), but I'm pretty sure it enables both.

@sheetalkamat
Copy link
MemberAuthor

@mjbvz We definitely want users to be able to share the syntaxOnly server logs easily.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@amcaseyamcaseyamcasey approved these changes

@andrewbranchandrewbranchAwaiting requested review from andrewbranch

@DanielRosenwasserDanielRosenwasserAwaiting requested review from DanielRosenwasser

@RyanCavanaughRyanCavanaughAwaiting requested review from RyanCavanaugh

Assignees

@sheetalkamatsheetalkamat

Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

5 participants
@sheetalkamat@typescript-bot@amcasey@DanielRosenwasser@CyrusNajmabadi

[8]ページ先頭

©2009-2025 Movatter.jp