Skip to content

NodeTypingsInstaller.handleMessage should treat PackageInstalledResponse roughly the same as SetTypings #20084

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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom
Closed
amcasey opened this issue Nov 17, 2017 · 1 comment
Closed

NodeTypingsInstaller.handleMessage should treat PackageInstalledResponse roughly the same as SetTypings #20084

amcasey opened this issue Nov 17, 2017 · 1 comment
Assignees
@amcasey
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue
Milestone
TypeScript 2.7

Comments

@amcasey
Copy link
Member

amcasey commented Nov 17, 2017

In particular, PackageInstalledResponse should probably be a ProjectResponse and handleMessage should call

                    this.projectService.updateTypingsForProject(response);

                    if (this.socket) {
                        this.sendEvent(0, "setTypings", response);
                    }

I'll leave it to the implementer to decide whether the event name should be different - VS doesn't need to distinguish.
@amcasey amcasey assigned ghost Nov 17, 2017
@amcasey amcasey mentioned this issue Nov 17, 2017
Closed
@mhegazy mhegazy added the Bug A bug in TypeScript label Nov 17, 2017
@mhegazy mhegazy added this to the TypeScript 2.6.2 milestone Nov 17, 2017
amcasey added a commit to amcasey/TypeScript that referenced this issue Nov 21, 2017
@amcasey
Update project on PackageInstalledResponse
e2a0551 
As with `SetTypings`, new typings have been installed so the project
should be updated and the client should be notified (via event).

Changed PackageInstalledResponse from "event" to "action" for the sake
of explicitness.

Fixes microsoft#20084.
amcasey added a commit to amcasey/TypeScript that referenced this issue Nov 21, 2017
@amcasey
Update project on PackageInstalledResponse
35545d4 
As with `SetTypings`, new typings have been installed so the project
should be updated and the client should be notified (via event).

Changed PackageInstalledResponse from "event" to "action" for the sake
of explicitness.

Fixes microsoft#20084.
@amcasey amcasey mentioned this issue Nov 22, 2017
Merged
@amcasey amcasey assigned amcasey and unassigned ghost Nov 22, 2017
amcasey added a commit to amcasey/TypeScript that referenced this issue Nov 22, 2017
@amcasey
Update project on PackageInstalledResponse
34b9d09 
As with `SetTypings`, new typings have been installed so the project
should be updated and the client should be notified (via event).

Changed PackageInstalledResponse from "event" to "action" for the sake
of explicitness.

Fixes microsoft#20084.
@mhegazy mhegazy added the Fixed A PR has been merged for this issue label Nov 22, 2017
@mhegazy mhegazy modified the milestones: TypeScript 2.6.2, TypeScript 2.7, TypeScript 2.6.3 Nov 22, 2017
@mhegazy
Copy link
Contributor

mhegazy commented Nov 22, 2017

marking this as 2.6.3.

@mhegazy mhegazy modified the milestones: TypeScript 2.6.3, TypeScript 2.7 Jan 17, 2018
@microsoft microsoft locked and limited conversation to collaborators Jul 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@amcasey amcasey

Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Milestone
TypeScript 2.7
Development

No branches or pull requests
2 participants
@mhegazy @amcasey